Un-break pg_upgrade from pre-v12 servers.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 7 Jul 2020 22:10:42 +0000 (18:10 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 7 Jul 2020 22:10:42 +0000 (18:10 -0400)
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

index fd7b3e092037730bbe688d7062d2d3f56c296751..c2627bb630b7f0c2117d22d6367e282a776fd71e 100644 (file)
@@ -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);
 }
 
 /*