upstream/postgresql
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-04-21 06:08:26 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Postpone calls of unsafe server-side functions in pg_dump.

Browse source 
Avoid calling pg_get_partkeydef(), pg_get_expr(relpartbound),
and regtypeout until we have lock on the relevant tables.
The existing coding is at serious risk of failure if there
are any concurrent DROP TABLE commands going on --- including
drops of other sessions' temp tables.

Back-patch of commit e3fcbbd62.  That's been in v15/HEAD long enough
to have some confidence about it, so now let's fix the problem in
older branches.

Original patch by me; thanks to Gilles Darold for back-patching
legwork.

Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc
Discussion: https://postgr.es/m/45c93d57-9973-248e-d2df-e02ca9af48d4@darold.net
This commit is contained in:
Tom Lane 2022-11-19 11:40:30 -05:00
parent d4acf2eb94
commit b1f106420b
2 changed files with 93 additions and 77 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

166
src/bin/pg_dump/pg_dump.c
View file

@ -5965,13 +5965,14 @@ getTables(Archive *fout, int *numTables)
int i_relpages;
int i_is_identity_sequence;
int i_changed_acl;
int i_partkeydef;
int i_ispartition;
int i_partbound;
/*
* Find all the tables and table-like objects.
*
* We must fetch all tables in this phase because otherwise we cannot
* correctly identify inherited columns, owned sequences, etc.
*
* We include system catalogs, so that we can work if a user table is
* defined to inherit from a system catalog (pretty weird, but...)
*
@ -5985,15 +5986,15 @@ getTables(Archive *fout, int *numTables)
*
* Note: in this phase we should collect only a minimal amount of
* information about each table, basically just enough to decide if it is
* interesting. We must fetch all tables in this phase because otherwise
* we cannot correctly identify inherited columns, owned sequences, etc.
* interesting. In particular, since we do not yet have lock on any user
* table, we MUST NOT invoke any server-side data collection functions
* (for instance, pg_get_partkeydef()). Those are likely to fail or give
* wrong answers if any concurrent DDL is happening.
*/
if (fout->remoteVersion >= 90600)
{
char *partkeydef = "NULL";
char *ispartition = "false";
char *partbound = "NULL";
PQExpBuffer acl_subquery = createPQExpBuffer();
PQExpBuffer racl_subquery = createPQExpBuffer();
@ -6009,13 +6010,8 @@ getTables(Archive *fout, int *numTables)
* Collect the information about any partitioned tables, which were
* added in PG10.
*/
if (fout->remoteVersion >= 100000)
{
partkeydef = "pg_get_partkeydef(c.oid)";
ispartition = "c.relispartition";
partbound = "pg_get_expr(c.relpartbound, c.oid)";
}
/*
* Left join to pick up dependency info linking sequences to their
@ -6049,7 +6045,7 @@ getTables(Archive *fout, int *numTables)
"tc.relminmxid AS tminmxid, "
"c.relpersistence, c.relispopulated, "
"c.relreplident, c.relpages, "
"CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS reloftype, "
"c.reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
@ -6069,9 +6065,7 @@ getTables(Archive *fout, int *numTables)
"OR %s IS NOT NULL"
"))"
"AS changed_acl, "
"%s AS partkeydef, "
"%s AS ispartition, "
"%s AS partbound "
"%s AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6095,9 +6089,7 @@ getTables(Archive *fout, int *numTables)
attracl_subquery->data,
attinitacl_subquery->data,
attinitracl_subquery->data,
partkeydef,
ispartition,
partbound,
RELKIND_SEQUENCE,
RELKIND_RELATION, RELKIND_SEQUENCE,
RELKIND_VIEW, RELKIND_COMPOSITE_TYPE,
@ -6135,7 +6127,7 @@ getTables(Archive *fout, int *numTables)
"tc.relminmxid AS tminmxid, "
"c.relpersistence, c.relispopulated, "
"c.relreplident, c.relpages, "
"CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS reloftype, "
"c.reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
@ -6144,9 +6136,7 @@ getTables(Archive *fout, int *numTables)
"WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
"tc.reloptions AS toast_reloptions, "
"NULL AS changed_acl, "
"NULL AS partkeydef, "
"false AS ispartition, "
"NULL AS partbound "
"false AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6184,7 +6174,7 @@ getTables(Archive *fout, int *numTables)
"tc.relminmxid AS tminmxid, "
"c.relpersistence, c.relispopulated, "
"c.relreplident, c.relpages, "
"CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS reloftype, "
"c.reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
@ -6193,9 +6183,7 @@ getTables(Archive *fout, int *numTables)
"WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
"tc.reloptions AS toast_reloptions, "
"NULL AS changed_acl, "
"NULL AS partkeydef, "
"false AS ispartition, "
"NULL AS partbound "
"false AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6233,7 +6221,7 @@ getTables(Archive *fout, int *numTables)
"tc.relminmxid AS tminmxid, "
"c.relpersistence, c.relispopulated, "
"'d' AS relreplident, c.relpages, "
"CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS reloftype, "
"c.reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
@ -6242,9 +6230,7 @@ getTables(Archive *fout, int *numTables)
"WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
"tc.reloptions AS toast_reloptions, "
"NULL AS changed_acl, "
"NULL AS partkeydef, "
"false AS ispartition, "
"NULL AS partbound "
"false AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6282,16 +6268,14 @@ getTables(Archive *fout, int *numTables)
"0 AS tminmxid, "
"c.relpersistence, 't' as relispopulated, "
"'d' AS relreplident, c.relpages, "
"CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS reloftype, "
"c.reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
"c.reloptions AS reloptions, "
"tc.reloptions AS toast_reloptions, "
"NULL AS changed_acl, "
"NULL AS partkeydef, "
"false AS ispartition, "
"NULL AS partbound "
"false AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6329,16 +6313,14 @@ getTables(Archive *fout, int *numTables)
"0 AS tminmxid, "
"'p' AS relpersistence, 't' as relispopulated, "
"'d' AS relreplident, c.relpages, "
"CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS reloftype, "
"c.reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
"c.reloptions AS reloptions, "
"tc.reloptions AS toast_reloptions, "
"NULL AS changed_acl, "
"NULL AS partkeydef, "
"false AS ispartition, "
"NULL AS partbound "
"false AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6375,16 +6357,14 @@ getTables(Archive *fout, int *numTables)
"0 AS tminmxid, "
"'p' AS relpersistence, 't' as relispopulated, "
"'d' AS relreplident, c.relpages, "
"NULL AS reloftype, "
"0 AS reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
"c.reloptions AS reloptions, "
"tc.reloptions AS toast_reloptions, "
"NULL AS changed_acl, "
"NULL AS partkeydef, "
"false AS ispartition, "
"NULL AS partbound "
"false AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6421,16 +6401,14 @@ getTables(Archive *fout, int *numTables)
"0 AS tminmxid, "
"'p' AS relpersistence, 't' as relispopulated, "
"'d' AS relreplident, c.relpages, "
"NULL AS reloftype, "
"0 AS reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
"c.reloptions AS reloptions, "
"NULL AS toast_reloptions, "
"NULL AS changed_acl, "
"NULL AS partkeydef, "
"false AS ispartition, "
"NULL AS partbound "
"false AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6466,16 +6444,14 @@ getTables(Archive *fout, int *numTables)
"0 AS tfrozenxid, 0 AS tminmxid,"
"'p' AS relpersistence, 't' as relispopulated, "
"'d' AS relreplident, relpages, "
"NULL AS reloftype, "
"0 AS reloftype, "
"d.refobjid AS owning_tab, "
"d.refobjsubid AS owning_col, "
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
"NULL AS reloptions, "
"NULL AS toast_reloptions, "
"NULL AS changed_acl, "
"NULL AS partkeydef, "
"false AS ispartition, "
"NULL AS partbound "
"false AS ispartition "
"FROM pg_class c "
"LEFT JOIN pg_depend d ON "
"(c.relkind = '%c' AND "
@ -6542,9 +6518,7 @@ getTables(Archive *fout, int *numTables)
i_reloftype = PQfnumber(res, "reloftype");
i_is_identity_sequence = PQfnumber(res, "is_identity_sequence");
i_changed_acl = PQfnumber(res, "changed_acl");
i_partkeydef = PQfnumber(res, "partkeydef");
i_ispartition = PQfnumber(res, "ispartition");
i_partbound = PQfnumber(res, "partbound");
if (dopt->lockWaitTimeout)
{
@ -6592,10 +6566,7 @@ getTables(Archive *fout, int *numTables)
tblinfo[i].toast_oid = atooid(PQgetvalue(res, i, i_toastoid));
tblinfo[i].toast_frozenxid = atooid(PQgetvalue(res, i, i_toastfrozenxid));
tblinfo[i].toast_minmxid = atooid(PQgetvalue(res, i, i_toastminmxid));
if (PQgetisnull(res, i, i_reloftype))
tblinfo[i].reloftype = NULL;
else
tblinfo[i].reloftype = pg_strdup(PQgetvalue(res, i, i_reloftype));
tblinfo[i].reloftype = atooid(PQgetvalue(res, i, i_reloftype));
tblinfo[i].ncheck = atoi(PQgetvalue(res, i, i_relchecks));
if (PQgetisnull(res, i, i_owning_tab))
{
@ -6647,10 +6618,8 @@ getTables(Archive *fout, int *numTables)
tblinfo[i].is_identity_sequence = (i_is_identity_sequence >= 0 &&
strcmp(PQgetvalue(res, i, i_is_identity_sequence), "t") == 0);
/* Partition key string or NULL */
tblinfo[i].partkeydef = pg_strdup(PQgetvalue(res, i, i_partkeydef));
/* Partition? */
tblinfo[i].ispartition = (strcmp(PQgetvalue(res, i, i_ispartition), "t") == 0);
tblinfo[i].partbound = pg_strdup(PQgetvalue(res, i, i_partbound));
/*
* Read-lock target tables to make sure they aren't DROPPED or altered
@ -15596,8 +15565,6 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
int actual_atts; /* number of attrs in this CREATE statement */
const char *reltypename;
char *storage;
char *srvname;
char *ftoptions;
int j,
k;
@ -15651,8 +15618,33 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
else
{
char *partkeydef = NULL;
char *ftoptions = NULL;
char *srvname = NULL;
/*
* Set reltypename, and collect any relkind-specific data that we
* didn't fetch during getTables().
*/
switch (tbinfo->relkind)
{
case RELKIND_PARTITIONED_TABLE:
{
PQExpBuffer query = createPQExpBuffer();
PGresult *res;
reltypename = "TABLE";
/* retrieve partition key definition */
appendPQExpBuffer(query,
"SELECT pg_get_partkeydef('%u')",
tbinfo->dobj.catId.oid);
res = ExecuteSqlQueryForSingleRow(fout, query->data);
partkeydef = pg_strdup(PQgetvalue(res, 0, 0));
PQclear(res);
destroyPQExpBuffer(query);
break;
}
case RELKIND_FOREIGN_TABLE:
{
PQExpBuffer query = createPQExpBuffer();
@ -15687,13 +15679,10 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
case RELKIND_MATVIEW:
reltypename = "MATERIALIZED VIEW";
srvname = NULL;
ftoptions = NULL;
break;
default:
reltypename = "TABLE";
srvname = NULL;
ftoptions = NULL;
break;
}
numParents = tbinfo->numParents;
@ -15715,8 +15704,10 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* Attach to type, if reloftype; except in case of a binary upgrade,
* we dump the table normally and attach it to the type afterward.
*/
if (tbinfo->reloftype && !dopt->binary_upgrade)
appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
if (OidIsValid(tbinfo->reloftype) && !dopt->binary_upgrade)
appendPQExpBuffer(q, " OF %s",
getFormattedTypeName(fout, tbinfo->reloftype,
zeroAsOpaque));
if (tbinfo->relkind != RELKIND_MATVIEW)
{
@ -15754,7 +15745,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* Skip column if fully defined by reloftype, except in
* binary upgrade
*/
if (tbinfo->reloftype && !print_default && !print_notnull &&
if (OidIsValid(tbinfo->reloftype) &&
!print_default && !print_notnull &&
!dopt->binary_upgrade)
continue;
@ -15787,7 +15779,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* table ('OF type_name'), but in binary-upgrade mode,
* print it in that case too.
*/
if (dopt->binary_upgrade || !tbinfo->reloftype)
if (dopt->binary_upgrade || !OidIsValid(tbinfo->reloftype))
{
appendPQExpBuffer(q, " %s",
tbinfo->atttypnames[j]);
@ -15843,7 +15835,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (actual_atts)
appendPQExpBufferStr(q, "\n)");
else if (!(tbinfo->reloftype && !dopt->binary_upgrade))
else if (!(OidIsValid(tbinfo->reloftype) && !dopt->binary_upgrade))
{
/*
* No attributes? we must have a parenthesized attribute list,
@ -15872,7 +15864,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
if (tbinfo->relkind == RELKIND_PARTITIONED_TABLE)
appendPQExpBuffer(q, "\nPARTITION BY %s", tbinfo->partkeydef);
appendPQExpBuffer(q, "\nPARTITION BY %s", partkeydef);
if (tbinfo->relkind == RELKIND_FOREIGN_TABLE)
appendPQExpBuffer(q, "\nSERVER %s", fmtId(srvname));
@ -16056,12 +16048,13 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
}
if (tbinfo->reloftype)
if (OidIsValid(tbinfo->reloftype))
{
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up typed tables this way.\n");
appendPQExpBuffer(q, "ALTER TABLE ONLY %s OF %s;\n",
qualrelname,
tbinfo->reloftype);
getFormattedTypeName(fout, tbinfo->reloftype,
zeroAsOpaque));
}
}
@ -16074,16 +16067,34 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
*/
if (tbinfo->ispartition)
{
PGresult *ares;
char *partbound;
PQExpBuffer q2;
/* With partitions there can only be one parent */
if (tbinfo->numParents != 1)
exit_horribly(NULL, "invalid number of parents %d for table \"%s\"\n",
tbinfo->numParents, tbinfo->dobj.name);
tbinfo->numParents, tbinfo->dobj.name);
q2 = createPQExpBuffer();
/* Fetch the partition's partbound */
appendPQExpBuffer(q2,
"SELECT pg_get_expr(c.relpartbound, c.oid) "
"FROM pg_class c "
"WHERE c.oid = '%u'",
tbinfo->dobj.catId.oid);
ares = ExecuteSqlQueryForSingleRow(fout, q2->data);
partbound = PQgetvalue(ares, 0, 0);
/* Perform ALTER TABLE on the parent */
appendPQExpBuffer(q,
"ALTER TABLE ONLY %s ATTACH PARTITION %s %s;\n",
fmtQualifiedDumpable(parents[0]),
qualrelname, tbinfo->partbound);
qualrelname, partbound);
PQclear(ares);
destroyPQExpBuffer(q2);
}
/*
@ -16243,6 +16254,13 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
tbinfo->attfdwoptions[j]);
}
}
if (partkeydef)
free(partkeydef);
if (ftoptions)
free(ftoptions);
if (srvname)
free(srvname);
}
/*

4
src/bin/pg_dump/pg_dump.h
View file

@ -288,7 +288,7 @@ typedef struct _tableInfo
uint32 toast_frozenxid; /* toast table's relfrozenxid, if any */
uint32 toast_minmxid; /* toast table's relminmxid */
int ncheck; /* # of CHECK expressions */
char *reloftype; /* underlying type for typed table */
Oid reloftype; /* underlying type for typed table */
/* these two are set only if table is a sequence owned by a column: */
Oid owning_tab; /* OID of table owning sequence */
int owning_col; /* attr # of column owning sequence */
@ -324,8 +324,6 @@ typedef struct _tableInfo
bool *inhNotNull; /* true if NOT NULL is inherited */
struct _attrDefInfo **attrdefs; /* DEFAULT expressions */
struct _constraintInfo *checkexprs; /* CHECK constraints */
char *partkeydef; /* partition key definition */
char *partbound; /* partition bound definition */
bool needs_override; /* has GENERATED ALWAYS AS IDENTITY */
/*