]> granicus.if.org Git - postgresql/commitdiff
Assert that we don't invent relfilenodes or type OIDs in binary upgrade.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 13 Jun 2017 00:04:32 +0000 (20:04 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 13 Jun 2017 00:04:32 +0000 (20:04 -0400)
During pg_upgrade's restore run, all relfilenode choices should be
overridden by commands in the dump script.  If we ever find ourselves
choosing a relfilenode in the ordinary way, someone blew it.  Likewise for
pg_type OIDs.  Since pg_upgrade might well succeed anyway, if there happens
not to be a conflict during the regression test run, we need assertions
here to keep us on the straight and narrow.

We might someday be able to remove the assertion in GetNewRelFileNode,
if pg_upgrade is rewritten to remove its assumption that old and new
relfilenodes always match.  But it's hard to see how to get rid of the
pg_type OID constraint, since those OIDs are embedded in user tables
in some cases.

Back-patch as far as 9.5, because of the risk of back-patches breaking
something here even if it works in HEAD.  I'd prefer to go back further,
but 9.4 fails both assertions due to get_rel_infos()'s use of a temporary
table.  We can't use the later-branch solution of a CTE for compatibility
reasons (cf commit 5d16332e9), and it doesn't seem worth inventing some
other way to do the query.  (I did check, by dint of changing the Asserts
to elog(WARNING), that there are no other cases of unwanted OID assignments
during 9.4's regression test run.)

Discussion: https://postgr.es/m/19785.1497215827@sss.pgh.pa.us

src/backend/catalog/catalog.c

index 11ee536726ce08bf22b87bc8a087e2f73869e821..92d943cac7584ff55381b551b37229bedc3ed524 100644 (file)
@@ -38,6 +38,7 @@
 #include "catalog/pg_shseclabel.h"
 #include "catalog/pg_subscription.h"
 #include "catalog/pg_tablespace.h"
+#include "catalog/pg_type.h"
 #include "catalog/toasting.h"
 #include "miscadmin.h"
 #include "storage/fd.h"
@@ -340,6 +341,14 @@ GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn)
        ScanKeyData key;
        bool            collides;
 
+       /*
+        * We should never be asked to generate a new pg_type OID during
+        * pg_upgrade; doing so would risk collisions with the OIDs it wants to
+        * assign.  Hitting this assert means there's some path where we failed to
+        * ensure that a type OID is determined by commands in the dump script.
+        */
+       Assert(!IsBinaryUpgrade || RelationGetRelid(relation) != TypeRelationId);
+
        InitDirtySnapshot(SnapshotDirty);
 
        /* Generate new OIDs until we find one not in the table */
@@ -391,6 +400,13 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence)
        bool            collides;
        BackendId       backend;
 
+       /*
+        * If we ever get here during pg_upgrade, there's something wrong; all
+        * relfilenode assignments during a binary-upgrade run should be
+        * determined by commands in the dump script.
+        */
+       Assert(!IsBinaryUpgrade);
+
        switch (relpersistence)
        {
                case RELPERSISTENCE_TEMP: