Fix pg_upgrade to not fail when new-cluster TOAST rules differ from old.

This patch essentially reverts commit 4c6780fd17aa43ed, in favor of a much
simpler solution for the case where the new cluster would choose to create
a TOAST table but the old cluster doesn't have one: just don't create a
TOAST table.

The existing code failed in at least two different ways if the situation
arose: (1) ALTER TABLE RESET didn't grab an exclusive lock, so that the
lock sanity check in create_toast_table failed; (2) pg_upgrade did not
provide a pg_type OID for the new toast table, so that the crosscheck in
TypeCreate failed.  While both these problems were introduced by later
patches, they show that the hack being used to cause TOAST table creation
is overwhelmingly fragile (and untested).  I also note that before the
TypeCreate crosscheck was added, the code would have resulted in assigning
an indeterminate pg_type OID to the toast table, possibly causing a later
OID conflict in that catalog; so that it didn't really work even when
committed.

If we simply don't create a TOAST table, there will only be a problem if
the code tries to store a tuple that's wider than a page, and field
compression isn't sufficient to get it under a page.  Given that the TOAST
creation threshold is intended to be about a quarter of a page, it's very
hard to believe that cross-version differences in the do-we-need-a-toast-
table heuristic could result in an observable problem.  So let's just
follow the old version's conclusion about whether a TOAST table is needed.

(If we ever do change needs_toast_table() so much that this conclusion
doesn't apply, we can devise a solution at that time, and hopefully do
it in a less klugy way than 4c6780fd17aa43ed did.)

Back-patch to 9.3, like the previous patch.

Discussion: <8110.1462291671@sss.pgh.pa.us>

diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 3652d7bf51b46ffe8654362e28cc58233cd6c2f3..cd7c62d70e3248c0c6be614479d2b2e4d7cee30e 100644 (file)
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -165,50 +165,38 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
    if (rel->rd_rel->reltoastrelid != InvalidOid)
        return false;
 
+   /*
+    * Check to see whether the table actually needs a TOAST table.
+    */
    if (!IsBinaryUpgrade)
    {
+       /* Normal mode, normal check */
        if (!needs_toast_table(rel))
            return false;
    }
    else
    {
        /*
-        * Check to see whether the table needs a TOAST table.
+        * In binary-upgrade mode, create a TOAST table if and only if
+        * pg_upgrade told us to (ie, a TOAST table OID has been provided).
         *
-        * If an update-in-place TOAST relfilenode is specified, force TOAST
-        * file creation even if it seems not to need one.  This handles the
-        * case where the old cluster needed a TOAST table but the new cluster
-        * would not normally create one.
-        */
-
-       /*
-        * If a TOAST oid is not specified, skip TOAST creation as we will do
-        * it later so we don't create a TOAST table whose OID later conflicts
-        * with a user-supplied OID.  This handles cases where the old cluster
-        * didn't need a TOAST table, but the new cluster does.
+        * This indicates that the old cluster had a TOAST table for the
+        * current table.  We must create a TOAST table to receive the old
+        * TOAST file, even if the table seems not to need one.
+        *
+        * Contrariwise, if the old cluster did not have a TOAST table, we
+        * should be able to get along without one even if the new version's
+        * needs_toast_table rules suggest we should have one.  There is a lot
+        * of daylight between where we will create a TOAST table and where
+        * one is really necessary to avoid failures, so small cross-version
+        * differences in the when-to-create heuristic shouldn't be a problem.
+        * If we tried to create a TOAST table anyway, we would have the
+        * problem that it might take up an OID that will conflict with some
+        * old-cluster table we haven't seen yet.
         */
-       if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid))
+       if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid) ||
+           !OidIsValid(binary_upgrade_next_toast_pg_type_oid))
            return false;
