Re: automatically assigning catalog toast oids

Hi,

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