Re: Transactional sequence stuff breaks pg_upgrade

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2017-06-11 20:03:28 -0400, Tom Lane wrote:
>> @@ -391,6 +391,13 @@ GetNewRelFileNode(Oid reltablespace, Rel
>> 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);
>> +

> I'm very doubtful that a) this doesn't get hit in practice, and b) that
> we can rely on it going forward. At least until we change toasting to
> not use the global oid counter.

This is not about assignments from the global OID counter; the function
it's touching is GetNewRelFileNode() not GetNewObjectId().

GetNewObjectId() definitely does get hit during a binary-upgrade restore,
for all object types that pg_upgrade doesn't try to control the OIDs of
--- which is most. We don't care, for the most part. But we *do* care
about relfilenode assignments, for precisely the reason seen in this bug.
*All* assignments of relfilenodes have to be shortcircuited by pg_upgrade
override calls during a binary-restore run, or we risk filename
collisions. So if this assert ever gets hit, we have something to fix.

I intend to not only commit this, but back-patch it. There's enough
changes in relevant code paths that logic that is fine in HEAD might
not be fine in back branches.

regards, tom lane