opensourcemirror
/
postgresql
mirror of git://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Releases Wiki Activity

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
This commit is contained in:
Tom Lane 2020-07-11 13:36:50 -04:00
parent f4ae676e31
commit 5fea14f4b2
3 changed files with 64 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/bin/pg_dump/common.c
View File

@ -30,7 +30,7 @@
*/ */
static DumpableObject **dumpIdMap = NULL; static DumpableObject **dumpIdMap = NULL;
static int allocedDumpIds = 0; static int allocedDumpIds = 0;
static DumpId lastDumpId = 0; static DumpId lastDumpId = 0; /* Note: 0 is InvalidDumpId */
/* /*
* Variables for mapping CatalogId to DumpableObject * Variables for mapping CatalogId to DumpableObject

6
src/bin/pg_dump/pg_backup.h
View File

@ -234,6 +234,12 @@ typedef struct
typedef int DumpId; typedef int DumpId;
#define InvalidDumpId 0
/*
* Function pointer prototypes for assorted callback methods.
*/
typedef int (*DataDumperPtr) (Archive *AH, void *userArg); typedef int (*DataDumperPtr) (Archive *AH, void *userArg);
typedef void (*SetupWorkerPtrType) (Archive *AH); typedef void (*SetupWorkerPtrType) (Archive *AH);

91
src/bin/pg_dump/pg_dump.c
View File

