Fix omission of column-level privileges in selective pg_restore.

In a selective restore, ACLs for a table should be dumped if the
table is selected to be dumped.  However, if the table has both
table-level and column-level ACLs, only the table-level ACL was
restored.  This happened because _tocEntryRequired assumed that
an ACL could have only one dependency (the one on its table),
and punted if there was more than one.  But since commit ea9125304,
column-level ACLs also depend on the table-level ACL if any, to
ensure correct ordering in parallel restores.  To fix, adjust the
logic in _tocEntryRequired to ignore dependencies on ACLs.

I extended a test case in 002_pg_dump.pl so that it purports to
test for this; but in fact the test passes even without the fix.
That's because this bug only manifests during a selective restore,
while the scenarios 002_pg_dump.pl tests include only selective dumps.
Perhaps somebody would like to extend the script so that it can test
scenarios including selective restore, but I'm not touching that.

Euler Taveira and Tom Lane, per report from Kong Man.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/DM4PR11MB73976902DBBA10B1D652F9498B06A@DM4PR11MB7397.namprd11.prod.outlook.com

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index cc06f1c81773f6bd68853edf6ce2ac67093457e9..256d1e35a4ecc61c8df648a3dd349d3809d1d074 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2879,7 +2879,10 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH)
             * TOC entry types only if their parent object is being restored.
             * Without selectivity options, we let through everything in the
             * archive.  Note there may be such entries with no parent, eg
-            * non-default ACLs for built-in objects.
+            * non-default ACLs for built-in objects.  Also, we make
+            * per-column ACLs additionally depend on the table's ACL if any
+            * to ensure correct restore order, so those dependencies should
+            * be ignored in this check.
             *
             * This code depends on the parent having been marked already,
             * which should be the case; if it isn't, perhaps due to
@@ -2890,8 +2893,23 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH)
             * But it's hard to tell which of their dependencies is the one to
             * consult.
             */
-           if (te->nDeps != 1 ||
-               TocIDRequired(AH, te->dependencies[0]) == 0)
+           bool        dumpthis = false;
+
+           for (int i = 0; i < te->nDeps; i++)
+           {
+               TocEntry   *pte = getTocEntryByDumpId(AH, te->dependencies[i]);
+
+               if (!pte)
+                   continue;   /* probably shouldn't happen */
+               if (strcmp(pte->desc, "ACL") == 0)
+                   continue;   /* ignore dependency on another ACL */
+               if (pte->reqs == 0)
+                   continue;   /* this object isn't marked, so ignore it */
+               /* Found a parent to be dumped, so we want to dump this too */
+               dumpthis = true;
+               break;
+           }
+           if (!dumpthis)
                return 0;
        }
    }

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 55e98ec8e39d1470165a87316e347d8b0ff5739f..326c9a763944a9f8b1968b8377869ba6482aeade 100644 (file)
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4245,11 +4245,13 @@ my %tests = (
 
    'GRANT SELECT ON TABLE measurement' => {
        create_order => 91,
-       create_sql => 'GRANT SELECT ON
-                          TABLE dump_test.measurement
-                          TO regress_dump_test_role;',
+       create_sql => 'GRANT SELECT ON TABLE dump_test.measurement
+                          TO regress_dump_test_role;
+                      GRANT SELECT(city_id) ON TABLE dump_test.measurement
+                          TO "regress_quoted  \"" role";',
        regexp =>
-         qr/^\QGRANT SELECT ON TABLE dump_test.measurement TO regress_dump_test_role;\E/m,
+         qr/^\QGRANT SELECT ON TABLE dump_test.measurement TO regress_dump_test_role;\E\n.*
+            ^\QGRANT SELECT(city_id) ON TABLE dump_test.measurement TO "regress_quoted  \"" role";\E/xms,
        like => {
            %full_runs,
            %dump_test_schema_runs,