Re: [HACKERS] Transactional sequence stuff breaks pg_upgrade

Andres Freund <andres@anarazel.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