Re: Transactional sequence stuff breaks pg_upgrade

On 2017-06-12 17:35:37 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > In my opinion the problem of:
>
> > On 2017-06-12 17:13:34 -0400, Tom Lane wrote:
> >> assignments of relfilenodes have to be shortcircuited by pg_upgrade
> >> override calls during a binary-restore run, or we risk filename
> >> collisions.
>
> > should instead be solved by simply not even trying to preserve
> > relfilenodes. We can "just" copy/link files to the the new
> > relfilenodes, there's no need to preserve them, in contrast to
> > pg_class.oid etc. But that's obviously something for the future.
>
> Right. But until somebody gets around to fixing that, it's a problem.
>
> Also, even if we fix this, we still have the issue of type OIDs residing
> on-disk in places like array headers; that results in pg_upgrade having
> to control type OID assignments as well.

Yes, but those should be largely controlled and controllable. With the
exception of the type oids of toast tables, more on that below.

> I thought about trying to insert an assert in GetNewOidWithIndex to
> ensure that no type OIDs are selected automatically during upgrade,
> but it seemed a bit too complicated. Might be a good idea to figure
> it out though, unless you have an idea for removing that requirement.

I think the only type oids assigned during pg_upgrade are the oids for
toast type types, right? I can think of two decent ways to deal with
those:

a) Do not create a corresponding composite type for toast tables. Not
super pretty, but I doubt it'd be a huge issue.
b) Use *one* composite type for all of the toast tables. That'd not be
entirely trivial because of pg_type.typrelid.

Both of these would have the advantage of removing some quite redundant
content from the catalogs.

I think with such a change, we'd have no issue with *additional* toast
tables being created during the run? We already take care of toast
tables not being created, by forcing the creation in binary upgrade mode
if a toast oid is set.

> Later versions avoid that by using a CTE instead of a temp table.
> I'm not excited enough about this to back-port the patch that
> changed it, so I'm thinking of just adding the assert back to 9.5.

I'm still a bit doubtful that we know enough to consider whether the
assert is actually safe in practice, to backpatch it. On the other
hand, it'd only affect people building with asserts...

Greetings,

Andres Freund