Thread: Re: pgsql: Remove WITH OIDS support, change oid catalog columnvisibility.

Re: Andres Freund 2018-11-21 <E1gPG3j-0003cy-03@gemulon.postgresql.org>
> The biggest user of WITH OID columns was postgres' catalog. This
> commit changes all 'magic' oid columns to be columns that are normally
> declared and stored.

postgres=# \d+ pg_class
[...]
Indexe:
    "pg_class_oid_index" UNIQUE, btree (oid)

Now that oid is a proper column, shouldn't that be a PRIMARY KEY?
(Just for the looks.)

Christoph

On 21/11/2018 21:20, Christoph Berg wrote:
> Re: Andres Freund 2018-11-21 <E1gPG3j-0003cy-03@gemulon.postgresql.org>
>> The biggest user of WITH OID columns was postgres' catalog. This
>> commit changes all 'magic' oid columns to be columns that are normally
>> declared and stored.
> postgres=# \d+ pg_class
> [...]
> Indexe:
>      "pg_class_oid_index" UNIQUE, btree (oid)
>
> Now that oid is a proper column, shouldn't that be a PRIMARY KEY?
> (Just for the looks.)
>
> Christoph
>
Curious, is there a reason 'Index' is spelt with a trailing 'e'?


Cheers,
Gavin

Re: Gavin Flower 2018-11-21 <f6f8ce89-e152-0c80-85a5-41aeee08603e@archidevsys.co.nz>
> Curious, is there a reason 'Index' is spelt with a trailing 'e'?

LANG=de_DE.UTF-8

Christoph

On 21/11/2018 09:20, Christoph Berg wrote:
> Re: Andres Freund 2018-11-21 <E1gPG3j-0003cy-03@gemulon.postgresql.org>
>> The biggest user of WITH OID columns was postgres' catalog. This
>> commit changes all 'magic' oid columns to be columns that are normally
>> declared and stored.
> 
> postgres=# \d+ pg_class
> [...]
> Indexe:
>     "pg_class_oid_index" UNIQUE, btree (oid)
> 
> Now that oid is a proper column, shouldn't that be a PRIMARY KEY?
> (Just for the looks.)

This is an independent consideration.  There was nothing before that
prevented an index on an oid column from being a primary key that isn't
still there.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I noticed that this patch has broken restores of existing dump files:

psql:testbed.public.backup:82: ERROR:  unrecognized configuration parameter "default_with_oids"

Quite aside from the fact that this won't work if the user tries to
restore in single-transaction or no-error mode, this is really really
unhelpful because it's impossible to tell whether the error is
ignorable or not, except by digging through the dump file.

What you should do is restore that GUC, but add a check-hook that throws
an error for an attempt to set it to "true".

            regards, tom lane

Hi,

On 2019-01-03 13:40:42 -0500, Tom Lane wrote:
> I noticed that this patch has broken restores of existing dump files:
> 
> psql:testbed.public.backup:82: ERROR:  unrecognized configuration parameter "default_with_oids"
> 
> Quite aside from the fact that this won't work if the user tries to
> restore in single-transaction or no-error mode, this is really really
> unhelpful because it's impossible to tell whether the error is
> ignorable or not, except by digging through the dump file.
> 
> What you should do is restore that GUC, but add a check-hook that throws
> an error for an attempt to set it to "true".

Makes sense, I (or a colleague) will look into that next week. Wanna get my
head out of pluggable storage first...

Greetings,

Andres Freund

On Sat, 5 Jan 2019 at 02:09, Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2019-01-03 13:40:42 -0500, Tom Lane wrote:
> > I noticed that this patch has broken restores of existing dump files:
> >
> > psql:testbed.public.backup:82: ERROR:  unrecognized configuration parameter "default_with_oids"
> >
> > Quite aside from the fact that this won't work if the user tries to
> > restore in single-transaction or no-error mode, this is really really
> > unhelpful because it's impossible to tell whether the error is
> > ignorable or not, except by digging through the dump file.
> >
> > What you should do is restore that GUC, but add a check-hook that throws
> > an error for an attempt to set it to "true".

Attached is a patch that does this.

>
> Makes sense, I (or a colleague) will look into that next week. Wanna get my
> head out of pluggable storage first...
>
> Greetings,
>
> Andres Freund
>


-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company

Hi,

On 2019-01-07 17:03:20 +0530, Amit Khandekar wrote:
> On Sat, 5 Jan 2019 at 02:09, Andres Freund <andres@anarazel.de> wrote:
> > On 2019-01-03 13:40:42 -0500, Tom Lane wrote:
> > > I noticed that this patch has broken restores of existing dump files:
> > >
> > > psql:testbed.public.backup:82: ERROR:  unrecognized configuration parameter "default_with_oids"
> > >
> > > Quite aside from the fact that this won't work if the user tries to
> > > restore in single-transaction or no-error mode, this is really really
> > > unhelpful because it's impossible to tell whether the error is
> > > ignorable or not, except by digging through the dump file.
> > >
> > > What you should do is restore that GUC, but add a check-hook that throws
> > > an error for an attempt to set it to "true".
> 
> Attached is a patch that does this.

Thanks!  I've pushed this, after some minor editorialization (hiding the
GUC, shortened description, shortened error message).

Regards,

Andres