Thread: automatically assigning catalog toast oids

automatically assigning catalog toast oids

From
John Naylor
Date:
Commit 96cdeae07 added toast tables to most catalogs. One disadvantage
is that the toast declarations require hard-coded oids, even though
only shared catalogs actually need stable oids. Now that we can assign
oids on the fly, it makes sense to do so for toast tables as well, as
in the attached.

-John Naylor

Attachment

Re: automatically assigning catalog toast oids

From
Tom Lane
Date:
John Naylor <jcnaylor@gmail.com> writes:
> Commit 96cdeae07 added toast tables to most catalogs. One disadvantage
> is that the toast declarations require hard-coded oids, even though
> only shared catalogs actually need stable oids. Now that we can assign
> oids on the fly, it makes sense to do so for toast tables as well, as
> in the attached.

I'm a bit dubious that this is a good idea.  It's handy, at least for
forensic situations, that the system catalogs have stable OIDs.

            regards, tom lane


Re: automatically assigning catalog toast oids

From
Stephen Frost
Date:
Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> John Naylor <jcnaylor@gmail.com> writes:
> > Commit 96cdeae07 added toast tables to most catalogs. One disadvantage
> > is that the toast declarations require hard-coded oids, even though
> > only shared catalogs actually need stable oids. Now that we can assign
> > oids on the fly, it makes sense to do so for toast tables as well, as
> > in the attached.
>
> I'm a bit dubious that this is a good idea.  It's handy, at least for
> forensic situations, that the system catalogs have stable OIDs.

I tend to agree...  What's the advantage of assigning them on the fly?

Thanks!

Stephen

Attachment

Re: automatically assigning catalog toast oids

From
Andres Freund
Date:
Hi,

On 2018-12-09 15:42:57 -0500, Stephen Frost wrote:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> > John Naylor <jcnaylor@gmail.com> writes:
> > > Commit 96cdeae07 added toast tables to most catalogs. One disadvantage
> > > is that the toast declarations require hard-coded oids, even though
> > > only shared catalogs actually need stable oids. Now that we can assign
> > > oids on the fly, it makes sense to do so for toast tables as well, as
> > > in the attached.
> > 
> > I'm a bit dubious that this is a good idea.  It's handy, at least for
> > forensic situations, that the system catalogs have stable OIDs.

Hm, but won't they have that for major versions anyway? We ought not to
change the .bki generation in a way that results in differing oids after
a release, no?


> I tend to agree...  What's the advantage of assigning them on the fly?

No idea if that's John's reasoning, but I do like not having to do yet
another manual step that you need to remember/figure out when adding a
new catalog relation.

Greetings,

Andres Freund


Re: automatically assigning catalog toast oids

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> On 2018-12-09 15:42:57 -0500, Stephen Frost wrote:
>> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> I'm a bit dubious that this is a good idea.  It's handy, at least for
> forensic situations, that the system catalogs have stable OIDs.

> Hm, but won't they have that for major versions anyway? We ought not to
> change the .bki generation in a way that results in differing oids after
> a release, no?

Well, that's just a different very-easily-broken assumption.  There are
a lot of things that make auto-assigned OIDs unstable, and I do not think
that we want to guarantee that they'll hold still across a release series.

BTW, now that I'm looking at it, I very much dislike the way commit
578b2297 handled auto-assignment of OIDs in catalogs.  Not only do I not
want to assign more OIDs that way, I don't think we can safely ship v12
like this.  There's basically nothing at all that guarantees
genbki-assigned OIDs won't overlap initdb-assigned OIDs.  In fact,
I think it's about guaranteed to blow up in the face of anybody who
inserts manually-assigned OIDs above around 9000.

What we probably need to do is restructure the FirstBootstrapObjectId
business so that there are separate, well-defined ranges for genbki.pl
and initdb to use.

            regards, tom lane


Re: automatically assigning catalog toast oids

From
Andres Freund
Date:
Hi,

On 2018-12-09 17:14:42 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2018-12-09 15:42:57 -0500, Stephen Frost wrote:
> >> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> > I'm a bit dubious that this is a good idea.  It's handy, at least for
> > forensic situations, that the system catalogs have stable OIDs.
>
> > Hm, but won't they have that for major versions anyway? We ought not to
> > change the .bki generation in a way that results in differing oids after
> > a release, no?
>
> Well, that's just a different very-easily-broken assumption.  There are
> a lot of things that make auto-assigned OIDs unstable, and I do not think
> that we want to guarantee that they'll hold still across a release series.

