Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 06c0c36

Browse files
committed
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 ea91253, 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
1 parent 1ccc1e0 commit 06c0c36

File tree

2 files changed

+27
-7
lines changed

2 files changed

+27
-7
lines changed

src/bin/pg_dump/pg_backup_archiver.c

+21-3
Original file line numberDiff line numberDiff line change
@@ -2879,7 +2879,10 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH)
28792879
* TOC entry types only if their parent object is being restored.
28802880
* Without selectivity options, we let through everything in the
28812881
* archive. Note there may be such entries with no parent, eg
2882-
* non-default ACLs for built-in objects.
2882+
* non-default ACLs for built-in objects. Also, we make
2883+
* per-column ACLs additionally depend on the table's ACL if any
2884+
* to ensure correct restore order, so those dependencies should
2885+
* be ignored in this check.
28832886
*
28842887
* This code depends on the parent having been marked already,
28852888
* which should be the case; if it isn't, perhaps due to
@@ -2890,8 +2893,23 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH)
28902893
* But it's hard to tell which of their dependencies is the one to
28912894
* consult.
28922895
*/
2893-
if (te->nDeps != 1 ||
2894-
TocIDRequired(AH, te->dependencies[0]) == 0)
2896+
bool dumpthis = false;
2897+
2898+
for (int i = 0; i < te->nDeps; i++)
2899+
{
2900+
TocEntry *pte = getTocEntryByDumpId(AH, te->dependencies[i]);
2901+
2902+
if (!pte)
2903+
continue; /* probably shouldn't happen */
2904+
if (strcmp(pte->desc, "ACL") == 0)
2905+
continue; /* ignore dependency on another ACL */
2906+
if (pte->reqs == 0)
2907+
continue; /* this object isn't marked, so ignore it */
2908+
/* Found a parent to be dumped, so we want to dump this too */
2909+
dumpthis = true;
2910+
break;
2911+
}
2912+
if (!dumpthis)
28952913
return 0;
28962914
}
28972915
}

src/bin/pg_dump/t/002_pg_dump.pl

+6-4
Original file line numberDiff line numberDiff line change
@@ -4245,11 +4245,13 @@
42454245
42464246
'GRANT SELECT ON TABLE measurement' => {
42474247
create_order => 91,
4248-
create_sql => 'GRANT SELECT ON
4249-
TABLE dump_test.measurement
4250-
TO regress_dump_test_role;',
4248+
create_sql => 'GRANT SELECT ON TABLE dump_test.measurement
4249+
TO regress_dump_test_role;
4250+
GRANT SELECT(city_id) ON TABLE dump_test.measurement
4251+
TO "regress_quoted \"" role";',
42514252
regexp =>
4252-
qr/^\QGRANT SELECT ON TABLE dump_test.measurement TO regress_dump_test_role;\E/m,
4253+
qr/^\QGRANT SELECT ON TABLE dump_test.measurement TO regress_dump_test_role;\E\n.*
4254+
^\QGRANT SELECT(city_id) ON TABLE dump_test.measurement TO "regress_quoted \"" role";\E/xms,
42534255
like => {
42544256
%full_runs,
42554257
%dump_test_schema_runs,

0 commit comments

Comments
 (0)