Re: automatically assigning catalog toast oids

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, John Naylor <jcnaylor(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: automatically assigning catalog toast oids
Date: 2018-12-14 01:21:12
Message-ID: 30851.1544750472@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> [ separation of FirstBootstrapObjectId and FirstGenbkiObjectId ]

It seems like we should also add a check to genbki.pl that the ending
value of GenbkiNextOid is <= FirstBootstrapObjectId, so that we'll
certainly notice when it's necessary to adjust those boundaries.

(There's not a similar check for whether initdb runs past
FirstNormalObjectId, but I think that's fine: the system should cope
with hitting those OIDs again after a wraparound. Also, because of our
uncertainty about just how many OIDs will be consumed for collations,
it'd be a bad idea to ship code with a hard constraint on that boundary
not being overrun.)

>> I did *not* change record_plan_function_dependency(), it seems correct
>> that it doesn't track genbki assigned oids, they certainly can't change
>> while a server is running. But I'm not entirely clear to why that's not
>> using FirstNormalObjectId as the cutoff, so perhaps I'm missing
>> something. Similar with logic in predicate.c.

It's not about whether they can change whether the server is running,
it's about whether they're pinned. OIDs above 10000 might or might
not be pinned; we shouldn't hard-wire assumptions about that. So
I suspect you made the wrong choice there.

BTW, this ties into something that was bugging me this afternoon while
looking at dependencies on the default collation: there's a bunch of
places that special-case DEFAULT_COLLATION_OID to skip adding dependencies
on it, for instance this in dependency.c:

/*
* We must also depend on the constant's collation: it could be
* different from the datatype's, if a CollateExpr was const-folded to
* a simple constant. However we can save work in the most common
* case where the collation is "default", since we know that's pinned.
*/
if (OidIsValid(con->constcollid) &&
con->constcollid != DEFAULT_COLLATION_OID)
add_object_address(OCLASS_COLLATION, con->constcollid, 0,
context->addrs);

I'm pretty sure that that special case is my fault, added in perhaps
over-eagerness to minimize the cost added by the collations feature.
Looking at it now, it seems like mostly a wart. But perhaps there is
a more general principle here: why don't we just hard-wire an assumption
that all OIDs below FirstGenbkiObjectId are pinned? I'm thinking of
getting rid of the DEFAULT_COLLATION_OID special cases in dependency-
recording logic and instead having someplace central like isObjectPinned
just assume that such OIDs are pinned, so that we don't bother to
consult or update pg_depend for them.

We could take it a bit further and not make the 'p' entries for such
OIDs in the first place, greatly reducing the size of pg_depend in
typical installations. Perhaps that'd confuse clients that look at
that catalog, though.

Looking at the existing entries, it seems like we'd have to have
one special case: schema public has OID 2200 but is intentionally
not pinned. I wouldn't have a hard time with teaching isObjectPinned
about that; though if it turns out that many places need to know
about it, maybe it's not workable.

Thoughts?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chapman Flack 2018-12-14 01:30:23 Re: Introducing SNI in TLS handshake for SSL connections
Previous Message Andres Freund 2018-12-14 00:55:21 Re: Minimal logical decoding on standbys