pg_upgrade: Fix large object COMMENTS, SECURITY LABELS

When performing a pg_upgrade, we copy the files behind pg_largeobject
and pg_largeobject_metadata, allowing us to avoid having to dump out and
reload the actual data for large objects and their ACLs.

Unfortunately, that isn't all of the information which can be associated
with large objects.  Currently, we also support COMMENTs and SECURITY
LABELs with large objects and these were being silently dropped during a
pg_upgrade as pg_dump would skip everything having to do with a large
object and pg_upgrade only copied the tables mentioned to the new
cluster.

As the file copies happen after the catalog dump and reload, we can't
simply include the COMMENTs and SECURITY LABELs in pg_dump's binary-mode
output but we also have to include the actual large object definition as
well.  With the definition, comments, and security labels in the pg_dump
output and the file copies performed by pg_upgrade, all of the data and
metadata associated with large objects is able to be successfully pulled
forward across a pg_upgrade.

In 9.6 and master, we can simply adjust the dump bitmask to indicate
which components we don't want.  In 9.5 and earlier, we have to put
explciit checks in in dumpBlob() and dumpBlobs() to not include the ACL
or the data when in binary-upgrade mode.

Adjustments made to the privileges regression test to allow another test
(large_object.sql) to be added which explicitly leaves a large object
with a comment in place to provide coverage of that case with
pg_upgrade.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/20170221162655[email protected]

diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 7ee560e8cd46786d65448dadf54ee8f170ca9d4e..6f65733e64786e25f1d4a2aef3f537022f2fc22e 100644 (file)
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -116,6 +116,7 @@ typedef struct _restoreOptions
 
    bool       *idWanted;       /* array showing which dump IDs to emit */
    int         enable_row_security;
+   int         binary_upgrade;
 } RestoreOptions;
 
 typedef struct _dumpOptions

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 6539235544c38a6860b5ee3d0de624941948fa04..c9d68d4de92b51a167da731c84bcb2a6a6a5d6e4 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2780,7 +2780,17 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
 
    /* Mask it if we only want schema */
    if (ropt->schemaOnly)
-       res = res & REQ_SCHEMA;
+   {
+       /*
+        * In binary-upgrade mode, even with schema-only set, we do not mask
+        * out large objects.  Only large object definitions, comments and
+        * other information should be generated in binary-upgrade mode (not
+        * the actual data).
+        */
+       if (!(ropt->binary_upgrade && strcmp(te->desc,"BLOB") == 0) &&
+       !(ropt->binary_upgrade && strncmp(te->tag,"LARGE OBJECT ", 13) == 0))
+           res = res & REQ_SCHEMA;
+   }
 
    /* Mask it if we only want data */
    if (ropt->dataOnly)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 180f51c76bce10c9f712500feb43d2f5ce37edeb..16f6c92d450fea641a374ec4d7b52f86ee0a5bf2 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -747,7 +747,15 @@ main(int argc, char **argv)
            getTableDataFKConstraints();
    }
 
-   if (dopt.outputBlobs)
+   /*
+    * In binary-upgrade mode, we do not have to worry about the actual blob
+    * data or the associated metadata that resides in the pg_largeobject and
+    * pg_largeobject_metadata tables, respectivly.
+    *
+    * However, we do need to collect blob information as there may be
+    * comments or other information on blobs that we do need to dump out.
+    */
+   if (dopt.outputBlobs || dopt.binary_upgrade)
        getBlobs(fout);
 
    /*
@@ -830,6 +838,7 @@ main(int argc, char **argv)
    ropt->lockWaitTimeout = dopt.lockWaitTimeout;
    ropt->include_everything = dopt.include_everything;
    ropt->enable_row_security = dopt.enable_row_security;
+   ropt->binary_upgrade = dopt.binary_upgrade;
 
    if (compressLevel == -1)
        ropt->compression = 0;
@@ -2825,8 +2834,14 @@ dumpBlob(Archive *fout, BlobInfo *binfo)
                 NULL, binfo->rolname,
                 binfo->dobj.catId, 0, binfo->dobj.dumpId);
 
-   /* Dump ACL if any */
-   if (binfo->blobacl)
+   /*
+    * Dump ACL if any
+    *
+    * Do not dump the ACL in binary-upgrade mode, however, as the ACL will be
+    * copied over by pg_upgrade as it is part of the pg_largeobject_metadata
+    * table.
+    */
+   if (binfo->blobacl && !fout->dopt->binary_upgrade)
        dumpACL(fout, binfo->dobj.catId, binfo->dobj.dumpId, "LARGE OBJECT",
                binfo->dobj.name, NULL, cquery->data,
                NULL, binfo->rolname, binfo->blobacl);
@@ -2851,6 +2866,13 @@ dumpBlobs(Archive *fout, void *arg)
    int         i;
    int         cnt;
 
+   /*
+    * Do not dump out blob data in binary-upgrade mode, pg_upgrade will copy
+    * the pg_largeobject table over entirely from the old cluster.
+    */
+   if (fout->dopt->binary_upgrade)
+       return 1;
+
    if (g_verbose)
        write_msg(NULL, "saving large objects\n");
 
