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

Commit bc9aaac

Browse files
tglsfdctglsfdc
committed
Avoid trying to restore table ACLs and per-column ACLs in parallel.
Parallel pg_restore has always supposed that ACL items for different objects are independent and can be restored in parallel without conflicts. However, there is one case where this fails: because REVOKE on a table is defined to also revoke the privilege(s) at column level, we can't restore per-column ACLs till after we restore any table-level privileges on their table. Failure to honor this restriction can lead to "tuple concurrently updated" errors during parallel restore, or even to the per-column ACLs silently disappearing because the table-level REVOKE is executed afterwards. To fix, add a dependency from each column-level ACL item to its table's ACL item, if there is one. Note that this doesn't fix the hazard for pre-existing archive files, only for ones made with a corrected pg_dump. Given that the bug's been there quite awhile without field reports, I think this is acceptable. This requires changing the API of pg_dump's dumpACL() function. To keep its argument list from getting even longer, I removed the "CatalogId objCatId" argument, which has been unused for ages. Per report from Justin Pryzby. Back-patch to all supported branches. Discussion: https://postgr.es/m/20200706050129.GW4107@telsasoft.com
1 parent 89a0b1a commit bc9aaac

File tree

3 files changed

+64
-35
lines changed

3 files changed

+64
-35
lines changed

‎src/bin/pg_dump/common.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
*/
2929
static DumpableObject **dumpIdMap = NULL;
3030
static int allocedDumpIds = 0;
31-
static DumpId lastDumpId = 0;
31+
static DumpId lastDumpId = 0; /* Note: 0 is InvalidDumpId */
3232

3333
/*
3434
* Variables for mapping CatalogId to DumpableObject

‎src/bin/pg_dump/pg_backup.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,12 @@ typedef struct
233233

234234
typedef int DumpId;
235235

236+
#define InvalidDumpId 0
237+
238+
/*
239+
* Function pointer prototypes for assorted callback methods.
240+
*/
241+
236242
typedef int (*DataDumperPtr) (Archive *AH, void *userArg);
237243

238244
typedef void (*SetupWorkerPtrType) (Archive *AH);

‎src/bin/pg_dump/pg_dump.c

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,11 @@ static void dumpUserMappings(Archive *fout,
220220
const char *owner, CatalogId catalogId, DumpId dumpId);
221221
static void dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo);
222222

223-
static void dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId,
224-
const char *type, const char *name, const char *subname,
225-
const char *nspname, const char *owner,
226-
const char *acls, const char *racls,
227-
const char *initacls, const char *initracls);
223+
static DumpId dumpACL(Archive *fout, DumpId objDumpId, DumpId altDumpId,
224+
const char *type, const char *name, const char *subname,
225+
const char *nspname, const char *owner,
226+
const char *acls, const char *racls,
227+
const char *initacls, const char *initracls);
228228