-
-       /*
-        * If a special TOAST value has been passed in, it means we are in
-        * cleanup mode --- we are creating needed TOAST tables after all user
-        * tables with specified OIDs have been created.  We let the system
-        * assign a TOAST oid for us.  The tables are empty so the missing
-        * TOAST tables were not a problem.
-        */
-       if (binary_upgrade_next_toast_pg_class_oid == OPTIONALLY_CREATE_TOAST_OID)
-       {
-           /* clear as it is not to be used; it is just a flag */
-           binary_upgrade_next_toast_pg_class_oid = InvalidOid;
-
-           if (!needs_toast_table(rel))
-               return false;
-       }
-
-       /* both should be set, or not set */
-       Assert(OidIsValid(binary_upgrade_next_toast_pg_class_oid) ==
-              OidIsValid(binary_upgrade_next_toast_pg_type_oid));
    }
 
    /*

diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c
index 6d6f84d7252a355ed9439438891b14149f5f37bb..19e52e2141141c843161c1b8a49ee839067505bb 100644 (file)
--- a/src/bin/pg_upgrade/dump.c
+++ b/src/bin/pg_upgrade/dump.c
@@ -12,7 +12,6 @@
 #include "pg_upgrade.h"
 
 #include <sys/types.h>
-#include "catalog/binary_upgrade.h"
 
 
 void
@@ -69,71 +68,3 @@ generate_old_dump(void)
    end_progress_output();
    check_ok();
 }
-
-
-/*
- * It is possible for there to be a mismatch in the need for TOAST tables
- * between the old and new servers, e.g. some pre-9.1 tables didn't need
- * TOAST tables but will need them in 9.1+.  (There are also opposite cases,
- * but these are handled by setting binary_upgrade_next_toast_pg_class_oid.)
- *
- * We can't allow the TOAST table to be created by pg_dump with a
- * pg_dump-assigned oid because it might conflict with a later table that
- * uses that oid, causing a "file exists" error for pg_class conflicts, and
- * a "duplicate oid" error for pg_type conflicts.  (TOAST tables need pg_type
- * entries.)
- *
- * Therefore, a backend in binary-upgrade mode will not create a TOAST
- * table unless an OID as passed in via pg_upgrade_support functions.
- * This function is called after the restore and uses ALTER TABLE to
- * auto-create any needed TOAST tables which will not conflict with
- * restored oids.
- */
-void
-optionally_create_toast_tables(void)
-{
-   int         dbnum;
-
-   prep_status("Creating newly-required TOAST tables");
-
-   for (dbnum = 0; dbnum < new_cluster.dbarr.ndbs; dbnum++)
-   {
-       PGresult   *res;
-       int         ntups;
-       int         rowno;
-       int         i_nspname,
-                   i_relname;
-       DbInfo     *active_db = &new_cluster.dbarr.dbs[dbnum];
-       PGconn     *conn = connectToServer(&new_cluster, active_db->db_name);
-
-       res = executeQueryOrDie(conn,
-                               "SELECT n.nspname, c.relname "
-                               "FROM   pg_catalog.pg_class c, "
-                               "       pg_catalog.pg_namespace n "
-                               "WHERE  c.relnamespace = n.oid AND "
-                               "       n.nspname NOT IN ('pg_catalog', 'information_schema') AND "
-                               "c.relkind IN ('r', 'm') AND "
-                               "c.reltoastrelid = 0");
-
-       ntups = PQntuples(res);
-       i_nspname = PQfnumber(res, "nspname");
-       i_relname = PQfnumber(res, "relname");
-       for (rowno = 0; rowno < ntups; rowno++)
-       {
-           /* enable auto-oid-numbered TOAST creation if needed */
-           PQclear(executeQueryOrDie(conn, "SELECT pg_catalog.binary_upgrade_set_next_toast_pg_class_oid('%d'::pg_catalog.oid);",
-                                     OPTIONALLY_CREATE_TOAST_OID));
-
-           /* dummy command that also triggers check for required TOAST table */
-           PQclear(executeQueryOrDie(conn, "ALTER TABLE %s.%s RESET (binary_upgrade_dummy_option);",
-                        quote_identifier(PQgetvalue(res, rowno, i_nspname)),
-                      quote_identifier(PQgetvalue(res, rowno, i_relname))));
-       }
-
-       PQclear(res);
-
-       PQfinish(conn);
-   }
-
-   check_ok();
-}

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 4a7281b36985a926ca187be7e926c32de477a778..566db9d40c135552dcc7bc16250301e6eaefb5d5 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -334,13 +334,11 @@ create_new_objects(void)
 
    /*
     * We don't have minmxids for databases or relations in pre-9.3 clusters,
-    * so set those after we have restores the schemas.
+    * so set those after we have restored the schema.
     */
    if (GET_MAJOR_VERSION(old_cluster.major_version) < 903)
        set_frozenxids(true);
 
-   optionally_create_toast_tables();
-
    /* regenerate now that we have objects in the databases */
    get_db_and_rel_infos(&new_cluster);
 }

diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index a43dff5f53b4db172f0efc2d9c07d58b3ff0ba4f..50660050fb9da190c358c7c4018b28a8439d3964 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -349,7 +349,6 @@ void        disable_old_cluster(void);
 /* dump.c */
 
 void       generate_old_dump(void);
-void       optionally_create_toast_tables(void);
 
 
 /* exec.c */

diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index efca09fa2df5f5f1942eb34b061b227b195f4d5f..f001cf3266721eab4c4b0bd8296f404f734db9fc 100644 (file)
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -14,11 +14,6 @@
 #ifndef BINARY_UPGRADE_H
 #define BINARY_UPGRADE_H
 
-#include "catalog/pg_authid.h"
-
-/* pick a OID that will never be used for TOAST tables */
-#define OPTIONALLY_CREATE_TOAST_OID BOOTSTRAP_SUPERUSERID
-
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_type_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_array_pg_type_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_type_oid;