Re: bug in 7.4 SET WITHOUT OIDs

I had a suspicion and it was confirmed:

test=# create table oidtest (a int4, unique(oid));
NOTICE: CREATE TABLE / UNIQUE will create implicit index
"oidtest_oid_key" for table "oidtest"
CREATE TABLE
test=# select oid from oidtest;
oid
-----
(0 rows)

test=# alter table oidtest set without oids;
ALTER TABLE
test=# select oid from oidtest;
ERROR: column "oid" does not exist
test=# \d oidtest
ERROR: cache lookup failed for attribute -2 of relation 765798

Chris

Christopher Kings-Lynne <chriskl(at)familyhealth(dot)com(dot)au> writes:
> I had a suspicion and it was confirmed:
> test=# create table oidtest (a int4, unique(oid));

The problem appears to be that ALTER TABLE SET WITHOUT OIDS doesn't make
the index on the OID column go away.

I don't have a strong opinion on whether to fix this by forcing a drop
of the index or by rejecting the ALTER command. Seems like we have to
do one or the other though.

This is actually just the simplest case of a dependency on the OID
column... whatever the fix is, it has to handle the general case.

regards, tom lane

> The problem appears to be that ALTER TABLE SET WITHOUT OIDS doesn't make
> the index on the OID column go away.
>
> I don't have a strong opinion on whether to fix this by forcing a drop
> of the index or by rejecting the ALTER command. Seems like we have to
> do one or the other though.
>
> This is actually just the simplest case of a dependency on the OID
> column... whatever the fix is, it has to handle the general case.

Maybe it needs CASCADE/RESTRICT added?

Christopher Kings-Lynne <chriskl(at)familyhealth(dot)com(dot)au> writes:
>> The problem appears to be that ALTER TABLE SET WITHOUT OIDS doesn't make
>> the index on the OID column go away.

> Maybe it needs CASCADE/RESTRICT added?

Seems like overkill, considering that this is a very marginal feature.
I'm happy to decree that it works in whichever way is the easiest to
implement.

regards, tom lane

>>Maybe it needs CASCADE/RESTRICT added?
>
> Seems like overkill, considering that this is a very marginal feature.
> I'm happy to decree that it works in whichever way is the easiest to
> implement.

In that case, it seems to me that it has to be default RESTRICT. If
anything depend on it, it must fail. Otherwise when you do it, it could
drop views, functions, everything.

Chris

Christopher Kings-Lynne wrote:
> >>Maybe it needs CASCADE/RESTRICT added?
> >
> > Seems like overkill, considering that this is a very marginal feature.
> > I'm happy to decree that it works in whichever way is the easiest to
> > implement.
>
> In that case, it seems to me that it has to be default RESTRICT. If
> anything depend on it, it must fail. Otherwise when you do it, it could
> drop views, functions, everything.

Seems it should behave just like dropping a column of a table that
already has an index on it:

test=> CREATE TABLE test(x int, y int);
CREATE TABLE
test=> CREATE INDEX ii ON test(y);
CREATE INDEX
test=> ALTER TABLE test DROP COLUMN y;
ALTER TABLE
test=> \d test
Table "public.test"
Column | Type | Modifiers
--------+---------+-----------
x | integer |

which I think means drop the index automatically.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Seems it should behave just like dropping a column of a table that
> already has an index on it:

Yeah. In fact, I am now wondering why we invented SET WITHOUT OIDS at
all, rather than making DROP COLUMN allow the target to be OID.

regards, tom lane

On Tuesday 23 March 2004 02:34, Christopher Kings-Lynne wrote:
> >>Maybe it needs CASCADE/RESTRICT added?
> >
> > Seems like overkill, considering that this is a very marginal feature.
> > I'm happy to decree that it works in whichever way is the easiest to
> > implement.
>
> In that case, it seems to me that it has to be default RESTRICT. If
> anything depend on it, it must fail. Otherwise when you do it, it could
> drop views, functions, everything.
>

FWIW current behavior when dropping a column is to restrict it if there is a
view dependent on the column, however we automagically drop indexes on
columns when dropping columns without even a notice.

Point being that in the original case, I think the index on the oid column
should be dropped automagically, to follow similar behavior with normal
columns.

Robert Treat
--
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> Point being that in the original case, I think the index on the oid column
> should be dropped automagically, to follow similar behavior with normal
> columns.

I am currently testing a fix that allows you to say
ALTER TABLE DROP COLUMN oid;
which will behave the same way a regular user-column DROP would.

I intend to interpret SET WITHOUT OIDS as DROP COLUMN oid RESTRICT.

regards, tom lane

Here's another case that is broken in 7.4, but works when SET WITHOUT
OIDs is reimplemented as a full-fledged DROP:

