Le 17/11/2022 à 17:59, Tom Lane a écrit :
Gilles Darold writes:
I have an incorrect behavior with pg_dump prior PG version 15. With
PostgreSQL 15, thanks to commit e3fcbbd623b9ccc16cdbda374654d91a4727d173
the problem is gone but for older versions it persists with locks on
partitioned tables.
I didn't want to back-patch e3fcbbd62 at the time, but it's probably aged
long enough now to be safe to back-patch. If we do anything here,
it should be to back-patch the whole thing, else we've only partially
fixed the issue.
Here are the different patched following the PostgreSQL version from 11
to 14, they should apply on the corresponding stable branches. The
patches only concern the move of the unsafe functions,
pg_get_partkeydef() and pg_get_expr(). They should all apply without
problem on their respective branch, pg_dump tap regression test passed
on all versions.
Regards,
--
Gilles Darold
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e110257395..cbe7c1e01d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6322,14 +6322,15 @@ getTables(Archive *fout, int *numTables)
int i_foreignserver;
int i_is_identity_sequence;
int i_changed_acl;
- int i_partkeydef;
int i_ispartition;
- int i_partbound;
int i_amname;
/*
* 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...)
*
@@ -6343,8 +6344,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.
*
* We purposefully ignore toast OIDs for partitioned tables; the reason is
* that versions 10 and 11 have them, but 12 does not, so emitting them
@@ -6353,9 +6356,7 @@ getTables(Archive *fout, int *numTables)
if (fout->remoteVersion >= 90600)
{
- char *partkeydef = "NULL";
char *ispartition = "false";
- char *partbound = "NULL";
char *relhasoids = "c.relhasoids";
PQExpBuffer acl_subquery = createPQExpBuffer();
@@ -6374,11 +6375,7 @@ getTables(Archive *fout, int *numTables)
*/
if (fout->remoteVersion >= 10)
- {
- partkeydef = "pg_get_partkeydef(c.oid)";
ispartition = "c.relispartition";
- partbound = "pg_get_expr(c.relpartbound, c.oid)";
- }
/* In PG12 upwards WITH OIDS does not exist anymore. */
if (fout->remoteVersion >= 12)
@@ -6419,7 +6416,7 @@ getTables(Archive *fout, int *numTables)
"CASE WHEN c.relkind = 'f' THEN "
"(SELECT ftserver FROM pg_catalog.pg_foreign_table WHERE ftrelid = c.oid) "
"ELSE 0 END AS foreignserver, "
- "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, "
@@ -6439,9 +6436,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 "
@@ -6467,9 +6462,7 @@ getTables(Archive *fout, int *numTables)
attracl_subquery->data,
attinitacl_subquery->data,
attinitracl_subquery->data,
- partkeydef,
ispartition,
- partbound,
RELKIND_SEQUENCE,
RELKIND_PARTITIONED_TABLE,
RELKIND_RELATION, RELKIND_SEQUENCE,
@@ -6512,7 +6505,7 @@ getTables(Archive *fout, int *numTables)
"CASE WHEN c.relkind = 'f' THEN "
"(SELECT ftserver FROM pg_catalog.pg_foreign_table WHERE ftrelid = c.oid) "
"ELSE 0 END AS foreignserver, "
- "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, "
@@ -6521,9 +6514,7 @@ getTables(Archive *fout, int *numTables)
"WHEN 'check_option=cascaded' = ANY
