Re: automatically assigning catalog toast oids

Hi,

On 2018-12-13 20:21:12 -0500, Tom Lane wrote:
> 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.

Hm, if we go there, it'd probably also good to check for <=
FirstGenbkiObjectId?

> >> 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.

Hm, but aren't pins setup by initdb's setup_depend()? IOW, genbki
assigned oids will be in there? Am I missing something?

> 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.

I think that'd be good, it's the second largest table in a new cluster,
and it's not going to get smaller. 'p' already is somewhat of a special
case, so I'm not particulary concerned with clients having to understand
that.

Greetings,

Andres Freund