]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Postpone calls of unsafe server-side functions in pg_dump.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 19 Nov 2022 16:40:30 +0000 (11:40 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 19 Nov 2022 16:40:30 +0000 (11:40 -0500)
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

src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/pg_dump.h

index 17ad1fe07e6b947626b138b940abb3def6173673..91b26f513676798dcb1f279323ff1d4c83e35fc8 100644 (file)
@@ -6042,14 +6042,15 @@ 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;
        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...)
         *
@@ -6063,8 +6064,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
@@ -6073,9 +6076,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();
@@ -6092,13 +6093,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)";
-               }
 
                /* In PG12 upwards WITH OIDS does not exist anymore. */
                if (fout->remoteVersion >= 120000)
@@ -6136,7 +6132,7 @@ getTables(Archive *fout, int *numTables)
                                                  "tc.relminmxid AS tminmxid, "
                                                  "c.relpersistence, c.relispopulated, "
                                                  "c.relreplident, c.relpages, am.amname, "
-                                                 "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, "
@@ -6156,9 +6152,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 "
@@ -6184,9 +6178,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,
@@ -6226,7 +6218,7 @@ getTables(Archive *fout, int *numTables)
                                                  "c.relpersistence, c.relispopulated, "
                                                  "c.relreplident, c.relpages, "
                                                  "NULL AS amname, "
-                                                 "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, "
@@ -6235,9 +6227,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 "
@@ -6276,7 +6266,7 @@ getTables(Archive *fout, int *numTables)
                                                  "c.relpersistence, c.relispopulated, "
                                                  "c.relreplident, c.relpages, "
                                                  "NULL AS amname, "
-                                                 "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, "
@@ -6285,9 +6275,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 "
@@ -6326,7 +6314,7 @@ getTables(Archive *fout, int *numTables)
                                                  "c.relpersistence, c.relispopulated, "
                                                  "'d' AS relreplident, c.relpages, "
                                                  "NULL AS amname, "
-                                                 "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, "
@@ -6335,9 +6323,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 "
@@ -6376,16 +6362,14 @@ getTables(Archive *fout, int *numTables)
                                                  "c.relpersistence, 't' as relispopulated, "
                                                  "'d' AS relreplident, c.relpages, "
                                                  "NULL AS amname, "
-                                                 "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 "
@@ -6424,16 +6408,14 @@ getTables(Archive *fout, int *numTables)
                                                  "'p' AS relpersistence, 't' as relispopulated, "
                                                  "'d' AS relreplident, c.relpages, "
                                                  "NULL AS amname, "
-                                                 "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 "
@@ -6471,16 +6453,14 @@ getTables(Archive *fout, int *numTables)
                                                  "'p' AS relpersistence, 't' as relispopulated, "
                                                  "'d' AS relreplident, c.relpages, "
                                                  "NULL AS amname, "
-                                                 "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 "
@@ -6518,16 +6498,14 @@ getTables(Archive *fout, int *numTables)
                                                  "'p' AS relpersistence, 't' as relispopulated, "
                                                  "'d' AS relreplident, c.relpages, "
                                                  "NULL AS amname, "
-                                                 "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 "
@@ -6564,16 +6542,14 @@ getTables(Archive *fout, int *numTables)
                                                  "'p' AS relpersistence, 't' as relispopulated, "
                                                  "'d' AS relreplident, relpages, "
                                                  "NULL AS amname, "
-                                                 "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 "
@@ -6640,9 +6616,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");
        i_amname = PQfnumber(res, "amname");
 
        if (dopt->lockWaitTimeout)
@@ -6691,10 +6665,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))
                {
@@ -6750,10 +6721,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
@@ -15816,11 +15785,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();
@@ -15858,6 +15849,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
                                break;
                        default:
                                reltypename = "TABLE";
+                               break;
                }
 
                numParents = tbinfo->numParents;
@@ -15879,8 +15871,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)
                {
@@ -15918,7 +15912,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;
 
@@ -15951,7 +15946,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]);
@@ -16014,7 +16009,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,
@@ -16043,7 +16038,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));
@@ -16227,12 +16222,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));
                        }
                }
 
@@ -16245,16 +16241,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)
                                fatal("invalid number of parents %d for table \"%s\"",
                                          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);
                }
 
                /*
@@ -16414,6 +16428,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
                        }
                }
 
+               if (partkeydef)
+                       free(partkeydef);
                if (ftoptions)
                        free(ftoptions);
                if (srvname)
index 0a29919c13c2f4ed049d65b7a14cf63cfabb1186..6cf02b4a947a38fd0f33ec63968ecb0a5988cd15 100644 (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 */
@@ -325,8 +325,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 */
        char       *amname;                     /* relation access method */