From 7332c3cbb39026e62f4bd0a8acf3df8f701a9e2f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 12 Jun 2017 20:04:32 -0400 Subject: [PATCH] Assert that we don't invent relfilenodes or type OIDs in binary upgrade. 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 | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 11ee536726..92d943cac7 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -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: -- 2.40.0