From 0c66c9f1211e16366cf471ef48a52e5447c79424 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 14 Feb 2023 13:09:04 -0500 Subject: [PATCH v1 1/3] Force load-via-partition-root for hash partitioning on an enum column. Without this, dump and restore will almost always fail because the enum values will receive different OIDs in the destination database, and their hash codes will therefore be different. (Improving the hash algorithm would not make this situation better, and would indeed break other cases as well.) Discussion: https://postgr.es/m/1376149.1675268279@sss.pgh.pa.us --- src/bin/pg_dump/common.c | 18 +++--- src/bin/pg_dump/pg_dump.c | 120 +++++++++++++++++++++++++++++++++++--- src/bin/pg_dump/pg_dump.h | 2 + 3 files changed, 124 insertions(+), 16 deletions(-) diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index a43f2e5553..3d0cfc1b8e 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -230,6 +230,9 @@ getSchemaData(Archive *fout, int *numTablesPtr) pg_log_info("flagging inherited columns in subtables"); flagInhAttrs(fout->dopt, tblinfo, numTables); + pg_log_info("reading partitioning data"); + getPartitioningInfo(fout); + pg_log_info("reading indexes"); getIndexes(fout, tblinfo, numTables); @@ -285,7 +288,6 @@ static void flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables, InhInfo *inhinfo, int numInherits) { - DumpOptions *dopt = fout->dopt; int i, j; @@ -301,18 +303,18 @@ flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables, continue; /* - * Normally, we don't bother computing anything for non-target tables, - * but if load-via-partition-root is specified, we gather information - * on every partition in the system so that getRootTableInfo can trace - * from any given to leaf partition all the way up to the root. (We - * don't need to mark them as interesting for getTableAttrs, though.) + * Normally, we don't bother computing anything for non-target tables. + * However, we must find the parents of non-root partitioned tables in + * any case, so that we can trace from leaf partitions up to the root + * (in case a leaf is to be dumped but its parents are not). We need + * not mark such parents interesting for getTableAttrs, though. */ if (!tblinfo[i].dobj.dump) { mark_parents = false; - if (!dopt->load_via_partition_root || - !tblinfo[i].ispartition) + if (!(tblinfo[i].relkind == RELKIND_PARTITIONED_TABLE && + tblinfo[i].ispartition)) find_parents = false; } diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 527c7651ab..51b317aa3d 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -320,6 +320,7 @@ static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions, static char *get_synchronized_snapshot(Archive *fout); static void setupDumpWorker(Archive *AH); static TableInfo *getRootTableInfo(const TableInfo *tbinfo); +static bool forcePartitionRootLoad(const TableInfo *tbinfo); int @@ -2228,11 +2229,13 @@ dumpTableData_insert(Archive *fout, const void *dcontext) insertStmt = createPQExpBuffer(); /* - * When load-via-partition-root is set, get the root table name - * for the partition table, so that we can reload data through the - * root table. + * When load-via-partition-root is set or forced, get the root + * table name for the partition table, so that we can reload data + * through the root table. */ - if (dopt->load_via_partition_root && tbinfo->ispartition) + if (tbinfo->ispartition && + (dopt->load_via_partition_root || + forcePartitionRootLoad(tbinfo))) targettab = getRootTableInfo(tbinfo); else targettab = tbinfo; @@ -2430,6 +2433,35 @@ getRootTableInfo(const TableInfo *tbinfo) return parentTbinfo; } +/* + * forcePartitionRootLoad + * Check if we must force load_via_partition_root for this partition. + * + * This is required if any level of ancestral partitioned table has an + * unsafe partitioning scheme. + */ +static bool +forcePartitionRootLoad(const TableInfo *tbinfo) +{ + TableInfo *parentTbinfo; + + Assert(tbinfo->ispartition); + Assert(tbinfo->numParents == 1); + + parentTbinfo = tbinfo->parents[0]; + if (parentTbinfo->unsafe_partitions) + return true; + while (parentTbinfo->ispartition) + { + Assert(parentTbinfo->numParents == 1); + parentTbinfo = parentTbinfo->parents[0]; + if (parentTbinfo->unsafe_partitions) + return true; + } + + return false; +} + /* * dumpTableData - * dump the contents of a single table @@ -2456,11 +2488,13 @@ dumpTableData(Archive *fout, const TableDataInfo *tdinfo) dumpFn = dumpTableData_copy; /* - * When load-via-partition-root is set, get the root table name for - * the partition table, so that we can reload data through the root - * table. + * When load-via-partition-root is set or forced, get the root table + * name for the partition table, so that we can reload data through + * the root table. */ - if (dopt->load_via_partition_root && tbinfo->ispartition) + if (tbinfo->ispartition && + (dopt->load_via_partition_root || + forcePartitionRootLoad(tbinfo))) { TableInfo *parentTbinfo; @@ -6744,6 +6778,76 @@ getInherits(Archive *fout, int *numInherits) return inhinfo; } +/* + * getPartitioningInfo + * get information about partitioning + * + * For the most part, we only collect partitioning info about tables we + * intend to dump. However, this function has to consider all partitioned + * tables in the database, because we need to know about parents of partitions + * we are going to dump even if the parents themselves won't be dumped. + * + * Specifically, what we need to know is whether each partitioned table + * has an "unsafe" partitioning scheme that requires us to force + * load-via-partition-root mode for its children. Currently the only case + * for which we force that is hash partitioning on enum columns, since the + * hash codes depend on enum value OIDs which won't be replicated across + * dump-and-reload. There are other cases in which load-via-partition-root + * might be necessary, but we expect users to cope with them. + */ +void +getPartitioningInfo(Archive *fout) +{ + PQExpBuffer query; + PGresult *res; + int ntups; + + /* no partitions before v10 */ + if (fout->remoteVersion < 100000) + return; + /* needn't bother if schema-only dump */ + if (fout->dopt->schemaOnly) + return; + + query = createPQExpBuffer(); + + /* + * Unsafe partitioning schemes are exactly those for which hash enum_ops + * appears among the partition opclasses. We needn't check partstrat. + * + * Note that this query may well retrieve info about tables we aren't + * going to dump and hence have no lock on. That's okay since we need not + * invoke any unsafe server-side functions. + */ + appendPQExpBufferStr(query, + "SELECT partrelid FROM pg_partitioned_table WHERE\n" + "(SELECT c.oid FROM pg_opclass c JOIN pg_am a " + "ON c.opcmethod = a.oid\n" + "WHERE opcname = 'enum_ops' " + "AND opcnamespace = 'pg_catalog'::regnamespace " + "AND amname = 'hash') = ANY(partclass)"); + + res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); + + ntups = PQntuples(res); + + for (int i = 0; i < ntups; i++) + { + Oid tabrelid = atooid(PQgetvalue(res, i, 0)); + TableInfo *tbinfo; + + tbinfo = findTableByOid(tabrelid); + if (tbinfo == NULL) + pg_fatal("failed sanity check, table OID %u appearing in pg_partitioned_table not found", + tabrelid); + tbinfo->unsafe_partitions = true; + } + + PQclear(res); + + destroyPQExpBuffer(query); +} + /* * getIndexes * get information about every index on a dumpable table diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index e7cbd8d7ed..bfd3cf17b7 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -318,6 +318,7 @@ typedef struct _tableInfo bool dummy_view; /* view's real definition must be postponed */ bool postponed_def; /* matview must be postponed into post-data */ bool ispartition; /* is table a partition? */ + bool unsafe_partitions; /* is it an unsafe partitioned table? */ /* * These fields are computed only if we decide the table is interesting @@ -715,6 +716,7 @@ extern ConvInfo *getConversions(Archive *fout, int *numConversions); extern TableInfo *getTables(Archive *fout, int *numTables); extern void getOwnedSeqs(Archive *fout, TableInfo tblinfo[], int numTables); extern InhInfo *getInherits(Archive *fout, int *numInherits); +extern void getPartitioningInfo(Archive *fout); extern void getIndexes(Archive *fout, TableInfo tblinfo[], int numTables); extern void getExtendedStatistics(Archive *fout); extern void getConstraints(Archive *fout, TableInfo tblinfo[], int numTables); -- 2.31.1