From 3f96af4619c8b129ec8d5f4fb961df4310999383 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 7 Jul 2020 18:10:42 -0400 Subject: [PATCH] Un-break pg_upgrade from pre-v12 servers. I neglected to test this scenario while preparing commit f3faf35f3, so of course it was broken, thanks to some very obscure and undocumented code in pg_dump. Pre-v12 databases might have toast tables attached to partitioned tables, which we need to ignore since newer servers never create such useless toast tables. There was a filter for this case in binary_upgrade_set_type_oids_by_rel_oid(), which appeared to just prevent the pg_type OID from being copied. But actually it managed to prevent the toast table from being created at all --- or it did before I took out that logic. But that was a fundamentally bizarre place to be making the test in the first place. The place where the filter should have been, one would think, is binary_upgrade_set_pg_class_oids(), so add it there. While at it, reorganize binary_upgrade_set_pg_class_oids() so that it doesn't make a completely useless query when it knows it's being invoked for an index. And correct a comment that mis-described the scenario where we need to force creation of a TOAST table. Per buildfarm. --- src/bin/pg_dump/pg_dump.c | 75 +++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index fd7b3e0920..c2627bb630 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -4525,43 +4525,56 @@ binary_upgrade_set_pg_class_oids(Archive *fout, PQExpBuffer upgrade_buffer, Oid pg_class_oid, bool is_index) { - PQExpBuffer upgrade_query = createPQExpBuffer(); - PGresult *upgrade_res; - Oid pg_class_reltoastrelid; - Oid pg_index_indexrelid; - - appendPQExpBuffer(upgrade_query, - "SELECT c.reltoastrelid, i.indexrelid " - "FROM pg_catalog.pg_class c LEFT JOIN " - "pg_catalog.pg_index i ON (c.reltoastrelid = i.indrelid AND i.indisvalid) " - "WHERE c.oid = '%u'::pg_catalog.oid;", - pg_class_oid); - - upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data); - - pg_class_reltoastrelid = atooid(PQgetvalue(upgrade_res, 0, PQfnumber(upgrade_res, "reltoastrelid"))); - pg_index_indexrelid = atooid(PQgetvalue(upgrade_res, 0, PQfnumber(upgrade_res, "indexrelid"))); - appendPQExpBufferStr(upgrade_buffer, "\n-- For binary upgrade, must preserve pg_class oids\n"); if (!is_index) { + PQExpBuffer upgrade_query = createPQExpBuffer(); + PGresult *upgrade_res; + Oid pg_class_reltoastrelid; + char pg_class_relkind; + Oid pg_index_indexrelid; + appendPQExpBuffer(upgrade_buffer, "SELECT pg_catalog.binary_upgrade_set_next_heap_pg_class_oid('%u'::pg_catalog.oid);\n", pg_class_oid); - /* only tables have toast tables, not indexes */ - if (OidIsValid(pg_class_reltoastrelid)) - { - /* - * One complexity is that the table definition might not require - * the creation of a TOAST table, and the TOAST table might have - * been created long after table creation, when the table was - * loaded with wide data. By setting the TOAST oid we force - * creation of the TOAST heap and TOAST index by the backend so we - * can cleanly copy the files during binary upgrade. - */ + /* + * Preserve the OIDs of the table's toast table and index, if any. + * Indexes cannot have toast tables, so we need not make this probe in + * the index code path. + * + * One complexity is that the current table definition might not + * require the creation of a TOAST table, but the old database might + * have a TOAST table that was created earlier, before some wide + * columns were dropped. By setting the TOAST oid we force creation + * of the TOAST heap and index by the new backend, so we can copy the + * files during binary upgrade without worrying about this case. + */ + appendPQExpBuffer(upgrade_query, + "SELECT c.reltoastrelid, c.relkind, i.indexrelid " + "FROM pg_catalog.pg_class c LEFT JOIN " + "pg_catalog.pg_index i ON (c.reltoastrelid = i.indrelid AND i.indisvalid) " + "WHERE c.oid = '%u'::pg_catalog.oid;", + pg_class_oid); + + upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data); + + pg_class_reltoastrelid = atooid(PQgetvalue(upgrade_res, 0, + PQfnumber(upgrade_res, "reltoastrelid"))); + pg_class_relkind = *PQgetvalue(upgrade_res, 0, + PQfnumber(upgrade_res, "relkind")); + pg_index_indexrelid = atooid(PQgetvalue(upgrade_res, 0, + PQfnumber(upgrade_res, "indexrelid"))); + + /* + * In a pre-v12 database, partitioned tables might be marked as having + * toast tables, but we should ignore them if so. + */ + if (OidIsValid(pg_class_reltoastrelid) && + pg_class_relkind != RELKIND_PARTITIONED_TABLE) + { appendPQExpBuffer(upgrade_buffer, "SELECT pg_catalog.binary_upgrade_set_next_toast_pg_class_oid('%u'::pg_catalog.oid);\n", pg_class_reltoastrelid); @@ -4571,6 +4584,9 @@ binary_upgrade_set_pg_class_oids(Archive *fout, "SELECT pg_catalog.binary_upgrade_set_next_index_pg_class_oid('%u'::pg_catalog.oid);\n", pg_index_indexrelid); } + + PQclear(upgrade_res); + destroyPQExpBuffer(upgrade_query); } else appendPQExpBuffer(upgrade_buffer, @@ -4578,9 +4594,6 @@ binary_upgrade_set_pg_class_oids(Archive *fout, pg_class_oid); appendPQExpBufferChar(upgrade_buffer, '\n'); - - PQclear(upgrade_res); - destroyPQExpBuffer(upgrade_query); } /*