Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce)

On Wed, Apr 20, 2022 at 2:34 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> The attached draft patch attempts to improve this situation.
> It reserves these OIDs, and creates the associated macros, through
> the normal BKI infrastructure by adding entries in pg_database.dat.
> We have to delete those rows again during initdb, which is slightly
> ugly but surely no more so than initdb's other direct manipulations
> of pg_database.

I'm not sure I really like this approach, but if you're firmly
convinced that it's better than cleaning up the loose ends in some
other way, I'm not going to waste a lot of energy fighting about it.
It doesn't seem horrible.

> There are a few loose ends:
>
> * I'm a bit inclined to simplify IsPinnedObject by just teaching
> it that *no* entries of pg_database are pinned, which would correspond
> to the evident lack of enforcement in dropdb(). Can anyone see a
> reason why we might pin some database in future?

It's kind of curious that we don't pin anything now. There's kind of
nothing to stop you from hosing the system by dropping template0
and/or template1, or mutating them into some crazy form that doesn't
work. But having said that, if as a matter of policy we don't even pin
template0 or template1 or postgres, then it seems unlikely that we
would suddenly decide to pin some new database in the future.

> * I had to set up the additional pg_database entries with nonzero
> datfrozenxid to avoid an assertion failure during initdb's first VACUUM.
> (That VACUUM will overwrite template1's datfrozenxid before computing
> the global minimum frozen XID, but not these others; and it doesn't like
> finding that the minimum is zero.) This feels klugy. An alternative is
> to delete the extra pg_database rows before that VACUUM, which would
> mean taking those deletes out of make_template0 and make_postgres and
> putting them somewhere seemingly unrelated, so that's a bit crufty too.
> Anybody have a preference?

Not really. If anything that's an argument against this entire
approach, but I already commented on that point above.

> * The new macro names seem ill-chosen. Template0ObjectId is spelled
> randomly differently from the longstanding TemplateDbOid, and surely
> PostgresObjectId is about as vague a name as could possibly have
> been thought of (please name an object that it couldn't apply to).
> I'm a little inclined to rename TemplateDbOid to Template1DbOid and
> use Template0DbOid and PostgresDbOid for the others, but I didn't pull
> that trigger here.

Seems totally reasonable. I don't find the current naming horrible or
I'd not have committed it that way, but putting Db in there makes
sense, too.

--
Robert Haas
EDB: http://www.enterprisedb.com