You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by sm...@apache.org on 2026/06/15 10:47:47 UTC
(cloudberry) 26/32: Postpone calls of unsafe server-side functions in pg_dump.
This is an automated email from the ASF dual-hosted git repository.
Smyatkin-Maxim pushed a commit to branch pg-dump-rebase-REL-2
in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 3b1c74ecd9783c63020b860d74228fc53279a9fd
Author: Tom Lane <tg...@sss.pgh.pa.us>
AuthorDate: Mon Dec 6 12:49:49 2021 -0500
Postpone calls of unsafe server-side functions in pg_dump.
Backported from PG15 e3fcbbd
Conflicts resolved:
* OidOptions enum value zeroIsError does not exist, instead used the
existing zeroAsOpaque.
* Minor conflicts in dumpTableSchema resulting from foreign server
related code introduced in PG13 commit 4e62091
Original commit message follows:
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.
Arguably this is a bug fix that should be back-patched, but it's
moderately invasive and we've not had all that many complaints
about such failures. Let's just put it in HEAD for now.
Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc
---
src/bin/pg_dump/pg_dump.c | 109 ++++++++++++++++++++++++++++------------------
src/bin/pg_dump/pg_dump.h | 4 +-
2 files changed, 68 insertions(+), 45 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9a0f83c19e3..43079a9ca98 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7058,24 +7058,25 @@ getTables(Archive *fout, int *numTables)
int i_toastreloptions;
int i_reloftype;
int i_relpages;
+ int i_relstorage;
int i_parrelid;
int i_parlevel;
int i_foreignserver;
int i_is_identity_sequence;
int i_relacl;
int i_acldefault;
- int i_partkeydef;
int i_ispartition;
- int i_partbound;
int i_amname;
int i_amoid;
int i_isivm;
int i_isdynamic;
- int i_relstorage;
/*
* 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...)
*
@@ -7089,8 +7090,10 @@ 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.
*/
appendPQExpBuffer(query,
@@ -7180,10 +7183,10 @@ getTables(Archive *fout, int *numTables)
if (fout->remoteVersion >= 90000)
appendPQExpBufferStr(query,
- "CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS reloftype, ");
+ "c.reloftype, ");
else
appendPQExpBufferStr(query,
- "NULL AS reloftype, ");
+ "0 AS reloftype, ");
if (fout->remoteVersion >= 90600)
appendPQExpBufferStr(query,
@@ -7241,16 +7244,10 @@ getTables(Archive *fout, int *numTables)
if (fout->remoteVersion >= 100000)
appendPQExpBufferStr(query,
- "pg_get_partkeydef(c.oid) AS partkeydef, "
- "c.relispartition AS ispartition, "
- "pg_get_expr(c.relpartbound, c.oid) AS partbound, "
- "NULL as partclause, "
- "NULL as parttemplate");
+ "c.relispartition AS ispartition ");
else
appendPQExpBufferStr(query,
- "NULL AS partkeydef, "
- "0 AS ispartition,"
- "NULL AS partbound ");
+ "false AS ispartition ");
/*
* Left join to pg_depend to pick up dependency info linking sequences to
@@ -7374,15 +7371,13 @@ getTables(Archive *fout, int *numTables)
i_is_identity_sequence = PQfnumber(res, "is_identity_sequence");
i_relacl = PQfnumber(res, "relacl");
i_acldefault = PQfnumber(res, "acldefault");
- i_partkeydef = PQfnumber(res, "partkeydef");
i_ispartition = PQfnumber(res, "ispartition");
- i_partbound = PQfnumber(res, "partbound");
i_amoid = PQfnumber(res, "amoid");
i_isivm = PQfnumber(res, "isivm");
i_isdynamic = PQfnumber(res, "isdynamic");
- i_relstorage = PQfnumber(res, "relstorage");
- i_parrelid = PQfnumber(res, "parrelid");
- i_parlevel = PQfnumber(res, "parlevel");
+ i_relstorage = PQfnumber(res, "relstorage");
+ i_parrelid = PQfnumber(res, "parrelid");
+ i_parlevel = PQfnumber(res, "parlevel");
if (dopt->lockWaitTimeout)
{
@@ -7451,18 +7446,13 @@ getTables(Archive *fout, int *numTables)
tblinfo[i].checkoption = pg_strdup(PQgetvalue(res, i, i_checkoption));
tblinfo[i].toast_reloptions = pg_strdup(PQgetvalue(res, i, i_toastreloptions));
tblinfo[i].parrelid = atooid(PQgetvalue(res, i, i_parrelid));
- 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));
if (PQgetisnull(res, i, i_amname))
tblinfo[i].amname = NULL;
else
tblinfo[i].amname = pg_strdup(PQgetvalue(res, i, i_amname));
tblinfo[i].is_identity_sequence = (strcmp(PQgetvalue(res, i, i_is_identity_sequence), "t") == 0);
- tblinfo[i].partkeydef = pg_strdup(PQgetvalue(res, i, i_partkeydef));
tblinfo[i].ispartition = (strcmp(PQgetvalue(res, i, i_ispartition), "t") == 0);
- tblinfo[i].partbound = pg_strdup(PQgetvalue(res, i, i_partbound));
tblinfo[i].relstorage = *(PQgetvalue(res, i, i_relstorage));
@@ -7523,10 +7513,6 @@ 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));
- tblinfo[i].ispartition = (strcmp(PQgetvalue(res, i, i_ispartition), "t") == 0);
- tblinfo[i].partbound = pg_strdup(PQgetvalue(res, i, i_partbound));
tblinfo[i].isivm = (strcmp(PQgetvalue(res, i, i_isivm), "t") == 0);
tblinfo[i].isdynamic = (strcmp(PQgetvalue(res, i, i_isdynamic), "t") == 0);
@@ -17534,7 +17520,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
char *ftoptions = NULL;
char *srvname = NULL;
char *foreign = "";
-
+ char *partkeydef = NULL;
/* We had better have loaded per-column details about this table */
Assert(tbinfo->interesting);
@@ -17589,8 +17575,29 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
}
else
{
+ /*
+ * 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();
@@ -17633,10 +17640,10 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
break;
default:
reltypename = "TABLE";
-
/* Is it an external table (server GPDB 6.x and below.) */
if (tbinfo->relstorage == RELSTORAGE_EXTERNAL)
reltypename = "EXTERNAL TABLE";
+ break;
}
}
@@ -17726,8 +17733,10 @@ dumpTableSchema(Archive *fout, const 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,
+ zeroIsError));
if (tbinfo->relkind != RELKIND_MATVIEW)
{
@@ -17765,7 +17774,8 @@ dumpTableSchema(Archive *fout, const 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;
@@ -17806,7 +17816,7 @@ dumpTableSchema(Archive *fout, const 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]);
@@ -17896,7 +17906,7 @@ dumpTableSchema(Archive *fout, const 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,
@@ -17925,7 +17935,7 @@ dumpTableSchema(Archive *fout, const 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));
@@ -18282,12 +18292,13 @@ dumpTableSchema(Archive *fout, const 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,
+ zeroIsError));
}
appendPQExpBuffer(q, "RESET allow_system_table_mods;\n");
}
@@ -18476,6 +18487,8 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
tbinfo->attfdwoptions[j]);
} /* end loop over columns */
+ if (partkeydef)
+ free(partkeydef);
if (ftoptions)
free(ftoptions);
if (srvname)
@@ -18583,6 +18596,8 @@ dumpTableAttach(Archive *fout, const TableAttachInfo *attachinfo)
{
DumpOptions *dopt = fout->dopt;
PQExpBuffer q;
+ PGresult *res;
+ char *partbound;
/* Do nothing in data-only dump */
if (dopt->dataOnly)
@@ -18593,14 +18608,23 @@ dumpTableAttach(Archive *fout, const TableAttachInfo *attachinfo)
q = createPQExpBuffer();
- /* Perform ALTER TABLE on the parent */
+ /* Fetch the partition's partbound */
appendPQExpBuffer(q,
+ "SELECT pg_get_expr(c.relpartbound, c.oid) "
+ "FROM pg_class c "
+ "WHERE c.oid = '%u'",
+ attachinfo->partitionTbl->dobj.catId.oid);
+ res = ExecuteSqlQueryForSingleRow(fout, q->data);
+ partbound = PQgetvalue(res, 0, 0);
+
+ /* Perform ALTER TABLE on the parent */
+ printfPQExpBuffer(q,
"ALTER TABLE ONLY %s ",
fmtQualifiedDumpable(attachinfo->parentTbl));
appendPQExpBuffer(q,
"ATTACH PARTITION %s %s;\n",
fmtQualifiedDumpable(attachinfo->partitionTbl),
- attachinfo->partitionTbl->partbound);
+ partbound);
/*
* There is no point in creating a drop query as the drop is done by table
@@ -18617,6 +18641,7 @@ dumpTableAttach(Archive *fout, const TableAttachInfo *attachinfo)
.section = SECTION_PRE_DATA,
.createStmt = q->data));
+ PQclear(res);
destroyPQExpBuffer(q);
}
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index f26d764d5b6..d53d2ab623e 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -335,7 +335,7 @@ typedef struct _tableInfo
uint32 toast_minmxid; /* toast table's relminmxid */
int ncheck; /* # of CHECK expressions */
Oid reltype; /* OID of table's composite type, if any */
- char *reloftype; /* underlying type for typed table */
+ Oid reloftype; /* underlying type for typed table */
Oid foreign_server; /* foreign server oid, if applicable */
/* these two are set only if table is a sequence owned by a column: */
Oid owning_tab; /* OID of table owning sequence */
@@ -376,8 +376,6 @@ typedef struct _tableInfo
char **attencoding; /* the attribute encoding values */
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 */
char *amname; /* relation access method */
Oid amoid; /* relation access method oid */
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org