@@ -8113,7 +8135,8 @@ dumpComment(Archive *fout, const char *target,
    }
    else
    {
-       if (dopt->schemaOnly)
+       /* We do dump blob comments in binary-upgrade mode */
+       if (dopt->schemaOnly && !dopt->binary_upgrade)
            return;
    }
 
@@ -13496,7 +13519,8 @@ dumpSecLabel(Archive *fout, const char *target,
    }
    else
    {
-       if (dopt->schemaOnly)
+       /* We do dump blob security labels in binary-upgrade mode */
+       if (dopt->schemaOnly && !dopt->binary_upgrade)
            return;
    }
 

diff --git a/src/test/regress/expected/large_object.out b/src/test/regress/expected/large_object.out
new file mode 100644 (file)
index 0000000..b00d47c
--- /dev/null
+++ b/src/test/regress/expected/large_object.out
@@ -0,0 +1,15 @@
+-- This is more-or-less DROP IF EXISTS LARGE OBJECT 3001;
+WITH unlink AS (SELECT lo_unlink(loid) FROM pg_largeobject WHERE loid = 3001) SELECT 1;
+ ?column? 
+----------
+        1
+(1 row)
+
+-- Test creation of a large object and leave it for testing pg_upgrade
+SELECT lo_create(3001);
+ lo_create 
+-----------
+      3001
+(1 row)
+
+COMMENT ON LARGE OBJECT 3001 IS 'testing comments';

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index c0cd9fac46896f58ff500d41c0f2714d2612b40f..cf6c54dbfc4bbe00ccb0c6fa91c6d05c5c261b74 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -12,7 +12,7 @@ DROP ROLE IF EXISTS regressuser3;
 DROP ROLE IF EXISTS regressuser4;
 DROP ROLE IF EXISTS regressuser5;
 DROP ROLE IF EXISTS regressuser6;
-SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
+SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
  lo_unlink 
 -----------
 (0 rows)
@@ -1174,11 +1174,11 @@ SELECT lo_unlink(2002);
 
 \c -
 -- confirm ACL setting
-SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata;
+SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
  oid  |  ownername   |                                          lomacl                                          
 ------+--------------+------------------------------------------------------------------------------------------
- 1002 | regressuser1 | 
  1001 | regressuser1 | {regressuser1=rw/regressuser1,=rw/regressuser1}
+ 1002 | regressuser1 | 
  1003 | regressuser1 | {regressuser1=rw/regressuser1,regressuser2=r/regressuser1}
  1004 | regressuser1 | {regressuser1=rw/regressuser1,regressuser2=rw/regressuser1}
  1005 | regressuser1 | {regressuser1=rw/regressuser1,regressuser2=r*w/regressuser1,regressuser3=r/regressuser2}
@@ -1547,7 +1547,7 @@ DROP TABLE atest6;
 DROP TABLE atestc;
 DROP TABLE atestp1;
 DROP TABLE atestp2;
-SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
+SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
  lo_unlink 
 -----------
          1

diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 60854066114fd4d6c05364b08af0d77c29d0ef0e..4e318763bce7a55b955d52777f4aea9532a95493 100644 (file)
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -84,7 +84,7 @@ test: select_into select_distinct select_distinct_on select_implicit select_havi
 # ----------
 # Another group of parallel tests
 # ----------
-test: brin gin gist spgist privileges security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets
+test: brin gin gist spgist privileges security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets large_object
 
 # ----------
 # Another group of parallel tests

diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 985f6c9f02f5b28101db87f2caf92e980f225bfa..e23ad31adb374084287aab4622fb784d09067dcf 100644 (file)
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -112,6 +112,7 @@ test: replica_identity
 test: rowsecurity
 test: object_address
 test: tablesample
+test: large_object
 test: alter_generic
 test: misc
 test: psql

diff --git a/src/test/regress/sql/large_object.sql b/src/test/regress/sql/large_object.sql
new file mode 100644 (file)
index 0000000..c06b393
--- /dev/null
+++ b/src/test/regress/sql/large_object.sql
@@ -0,0 +1,7 @@
+-- This is more-or-less DROP IF EXISTS LARGE OBJECT 3001;
+WITH unlink AS (SELECT lo_unlink(loid) FROM pg_largeobject WHERE loid = 3001) SELECT 1;
+
+-- Test creation of a large object and leave it for testing pg_upgrade
+SELECT lo_create(3001);
+
+COMMENT ON LARGE OBJECT 3001 IS 'testing comments';

diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index c1837c497af598236b793f14919049828038dcdf..0be21cdcb0d52659b39811574d5d852df0e2ef4a 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -17,7 +17,7 @@ DROP ROLE IF EXISTS regressuser4;
 DROP ROLE IF EXISTS regressuser5;
 DROP ROLE IF EXISTS regressuser6;
 
-SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
+SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
 
 RESET client_min_messages;
 
@@ -729,7 +729,7 @@ SELECT lo_unlink(2002);
 
 \c -
 -- confirm ACL setting
-SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata;
+SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
 
 SET SESSION AUTHORIZATION regressuser3;
 
@@ -960,7 +960,7 @@ DROP TABLE atestc;
 DROP TABLE atestp1;
 DROP TABLE atestp2;
 
-SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
+SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
 
 DROP GROUP regressgroup1;
 DROP GROUP regressgroup2;