Why wouldn't they be for genbki (rather than initdb) assigned oids?  I
don't think it's reasonable to add new functions or such post release
that would have move oid assignments for other objects?


> BTW, now that I'm looking at it, I very much dislike the way commit
> 578b2297 handled auto-assignment of OIDs in catalogs.  Not only do I not
> want to assign more OIDs that way

Why do you not want that?


> , I don't think we can safely ship v12 like this.  There's basically
> nothing at all that guarantees genbki-assigned OIDs won't overlap
> initdb-assigned OIDs.  In fact, I think it's about guaranteed to blow
> up in the face of anybody who inserts manually-assigned OIDs above
> around 9000.

I'm not sure I really see a pressing problem with that. I think it'd not
be insane to treat this as a "well, don't do that then".


> What we probably need to do is restructure the FirstBootstrapObjectId
> business so that there are separate, well-defined ranges for genbki.pl
> and initdb to use.

I'm fine with adding a distinct range, the earlier version of the patch
had that. I'd asked for comments if anybody felt a need to keep that,
nobody replied...  I alternatively proposed that we could just start at
FirstNormalObjectId for those and update the server's oid start value to
the maximum genbki assigned oid.  Do you have preferences around that?

Greetings,

Andres Freund


Re: automatically assigning catalog toast oids

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> On 2018-12-09 17:14:42 -0500, Tom Lane wrote:
>> Well, that's just a different very-easily-broken assumption.  There are
>> a lot of things that make auto-assigned OIDs unstable, and I do not think
>> that we want to guarantee that they'll hold still across a release series.

> Why wouldn't they be for genbki (rather than initdb) assigned oids?  I
> don't think it's reasonable to add new functions or such post release
> that would have move oid assignments for other objects?

As you've got this set up, we couldn't change *anything* for fear of
it moving auto-assignments; there's no isolation between catalogs.

Another thing I seriously dislike is that this allows people to omit OIDs
from .dat entries in catalogs where we traditionally hand-assign OIDs.
That's not a good idea because it would mean those entries don't have
stable OIDs, whereas the whole point of hand assignment is to ensure
all built-in objects of a particular type have stable OIDs.  Now, you
could argue about the usefulness of that policy for any given catalog;
but if we decide that catalog X doesn't need stable OIDs then that should
be an intentional policy change, not something that can happen because
one lazy hacker didn't follow the policy.

(This is, btw, another reason why I don't much like allowing toast
tables to not have stable OIDs; it confuses what the policy is for
pg_class.)

>> In fact, I think it's about guaranteed to blow
>> up in the face of anybody who inserts manually-assigned OIDs above
>> around 9000.

> I'm not sure I really see a pressing problem with that. I think it'd not
> be insane to treat this as a "well, don't do that then".

I can name more than one company that will be pretty damn unhappy when
we break their local patches that make use of 9K-range OIDs.  For better
or worse, the policy for the last 20 years has been that OIDs up to 9999
are available for hand assignment.  What you've just done is to break
that for OIDs above some boundary that's not even very well defined
(though it seems to be somewhere around 8500 as of HEAD).

>> What we probably need to do is restructure the FirstBootstrapObjectId
>> business so that there are separate, well-defined ranges for genbki.pl
>> and initdb to use.

> I'm fine with adding a distinct range, the earlier version of the patch
> had that. I'd asked for comments if anybody felt a need to keep that,
> nobody replied...  I alternatively proposed that we could just start at
> FirstNormalObjectId for those and update the server's oid start value to
> the maximum genbki assigned oid.  Do you have preferences around that?

Yeah, I thought about the latter as well.  But it adds complexity to the
bootstrap process and makes it harder to tell what assigned a particular
OID, so I'd rather go with the former, at least until the OID situation
gets too tight to allow for daylight between the ranges.

It looks to me like as of HEAD, genbki.pl is auto-assigning about 1470
OIDs.  Meanwhile, on my RHEL6 machine, initdb is auto-assigning about
1740 OIDs (what a coincidence); of those, 872 are collation entries
that are absorbed from the system environment.  So the second number is
likely to vary a lot from platform to platform.  (I don't have ICU
enabled; I wonder how many that typically adds.)

I'd be inclined to allow say 2000 OIDs for genbki.pl, with 4384 therefore
available for initdb.  We could expect to have to raise the boundary
from time to time, but not very often.

            regards, tom lane


Re: automatically assigning catalog toast oids