regression=# create table foo1(f1 int, unique(oid));
NOTICE: CREATE TABLE / UNIQUE will create implicit index "foo1_oid_key" for table "foo1"
CREATE TABLE
regression=# create table foo2(f1 oid references foo1(oid));
CREATE TABLE
regression=# alter table foo1 set without oids;
NOTICE: constraint $1 on table foo2 depends on table foo1 column oid
ERROR: cannot drop table foo1 column oid because other objects depend on it
HINT: Use DROP ... CASCADE to drop the dependent objects too.
regression=# alter table foo1 drop column oid;
NOTICE: constraint $1 on table foo2 depends on table foo1 column oid
ERROR: cannot drop table foo1 column oid because other objects depend on it
HINT: Use DROP ... CASCADE to drop the dependent objects too.
regression=# alter table foo1 drop column oid cascade;
NOTICE: drop cascades to constraint $1 on table foo2
ALTER TABLE
regression=#

regards, tom lane

> I am currently testing a fix that allows you to say
> ALTER TABLE DROP COLUMN oid;
> which will behave the same way a regular user-column DROP would.
>
> I intend to interpret SET WITHOUT OIDS as DROP COLUMN oid RESTRICT.

Will it handle this case:

usa=# create table testy (a int4) without oids;
CREATE TABLE
usa=# alter table testy add oid int4;
ALTER TABLE
usa=# \d testy
Table "public.testy"
Column | Type | Modifiers
--------+---------+-----------
a | integer |
oid | integer |

How about the syntax:

ALTER TABLE t DROP OIDS;

This I can make:

ALTER TABLE t DROP CLUSTER;

Chris

Christopher Kings-Lynne <chriskl(at)familyhealth(dot)com(dot)au> writes:
>> I am currently testing a fix that allows you to say
>> ALTER TABLE DROP COLUMN oid;
>> which will behave the same way a regular user-column DROP would.

> Will it handle this case:
> usa=# create table testy (a int4) without oids;
> usa=# alter table testy add oid int4;

No. This is DROP not ADD.

> How about the syntax:
> ALTER TABLE t DROP OIDS;

Why? We already have one nonstandard-and-redundant syntax for this,
we surely do not need another.

regards, tom lane

Christopher Kings-Lynne <chriskl(at)familyhealth(dot)com(dot)au> writes:
> What I meant is - does it handle dropping a non-system 'oid' column?
> ie. A user column that just happens to be named 'oid'.

If you have one (implying that you don't have a system OID column) then
"DROP COLUMN oid" will drop it, but "SET WITHOUT OIDS" will not. Okay
with you?

regards, tom lane

>>Will it handle this case:
>>usa=# create table testy (a int4) without oids;
>>usa=# alter table testy add oid int4;
>
>
> No. This is DROP not ADD.

What I meant is - does it handle dropping a non-system 'oid' column?
ie. A user column that just happens to be named 'oid'.

Chris

> If you have one (implying that you don't have a system OID column) then
> "DROP COLUMN oid" will drop it, but "SET WITHOUT OIDS" will not. Okay
> with you?

Sounds fair.

Chris

I can confirm that current CVS handles this OK.

---------------------------------------------------------------------------

Christopher Kings-Lynne wrote:
> I had a suspicion and it was confirmed:
>
> test=# create table oidtest (a int4, unique(oid));
> NOTICE: CREATE TABLE / UNIQUE will create implicit index
> "oidtest_oid_key" for table "oidtest"
> CREATE TABLE
> test=# select oid from oidtest;
> oid
> -----
> (0 rows)
>
> test=# alter table oidtest set without oids;
> ALTER TABLE
> test=# select oid from oidtest;
> ERROR: column "oid" does not exist
> test=# \d oidtest
> ERROR: cache lookup failed for attribute -2 of relation 765798
>
>
> Chris
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Yep, Tom fixed it good.

Bruce Momjian wrote:

> I can confirm that current CVS handles this OK.
>
> ---------------------------------------------------------------------------
>
> Christopher Kings-Lynne wrote:
>
>>I had a suspicion and it was confirmed:
>>
>>test=# create table oidtest (a int4, unique(oid));
>>NOTICE: CREATE TABLE / UNIQUE will create implicit index
>>"oidtest_oid_key" for table "oidtest"
>>CREATE TABLE
>>test=# select oid from oidtest;
>> oid
>>-----
>>(0 rows)
>>
>>test=# alter table oidtest set without oids;
>>ALTER TABLE
>>test=# select oid from oidtest;
>>ERROR: column "oid" does not exist
>>test=# \d oidtest
>>ERROR: cache lookup failed for attribute -2 of relation 765798

Christopher Kings-Lynne <chriskl(at)familyhealth(dot)com(dot)au> writes:
> Yep, Tom fixed it good.

Was this another of those darn regurgitated-from-February messages?
I'm about ready to go out and acquire missile targeting coordinates
for pcbuddy.com ...

regards, tom lane

> Was this another of those darn regurgitated-from-February messages?
> I'm about ready to go out and acquire missile targeting coordinates
> for pcbuddy.com ...

Hmmm, maybe - I don't have the email any more though, as I deleted it :(

I get regurgitated emails all the time - it can be quite confusing...

Chris

Tom Lane wrote:
> Christopher Kings-Lynne <chriskl(at)familyhealth(dot)com(dot)au> writes:
> > Yep, Tom fixed it good.
>
> Was this another of those darn regurgitated-from-February messages?
> I'm about ready to go out and acquire missile targeting coordinates
> for pcbuddy.com ...

No, it was me cleaning out my old email. It wasn't clear to me that
this item had been fixed already.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073