229229
static void getDependencies(Archive *fout);
230230
static void BuildArchiveDependencies(Archive *fout);
@@ -2992,7 +2992,7 @@ dumpDatabase(Archive *fout)
29922992
* Dump ACL if any. Note that we do not support initial privileges
29932993
* (pg_init_privs) on databases.
29942994
*/
2995-
dumpACL(fout, dbCatId, dbDumpId, "DATABASE",
2995+
dumpACL(fout, dbDumpId, InvalidDumpId, "DATABASE",
29962996
qdatname, NULL, NULL,
29972997
dba, datacl, rdatacl, "", "");
29982998

@@ -3477,7 +3477,7 @@ dumpBlob(Archive *fout, BlobInfo *binfo)
34773477

34783478
/* Dump ACL if any */
34793479
if (binfo->blobacl && (binfo->dobj.dump & DUMP_COMPONENT_ACL))
3480-
dumpACL(fout, binfo->dobj.catId, binfo->dobj.dumpId, "LARGE OBJECT",
3480+
dumpACL(fout, binfo->dobj.dumpId, InvalidDumpId, "LARGE OBJECT",
34813481
binfo->dobj.name, NULL,
34823482
NULL, binfo->rolname, binfo->blobacl, binfo->rblobacl,
34833483
binfo->initblobacl, binfo->initrblobacl);
@@ -10208,7 +10208,7 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
1020810208
nspinfo->dobj.catId, 0, nspinfo->dobj.dumpId);
1020910209

1021010210
if (nspinfo->dobj.dump & DUMP_COMPONENT_ACL)
10211-
dumpACL(fout, nspinfo->dobj.catId, nspinfo->dobj.dumpId, "SCHEMA",
10211+
dumpACL(fout, nspinfo->dobj.dumpId, InvalidDumpId, "SCHEMA",
1021210212
qnspname, NULL, NULL,
1021310213
nspinfo->rolname, nspinfo->nspacl, nspinfo->rnspacl,
1021410214
nspinfo->initnspacl, nspinfo->initrnspacl);
@@ -10492,7 +10492,7 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
1049210492
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
1049310493

1049410494
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
10495-
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10495+
dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
1049610496
qtypname, NULL,
1049710497
tyinfo->dobj.namespace->dobj.name,
1049810498
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@@ -10618,7 +10618,7 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
1061810618
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
1061910619

1062010620
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
10621-
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10621+
dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
1062210622
qtypname, NULL,
1062310623
tyinfo->dobj.namespace->dobj.name,
1062410624
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@@ -10690,7 +10690,7 @@ dumpUndefinedType(Archive *fout, TypeInfo *tyinfo)
1069010690
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
1069110691

1069210692
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
10693-
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10693+
dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
1069410694
qtypname, NULL,
1069510695
tyinfo->dobj.namespace->dobj.name,
1069610696
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@@ -10971,7 +10971,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
1097110971
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
1097210972

1097310973
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
10974-
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10974+
dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
1097510975
qtypname, NULL,
1097610976
tyinfo->dobj.namespace->dobj.name,
1097710977
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@@ -11127,7 +11127,7 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
1112711127
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
1112811128

1112911129
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
11130-
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
11130+
dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
1113111131
qtypname, NULL,
1113211132
tyinfo->dobj.namespace->dobj.name,
1113311133
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@@ -11349,7 +11349,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
1134911349
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
1135011350

1135111351
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
11352-
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
11352+
dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
1135311353
qtypname, NULL,
1135411354
tyinfo->dobj.namespace->dobj.name,
1135511355
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@@ -11649,7 +11649,7 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
1164911649
plang->dobj.catId, 0, plang->dobj.dumpId);
1165011650

1165111651
if (plang->lanpltrusted && plang->dobj.dump & DUMP_COMPONENT_ACL)
11652-
dumpACL(fout, plang->dobj.catId, plang->dobj.dumpId, "LANGUAGE",
11652+
dumpACL(fout, plang->dobj.dumpId, InvalidDumpId, "LANGUAGE",
1165311653
qlanname, NULL, NULL,
1165411654
plang->lanowner, plang->lanacl, plang->rlanacl,
1165511655
plang->initlanacl, plang->initrlanacl);
@@ -12359,7 +12359,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1235912359
finfo->dobj.catId, 0, finfo->dobj.dumpId);
1236012360

1236112361
if (finfo->dobj.dump & DUMP_COMPONENT_ACL)
12362-
dumpACL(fout, finfo->dobj.catId, finfo->dobj.dumpId, keyword,
12362+
dumpACL(fout, finfo->dobj.dumpId, InvalidDumpId, keyword,
1236312363
funcsig, NULL,
1236412364
finfo->dobj.namespace->dobj.name,
1236512365
finfo->rolname, finfo->proacl, finfo->rproacl,
@@ -14380,7 +14380,7 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
1438014380
aggsig = format_function_signature(fout, &agginfo->aggfn, true);
1438114381

1438214382
if (agginfo->aggfn.dobj.dump & DUMP_COMPONENT_ACL)
14383-
dumpACL(fout, agginfo->aggfn.dobj.catId, agginfo->aggfn.dobj.dumpId,
14383+
dumpACL(fout, agginfo->aggfn.dobj.dumpId, InvalidDumpId,
1438414384
"FUNCTION", aggsig, NULL,
1438514385
agginfo->aggfn.dobj.namespace->dobj.name,
1438614386
agginfo->aggfn.rolname, agginfo->aggfn.proacl,
@@ -14782,7 +14782,7 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
1478214782

1478314783
/* Handle the ACL */
1478414784
if (fdwinfo->dobj.dump & DUMP_COMPONENT_ACL)
14785-
dumpACL(fout, fdwinfo->dobj.catId, fdwinfo->dobj.dumpId,
14785+
dumpACL(fout, fdwinfo->dobj.dumpId, InvalidDumpId,
1478614786
"FOREIGN DATA WRAPPER", qfdwname, NULL,
1478714787
NULL, fdwinfo->rolname,
1478814788
fdwinfo->fdwacl, fdwinfo->rfdwacl,
@@ -14871,7 +14871,7 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
1487114871

1487214872
/* Handle the ACL */
1487314873
if (srvinfo->dobj.dump & DUMP_COMPONENT_ACL)
14874-
dumpACL(fout, srvinfo->dobj.catId, srvinfo->dobj.dumpId,
14874+
dumpACL(fout, srvinfo->dobj.dumpId, InvalidDumpId,
1487514875
"FOREIGN SERVER", qsrvname, NULL,
1487614876
NULL, srvinfo->rolname,
1487714877
srvinfo->srvacl, srvinfo->rsrvacl,
@@ -15064,8 +15064,9 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
1506415064
/*----------
1506515065
* Write out grant/revoke information
1506615066
*
15067-
* 'objCatId' is the catalog ID of the underlying object.
1506815067
* 'objDumpId' is the dump ID of the underlying object.
15068+
* 'altDumpId' can be a second dumpId that the ACL entry must also depend on,
15069+
* or InvalidDumpId if there is no need for a second dependency.
1506915070
* 'type' must be one of
1507015071
* TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, TABLESPACE,
1507115072
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT.
@@ -15088,25 +15089,29 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
1508815089
* NB: initacls/initracls are needed because extensions can set privileges on
1508915090
* an object during the extension's script file and we record those into
1509015091
* pg_init_privs as that object's initial privileges.
15092+
*
15093+
* Returns the dump ID assigned to the ACL TocEntry, or InvalidDumpId if
15094+
* no ACL entry was created.
1509115095
*----------
1509215096
*/
15093-
static void
15094-
dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId,
15097+
static DumpId
15098+
dumpACL(Archive *fout, DumpId objDumpId, DumpId altDumpId,
1509515099
const char *type, const char *name, const char *subname,
1509615100
const char *nspname, const char *owner,
1509715101
const char *acls, const char *racls,
1509815102
const char *initacls, const char *initracls)
1509915103
{
15104+
DumpId aclDumpId = InvalidDumpId;
1510015105
DumpOptions *dopt = fout->dopt;
1510115106
PQExpBuffer sql;
1510215107

1510315108
/* Do nothing if ACL dump is not enabled */
1510415109
if (dopt->aclsSkip)
15105-
return;
15110+
return InvalidDumpId;
1510615111

1510715112
/* --data-only skips ACLs *except* BLOB ACLs */
1510815113
if (dopt->dataOnly && strcmp(type, "LARGE OBJECT") != 0)
15109-
return;
15114+
return InvalidDumpId;
1511015115

1511115116
sql = createPQExpBuffer();
1511215117

@@ -15138,25 +15143,36 @@ dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId,
1513815143
if (sql->len > 0)
1513915144
{
1514015145
PQExpBuffer tag = createPQExpBuffer();
15146+
DumpId aclDeps[2];
15147+
int nDeps = 0;
1514115148

1514215149
if (subname)
1514315150
appendPQExpBuffer(tag, "COLUMN %s.%s", name, subname);
1514415151
else
1514515152
appendPQExpBuffer(tag, "%s %s", type, name);
1514615153

15147-
ArchiveEntry(fout, nilCatalogId, createDumpId(),
15154+
aclDeps[nDeps++] = objDumpId;
15155+
if (altDumpId != InvalidDumpId)
15156+
aclDeps[nDeps++] = altDumpId;
15157+
15158+
aclDumpId = createDumpId();
15159+
15160+
ArchiveEntry(fout, nilCatalogId, aclDumpId,
1514815161
ARCHIVE_OPTS(.tag = tag->data,
1514915162
.namespace = nspname,
1515015163
.owner = owner,
1515115164
.description = "ACL",
1515215165
.section = SECTION_NONE,
1515315166
.createStmt = sql->data,
15154-
.deps = &objDumpId,
15155-
.nDeps = 1));
15167+
.deps = aclDeps,
15168+
.nDeps = nDeps));
15169+
1515615170
destroyPQExpBuffer(tag);
1515715171
}
1515815172

1515915173
destroyPQExpBuffer(sql);
15174+
15175+
return aclDumpId;
1516015176
}
1516115177

1516215178
/*
@@ -15482,6 +15498,7 @@ static void
1548215498
dumpTable(Archive *fout, TableInfo *tbinfo)
1548315499
{
1548415500
DumpOptions *dopt = fout->dopt;
15501+
DumpId tableAclDumpId = InvalidDumpId;
1548515502
char *namecopy;
1548615503

1548715504
/*
@@ -15503,11 +15520,12 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1550315520
const char *objtype =
1550415521
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" : "TABLE";
1550515522

15506-
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
15507-
objtype, namecopy, NULL,
15508-
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
15509-
tbinfo->relacl, tbinfo->rrelacl,
15510-
tbinfo->initrelacl, tbinfo->initrrelacl);
15523+
tableAclDumpId =
15524+
dumpACL(fout, tbinfo->dobj.dumpId, InvalidDumpId,
15525+
objtype, namecopy, NULL,
15526+
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
15527+
tbinfo->relacl, tbinfo->rrelacl,
15528+
tbinfo->initrelacl, tbinfo->initrrelacl);
1551115529
}
1551215530

1551315531
/*
@@ -15591,8 +15609,13 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1559115609
char *attnamecopy;
1559215610

1559315611
attnamecopy = pg_strdup(fmtId(attname));
15594-
/* Column's GRANT type is always TABLE */
15595-
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
15612+
15613+
/*
15614+
* Column's GRANT type is always TABLE. Each column ACL depends
15615+
* on the table-level ACL, since we can restore column ACLs in
15616+
* parallel but the table-level ACL has to be done first.
15617+
*/
15618+
dumpACL(fout, tbinfo->dobj.dumpId, tableAclDumpId,
1559615619
"TABLE", namecopy, attnamecopy,
1559715620
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
1559815621
attacl, rattacl, initattacl, initrattacl);

0 commit comments

Comments
 (0)