From
John Naylor
Date:
On 12/9/18, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Another thing I seriously dislike is that this allows people to omit OIDs
> from .dat entries in catalogs where we traditionally hand-assign OIDs.
> That's not a good idea because it would mean those entries don't have
> stable OIDs, whereas the whole point of hand assignment is to ensure
> all built-in objects of a particular type have stable OIDs.  Now, you
> could argue about the usefulness of that policy for any given catalog;
> but if we decide that catalog X doesn't need stable OIDs then that should
> be an intentional policy change, not something that can happen because
> one lazy hacker didn't follow the policy.

On this point, I believe this could have happened anyway. pg_opclass
has a mix of hand- and initdb-assigned oids, and there was nothing
previously to stop that from creeping into any other catalog, as far
as I can tell.

-John Naylor


Re: automatically assigning catalog toast oids

From
Andres Freund
Date:
Hi,

On 2018-12-09 18:43:14 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2018-12-09 17:14:42 -0500, Tom Lane wrote:
> >> Well, that's just a different very-easily-broken assumption.  There are
> >> a lot of things that make auto-assigned OIDs unstable, and I do not think
> >> that we want to guarantee that they'll hold still across a release series.
> 
> > Why wouldn't they be for genbki (rather than initdb) assigned oids?  I
> > don't think it's reasonable to add new functions or such post release
> > that would have move oid assignments for other objects?
> 
> As you've got this set up, we couldn't change *anything* for fear of
> it moving auto-assignments; there's no isolation between catalogs.

But there wasn't any previously either?


> Another thing I seriously dislike is that this allows people to omit OIDs
> from .dat entries in catalogs where we traditionally hand-assign OIDs.

That's not new, is it?  Sure, now genbki.pl assigns the oid, but
previously it'd just have been heap_insert()? bootparse.y/bootstrap.c
never enforced that oids are assigned for tables that have oids.


> That's not a good idea because it would mean those entries don't have
> stable OIDs, whereas the whole point of hand assignment is to ensure
> all built-in objects of a particular type have stable OIDs.  Now, you
> could argue about the usefulness of that policy for any given catalog;
> but if we decide that catalog X doesn't need stable OIDs then that should
> be an intentional policy change, not something that can happen because
> one lazy hacker didn't follow the policy.

I think we should change that policy, but I also think that there wasn't
any meaningful "assignment policy" change in what I did. So that just
seems like a separate argument.

Note that changing that for "prominent" catalogs would be a bit more
work than just changing the policy, as we'd need to assign oids before
the lookup tables are built - although the current behaviour would kind
of allow us to implement the "not crazy" policy of allowing
auto-assignment as long as the object isn't referenced; but via an imo
fairly opaque mechanism.


> > I'm fine with adding a distinct range, the earlier version of the patch
> > had that. I'd asked for comments if anybody felt a need to keep that,
> > nobody replied...  I alternatively proposed that we could just start at
> > FirstNormalObjectId for those and update the server's oid start value to
> > the maximum genbki assigned oid.  Do you have preferences around that?
> 
> Yeah, I thought about the latter as well.  But it adds complexity to the
> bootstrap process and makes it harder to tell what assigned a particular
> OID, so I'd rather go with the former, at least until the OID situation
> gets too tight to allow for daylight between the ranges.

Yea, it doesn't seem perfect, that's basically why I didn't go for it
last time.


> It looks to me like as of HEAD, genbki.pl is auto-assigning about 1470
> OIDs.  Meanwhile, on my RHEL6 machine, initdb is auto-assigning about
> 1740 OIDs (what a coincidence); of those, 872 are collation entries
> that are absorbed from the system environment.  So the second number is
> likely to vary a lot from platform to platform.  (I don't have ICU
> enabled; I wonder how many that typically adds.)
> 
> I'd be inclined to allow say 2000 OIDs for genbki.pl, with 4384 therefore
> available for initdb.  We could expect to have to raise the boundary
> from time to time, but not very often.

I've attached a patch implementing that.  I'm not particularly in love
with FirstGenbkiObjectId as the symbol, but I couldn't think of
something more descriptive.

I changed the length of fmgr_builtin_oid_index to FirstGenbkiObjectId -
until we allow pg_proc oids to be auto-assigned that'd just be wasted
memory otherwise?

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.