@ -226,11 +226,11 @@ static void dumpUserMappings(Archive *fout,
const char *owner, CatalogId catalogId, DumpId dumpId); const char *owner, CatalogId catalogId, DumpId dumpId);
static void dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo); static void dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo);
static void dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId, static DumpId dumpACL(Archive *fout, DumpId objDumpId, DumpId altDumpId,
const char *type, const char *name, const char *subname, const char *type, const char *name, const char *subname,
const char *nspname, const char *owner, const char *nspname, const char *owner,
const char *acls, const char *racls, const char *acls, const char *racls,
const char *initacls, const char *initracls); const char *initacls, const char *initracls);
static void getDependencies(Archive *fout); static void getDependencies(Archive *fout);
static void BuildArchiveDependencies(Archive *fout); static void BuildArchiveDependencies(Archive *fout);
@ -2922,7 +2922,7 @@ dumpDatabase(Archive *fout)
* Dump ACL if any. Note that we do not support initial privileges * Dump ACL if any. Note that we do not support initial privileges
* (pg_init_privs) on databases. * (pg_init_privs) on databases.
*/ */
dumpACL(fout, dbCatId, dbDumpId, "DATABASE", dumpACL(fout, dbDumpId, InvalidDumpId, "DATABASE",
qdatname, NULL, NULL, qdatname, NULL, NULL,
dba, datacl, rdatacl, "", ""); dba, datacl, rdatacl, "", "");
@ -3407,7 +3407,7 @@ dumpBlob(Archive *fout, BlobInfo *binfo)
/* Dump ACL if any */ /* Dump ACL if any */
if (binfo->blobacl && (binfo->dobj.dump & DUMP_COMPONENT_ACL)) if (binfo->blobacl && (binfo->dobj.dump & DUMP_COMPONENT_ACL))
dumpACL(fout, binfo->dobj.catId, binfo->dobj.dumpId, "LARGE OBJECT", dumpACL(fout, binfo->dobj.dumpId, InvalidDumpId, "LARGE OBJECT",
binfo->dobj.name, NULL, binfo->dobj.name, NULL,
NULL, binfo->rolname, binfo->blobacl, binfo->rblobacl, NULL, binfo->rolname, binfo->blobacl, binfo->rblobacl,
binfo->initblobacl, binfo->initrblobacl); binfo->initblobacl, binfo->initrblobacl);
@ -10085,7 +10085,7 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
nspinfo->dobj.catId, 0, nspinfo->dobj.dumpId); nspinfo->dobj.catId, 0, nspinfo->dobj.dumpId);
if (nspinfo->dobj.dump & DUMP_COMPONENT_ACL) if (nspinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, nspinfo->dobj.catId, nspinfo->dobj.dumpId, "SCHEMA", dumpACL(fout, nspinfo->dobj.dumpId, InvalidDumpId, "SCHEMA",
qnspname, NULL, NULL, qnspname, NULL, NULL,
nspinfo->rolname, nspinfo->nspacl, nspinfo->rnspacl, nspinfo->rolname, nspinfo->nspacl, nspinfo->rnspacl,
nspinfo->initnspacl, nspinfo->initrnspacl); nspinfo->initnspacl, nspinfo->initrnspacl);
@ -10369,7 +10369,7 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId); tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL) if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE", dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
qtypname, NULL, qtypname, NULL,
tyinfo->dobj.namespace->dobj.name, tyinfo->dobj.namespace->dobj.name,
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl, tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@ -10495,7 +10495,7 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId); tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL) if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE", dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
qtypname, NULL, qtypname, NULL,
tyinfo->dobj.namespace->dobj.name, tyinfo->dobj.namespace->dobj.name,
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl, tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@ -10567,7 +10567,7 @@ dumpUndefinedType(Archive *fout, TypeInfo *tyinfo)
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId); tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL) if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE", dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
qtypname, NULL, qtypname, NULL,
tyinfo->dobj.namespace->dobj.name, tyinfo->dobj.namespace->dobj.name,
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl, tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@ -10848,7 +10848,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId); tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL) if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE", dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
qtypname, NULL, qtypname, NULL,
tyinfo->dobj.namespace->dobj.name, tyinfo->dobj.namespace->dobj.name,
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl, tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@ -11004,7 +11004,7 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId); tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL) if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE", dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
qtypname, NULL, qtypname, NULL,
tyinfo->dobj.namespace->dobj.name, tyinfo->dobj.namespace->dobj.name,
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl, tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@ -11226,7 +11226,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId); tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL) if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE", dumpACL(fout, tyinfo->dobj.dumpId, InvalidDumpId, "TYPE",
qtypname, NULL, qtypname, NULL,
tyinfo->dobj.namespace->dobj.name, tyinfo->dobj.namespace->dobj.name,
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl, tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
@ -11526,7 +11526,7 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
plang->dobj.catId, 0, plang->dobj.dumpId); plang->dobj.catId, 0, plang->dobj.dumpId);
if (plang->lanpltrusted && plang->dobj.dump & DUMP_COMPONENT_ACL) if (plang->lanpltrusted && plang->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, plang->dobj.catId, plang->dobj.dumpId, "LANGUAGE", dumpACL(fout, plang->dobj.dumpId, InvalidDumpId, "LANGUAGE",
qlanname, NULL, NULL, qlanname, NULL, NULL,
plang->lanowner, plang->lanacl, plang->rlanacl, plang->lanowner, plang->lanacl, plang->rlanacl,
plang->initlanacl, plang->initrlanacl); plang->initlanacl, plang->initrlanacl);
@ -12236,7 +12236,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
finfo->dobj.catId, 0, finfo->dobj.dumpId); finfo->dobj.catId, 0, finfo->dobj.dumpId);
if (finfo->dobj.dump & DUMP_COMPONENT_ACL) if (finfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, finfo->dobj.catId, finfo->dobj.dumpId, keyword, dumpACL(fout, finfo->dobj.dumpId, InvalidDumpId, keyword,
funcsig, NULL, funcsig, NULL,
finfo->dobj.namespace->dobj.name, finfo->dobj.namespace->dobj.name,
finfo->rolname, finfo->proacl, finfo->rproacl, finfo->rolname, finfo->proacl, finfo->rproacl,
@ -14257,7 +14257,7 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
aggsig = format_function_signature(fout, &agginfo->aggfn, true); aggsig = format_function_signature(fout, &agginfo->aggfn, true);
if (agginfo->aggfn.dobj.dump & DUMP_COMPONENT_ACL) if (agginfo->aggfn.dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, agginfo->aggfn.dobj.catId, agginfo->aggfn.dobj.dumpId, dumpACL(fout, agginfo->aggfn.dobj.dumpId, InvalidDumpId,
"FUNCTION", aggsig, NULL, "FUNCTION", aggsig, NULL,
agginfo->aggfn.dobj.namespace->dobj.name, agginfo->aggfn.dobj.namespace->dobj.name,
agginfo->aggfn.rolname, agginfo->aggfn.proacl, agginfo->aggfn.rolname, agginfo->aggfn.proacl,
@ -14659,7 +14659,7 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
/* Handle the ACL */ /* Handle the ACL */
if (fdwinfo->dobj.dump & DUMP_COMPONENT_ACL) if (fdwinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, fdwinfo->dobj.catId, fdwinfo->dobj.dumpId, dumpACL(fout, fdwinfo->dobj.dumpId, InvalidDumpId,
"FOREIGN DATA WRAPPER", qfdwname, NULL, "FOREIGN DATA WRAPPER", qfdwname, NULL,
NULL, fdwinfo->rolname, NULL, fdwinfo->rolname,
fdwinfo->fdwacl, fdwinfo->rfdwacl, fdwinfo->fdwacl, fdwinfo->rfdwacl,
@ -14748,7 +14748,7 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
/* Handle the ACL */ /* Handle the ACL */
if (srvinfo->dobj.dump & DUMP_COMPONENT_ACL) if (srvinfo->dobj.dump & DUMP_COMPONENT_ACL)
dumpACL(fout, srvinfo->dobj.catId, srvinfo->dobj.dumpId, dumpACL(fout, srvinfo->dobj.dumpId, InvalidDumpId,
"FOREIGN SERVER", qsrvname, NULL, "FOREIGN SERVER", qsrvname, NULL,
NULL, srvinfo->rolname, NULL, srvinfo->rolname,
srvinfo->srvacl, srvinfo->rsrvacl, srvinfo->srvacl, srvinfo->rsrvacl,
@ -14941,8 +14941,9 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
/*---------- /*----------
* Write out grant/revoke information * Write out grant/revoke information
* *
* 'objCatId' is the catalog ID of the underlying object.
* 'objDumpId' is the dump ID of the underlying object. * 'objDumpId' is the dump ID of the underlying object.
* 'altDumpId' can be a second dumpId that the ACL entry must also depend on,
* or InvalidDumpId if there is no need for a second dependency.
* 'type' must be one of * 'type' must be one of
* TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, TABLESPACE, * TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, TABLESPACE,
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT. * FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT.
@ -14965,25 +14966,29 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
* NB: initacls/initracls are needed because extensions can set privileges on * NB: initacls/initracls are needed because extensions can set privileges on
* an object during the extension's script file and we record those into * an object during the extension's script file and we record those into
* pg_init_privs as that object's initial privileges. * pg_init_privs as that object's initial privileges.
*
* Returns the dump ID assigned to the ACL TocEntry, or InvalidDumpId if
* no ACL entry was created.
*---------- *----------
*/ */
static void static DumpId
dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId, dumpACL(Archive *fout, DumpId objDumpId, DumpId altDumpId,
const char *type, const char *name, const char *subname, const char *type, const char *name, const char *subname,
const char *nspname, const char *owner, const char *nspname, const char *owner,
const char *acls, const char *racls, const char *acls, const char *racls,
const char *initacls, const char *initracls) const char *initacls, const char *initracls)
{ {
DumpId aclDumpId = InvalidDumpId;
DumpOptions *dopt = fout->dopt; DumpOptions *dopt = fout->dopt;
PQExpBuffer sql; PQExpBuffer sql;
/* Do nothing if ACL dump is not enabled */ /* Do nothing if ACL dump is not enabled */
if (dopt->aclsSkip) if (dopt->aclsSkip)
return; return InvalidDumpId;
/* --data-only skips ACLs *except* BLOB ACLs */ /* --data-only skips ACLs *except* BLOB ACLs */
if (dopt->dataOnly && strcmp(type, "LARGE OBJECT") != 0) if (dopt->dataOnly && strcmp(type, "LARGE OBJECT") != 0)
return; return InvalidDumpId;
sql = createPQExpBuffer(); sql = createPQExpBuffer();
@ -15015,25 +15020,36 @@ dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId,
if (sql->len > 0) if (sql->len > 0)
{ {
PQExpBuffer tag = createPQExpBuffer(); PQExpBuffer tag = createPQExpBuffer();
DumpId aclDeps[2];
int nDeps = 0;
if (subname) if (subname)
appendPQExpBuffer(tag, "COLUMN %s.%s", name, subname); appendPQExpBuffer(tag, "COLUMN %s.%s", name, subname);
else else
appendPQExpBuffer(tag, "%s %s", type, name); appendPQExpBuffer(tag, "%s %s", type, name);
ArchiveEntry(fout, nilCatalogId, createDumpId(), aclDeps[nDeps++] = objDumpId;
if (altDumpId != InvalidDumpId)
aclDeps[nDeps++] = altDumpId;
aclDumpId = createDumpId();
ArchiveEntry(fout, nilCatalogId, aclDumpId,
ARCHIVE_OPTS(.tag = tag->data, ARCHIVE_OPTS(.tag = tag->data,
.namespace = nspname, .namespace = nspname,
.owner = owner, .owner = owner,
.description = "ACL", .description = "ACL",
.section = SECTION_NONE, .section = SECTION_NONE,
.createStmt = sql->data, .createStmt = sql->data,
.deps = &objDumpId, .deps = aclDeps,
.nDeps = 1)); .nDeps = nDeps));
destroyPQExpBuffer(tag); destroyPQExpBuffer(tag);
} }
destroyPQExpBuffer(sql); destroyPQExpBuffer(sql);
return aclDumpId;
} }
/* /*
@ -15359,6 +15375,7 @@ static void
dumpTable(Archive *fout, TableInfo *tbinfo) dumpTable(Archive *fout, TableInfo *tbinfo)
{ {
DumpOptions *dopt = fout->dopt; DumpOptions *dopt = fout->dopt;
DumpId tableAclDumpId = InvalidDumpId;
char *namecopy; char *namecopy;
/* /*
@ -15380,11 +15397,12 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
const char *objtype = const char *objtype =
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" : "TABLE"; (tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" : "TABLE";
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, tableAclDumpId =
objtype, namecopy, NULL, dumpACL(fout, tbinfo->dobj.dumpId, InvalidDumpId,
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname, objtype, namecopy, NULL,
tbinfo->relacl, tbinfo->rrelacl, tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
tbinfo->initrelacl, tbinfo->initrrelacl); tbinfo->relacl, tbinfo->rrelacl,
tbinfo->initrelacl, tbinfo->initrrelacl);
} }
/* /*
@ -15468,8 +15486,13 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
char *attnamecopy; char *attnamecopy;
attnamecopy = pg_strdup(fmtId(attname)); attnamecopy = pg_strdup(fmtId(attname));
/* Column's GRANT type is always TABLE */
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, /*
* Column's GRANT type is always TABLE. Each column ACL depends
* on the table-level ACL, since we can restore column ACLs in
* parallel but the table-level ACL has to be done first.
*/
dumpACL(fout, tbinfo->dobj.dumpId, tableAclDumpId,
"TABLE", namecopy, attnamecopy, "TABLE", namecopy, attnamecopy,
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname, tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
attacl, rattacl, initattacl, initrattacl); attacl, rattacl, initattacl, initrattacl);