I did however change postgres_fdw's is_builtin(), as that says:
 /*
  * Return true if given object is one of PostgreSQL's built-in objects.
  *
- * We use FirstBootstrapObjectId as the cutoff, so that we only consider
+ * We use FirstGenbkiObjectId as the cutoff, so that we only consider
  * objects with hand-assigned OIDs to be "built in", not for instance any
  * function or type defined in the information_schema.
  *
@@ -154,7 +154,7 @@ lookup_shippable(Oid objectId, Oid classId, PgFdwRelationInfo *fpinfo)

and >= FirstGenbkiObjectId would not be maniually assigned.


I added a throwaway "with 9000-9999 tentatively reserved for forks." to
transam.h, but I'm not sure we really want that, or whether that's good
wording.

Greetings,

Andres Freund

Attachment

Re: automatically assigning catalog toast oids

From
John Naylor
Date:
On 12/11/18, Andres Freund <andres@anarazel.de> wrote:
> I've attached a patch implementing that.  I'm not particularly in love
> with FirstGenbkiObjectId as the symbol, but I couldn't think of
> something more descriptive.

How about FirstAutoAssignedObjectId?


-John Naylor


Re: automatically assigning catalog toast oids

From
Andres Freund
Date:
Hi,

On 2018-12-11 19:33:23 -0500, John Naylor wrote:
> On 12/11/18, Andres Freund <andres@anarazel.de> wrote:
> > I've attached a patch implementing that.  I'm not particularly in love
> > with FirstGenbkiObjectId as the symbol, but I couldn't think of
> > something more descriptive.
> 
> How about FirstAutoAssignedObjectId?

That sounds too general to me. Could just as well be what
FirstNormalObjectId is, or what FirstBootstrapObjectId is. So between
those I do prefer FirstGenbkiObjectId, as that's clearer.  Could go for
FirstGenbkiAssignedObjectId too, but that's probably a bit redundant.

Greetings,

Andres Freund


Re: automatically assigning catalog toast oids

From
Andres Freund
Date:
On 2018-12-11 15:08:02 -0800, Andres Freund wrote:
> Hi,
> 
> On 2018-12-09 18:43:14 -0500, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > On 2018-12-09 17:14:42 -0500, Tom Lane wrote:
> > >> Well, that's just a different very-easily-broken assumption.  There are
> > >> a lot of things that make auto-assigned OIDs unstable, and I do not think
> > >> that we want to guarantee that they'll hold still across a release series.
> > 
> > > Why wouldn't they be for genbki (rather than initdb) assigned oids?  I
> > > don't think it's reasonable to add new functions or such post release
> > > that would have move oid assignments for other objects?
> > 
> > As you've got this set up, we couldn't change *anything* for fear of
> > it moving auto-assignments; there's no isolation between catalogs.
> 
> But there wasn't any previously either?
> 
> 
> > Another thing I seriously dislike is that this allows people to omit OIDs
> > from .dat entries in catalogs where we traditionally hand-assign OIDs.
> 
> That's not new, is it?  Sure, now genbki.pl assigns the oid, but
> previously it'd just have been heap_insert()? bootparse.y/bootstrap.c
> never enforced that oids are assigned for tables that have oids.
> 
> 
> > That's not a good idea because it would mean those entries don't have
> > stable OIDs, whereas the whole point of hand assignment is to ensure
> > all built-in objects of a particular type have stable OIDs.  Now, you
> > could argue about the usefulness of that policy for any given catalog;
> > but if we decide that catalog X doesn't need stable OIDs then that should
> > be an intentional policy change, not something that can happen because
> > one lazy hacker didn't follow the policy.
> 
> I think we should change that policy, but I also think that there wasn't
> any meaningful "assignment policy" change in what I did. So that just
> seems like a separate argument.
> 
> Note that changing that for "prominent" catalogs would be a bit more
> work than just changing the policy, as we'd need to assign oids before
> the lookup tables are built - although the current behaviour would kind
> of allow us to implement the "not crazy" policy of allowing
> auto-assignment as long as the object isn't referenced; but via an imo
> fairly opaque mechanism.
> 
> 
> > > I'm fine with adding a distinct range, the earlier version of the patch
> > > had that. I'd asked for comments if anybody felt a need to keep that,
> > > nobody replied...  I alternatively proposed that we could just start at
> > > FirstNormalObjectId for those and update the server's oid start value to
> > > the maximum genbki assigned oid.  Do you have preferences around that?
> > 
> > Yeah, I thought about the latter as well.  But it adds complexity to the
> > bootstrap process and makes it harder to tell what assigned a particular
> > OID, so I'd rather go with the former, at least until the OID situation
> > gets too tight to allow for daylight between the ranges.
> 
> Yea, it doesn't seem perfect, that's basically why I didn't go for it
> last time.
> 
> 
> > It looks to me like as of HEAD, genbki.pl is auto-assigning about 1470
> > OIDs.  Meanwhile, on my RHEL6 machine, initdb is auto-assigning about
> > 1740 OIDs (what a coincidence); of those, 872 are collation entries
> > that are absorbed from the system environment.  So the second number is
> > likely to vary a lot from platform to platform.  (I don't have ICU
> > enabled; I wonder how many that typically adds.)
> > 
> > I'd be inclined to allow say 2000 OIDs for genbki.pl, with 4384 therefore
> > available for initdb.  We could expect to have to raise the boundary
> > from time to time, but not very often.
> 
> I've attached a patch implementing that.  I'm not particularly in love
> with FirstGenbkiObjectId as the symbol, but I couldn't think of
> something more descriptive.
> 
> I changed the length of fmgr_builtin_oid_index to FirstGenbkiObjectId -
> until we allow pg_proc oids to be auto-assigned that'd just be wasted
> memory otherwise?
> 
> 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.
> 
> I did however change postgres_fdw's is_builtin(), as that says:
>  /*
>   * Return true if given object is one of PostgreSQL's built-in objects.
>   *
> - * We use FirstBootstrapObjectId as the cutoff, so that we only consider
> + * We use FirstGenbkiObjectId as the cutoff, so that we only consider
>   * objects with hand-assigned OIDs to be "built in", not for instance any
>   * function or type defined in the information_schema.
>   *
> @@ -154,7 +154,7 @@ lookup_shippable(Oid objectId, Oid classId, PgFdwRelationInfo *fpinfo)
> 
> and >= FirstGenbkiObjectId would not be maniually assigned.
> 
> 
> I added a throwaway "with 9000-9999 tentatively reserved for forks." to
> transam.h, but I'm not sure we really want that, or whether that's good
> wording.

I've pushed it like that, after blindly attempting to satisfy
Solution.pm...

- Andres


Re: automatically assigning catalog toast oids

From
Tom Lane
Date:
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.

(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


Re: automatically assigning catalog toast oids

From
John Naylor
Date:

> On 12/13/18, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.

setup_depend() mentions other exceptions, but only this one currently has any effect as far as I can see:

"pg_database: it's a feature, not a bug, that template1 is not pinned."

-John Naylor


Re: automatically assigning catalog toast oids

From
Andres Freund
Date:
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


Re: automatically assigning catalog toast oids

From
Alvaro Herrera
Date:
On 2018-Dec-13, Tom Lane wrote:

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

The number of 'p' entries in pg_depend is often annoying when manually
querying it.  I support the idea of special-casing such entries.

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

Why not just move that OID outside the Genbki special range?  I have
seen quite a few installs where schema public was removed and later
re-added.  I've never seen a query hardcode OID 2200, and I'd call one
which does buggy.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: automatically assigning catalog toast oids

From
Andres Freund
Date:
Hi,

On 2018-12-14 22:54:14 -0300, Alvaro Herrera wrote:
> On 2018-Dec-13, Tom Lane wrote:
> > 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.
> 
> Why not just move that OID outside the Genbki special range?  I have
> seen quite a few installs where schema public was removed and later
> re-added.  I've never seen a query hardcode OID 2200, and I'd call one
> which does buggy.

+1

Greetings,

Andres Freund


Re: automatically assigning catalog toast oids

From
John Naylor
Date:
On 12/15/18, Andres Freund <andres@anarazel.de> wrote:
>> > [ separation of FirstBootstrapObjectId and FirstGenbkiObjectId ]

I just noticed a small typo in transam.h. Patch attached.

-John Naylor

Attachment

Re: automatically assigning catalog toast oids

From
Michael Paquier
Date:
On Sat, Feb 16, 2019 at 04:50:51PM +0100, John Naylor wrote:
> I just noticed a small typo in transam.h. Patch attached.

Thanks, fixed.
--
Michael

Attachment

Re: automatically assigning catalog toast oids

From
Andres Freund
Date:
On 2019-02-18 12:45:10 +0900, Michael Paquier wrote:
> On Sat, Feb 16, 2019 at 04:50:51PM +0100, John Naylor wrote:
> > I just noticed a small typo in transam.h. Patch attached.
> 
> Thanks, fixed.

Thx.