Patch for ALTER TABLE / TYPE

Started by NAKANO Yoshihisaover 20 years ago4 messagespatches
Jump to latest
#1NAKANO Yoshihisa
nakano.yosihisa@jp.fujitsu.com

Hi,

Please find the patch attached. This is for the bug which is posted to
hackers before.
http://archives.postgresql.org/pgsql-hackers/2005-06/msg01442.php

We can see a problem by this bug in following way.

CREATE TABLE pktable (a int primary key);
CREATE TABLE fktable (b int references pktable);
ALTER TABLE pktable ALTER COLUMN a TYPE bigint; -- succeed
REINDEX TABLE pg_depend;
ALTER TABLE pktable ALTER COLUMN a TYPE int; -- fail
NOTICE: constraint fktable_b_fkey on table fktable depends on index
pktable_pkey
ERROR: cannot drop constraint pktable_pkey on table pktable because
other objects depend on it
HINT: Use DROP ... CASCADE to drop the dependent objects too.

I changed the order of constraints list to delete foreign key
constraints first.

Any comments are welcome.

Regards,
Nakano

Attachments:

tablecmds.c.patchtext/plain; name=tablecmds.c.patchDownload+58-58
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: NAKANO Yoshihisa (#1)
Re: Patch for ALTER TABLE / TYPE

NAKANO Yoshihisa <nakano.yosihisa@jp.fujitsu.com> writes:

Please find the patch attached. This is for the bug which is posted to
hackers before.
http://archives.postgresql.org/pgsql-hackers/2005-06/msg01442.php

We can see a problem by this bug in following way.

CREATE TABLE pktable (a int primary key);
CREATE TABLE fktable (b int references pktable);
ALTER TABLE pktable ALTER COLUMN a TYPE bigint; -- succeed
REINDEX TABLE pg_depend;
ALTER TABLE pktable ALTER COLUMN a TYPE int; -- fail
NOTICE: constraint fktable_b_fkey on table fktable depends on index
pktable_pkey
ERROR: cannot drop constraint pktable_pkey on table pktable because
other objects depend on it
HINT: Use DROP ... CASCADE to drop the dependent objects too.

Ah, thanks for providing the simple test case. Much nicer than
Neil's way...

I don't much like the patch though :-(. It seems like a brute force
solution, and it's lacking error checking.

After looking at the test case a bit, I have an alternate approach:
constraints on the column will have DEPENDENCY_AUTO type, while
constraints using the column will have DEPENDENCY_NORMAL type.
Therefore, if we drop the NORMAL ones before the AUTO ones, that
should be enough to fix it. This doesn't require much extra code,
or any extra catalog searches, since the pg_depend record is already
available in ATExecAlterColumnType where we need to decide whether
to stick the item on the front or back of the list.

regards, tom lane

#3NAKANO Yoshihisa
nakano.yosihisa@jp.fujitsu.com
In reply to: Tom Lane (#2)
Re: Patch for ALTER TABLE / TYPE

Tom Lane wrote:

After looking at the test case a bit, I have an alternate approach:
constraints on the column will have DEPENDENCY_AUTO type, while
constraints using the column will have DEPENDENCY_NORMAL type.
Therefore, if we drop the NORMAL ones before the AUTO ones, that
should be enough to fix it. This doesn't require much extra code,
or any extra catalog searches, since the pg_depend record is already
available in ATExecAlterColumnType where we need to decide whether
to stick the item on the front or back of the list.

O.K. It seems nicer than my solution.

Please find the patch attached. I fixed the patch to decide the order
of the list by deptype of the pg_depend record.

Regards,
Nakano

Attachments:

tablecmds.c.patchtext/plain; name=tablecmds.c.patchDownload+33-33
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: NAKANO Yoshihisa (#3)
Re: Patch for ALTER TABLE / TYPE

NAKANO Yoshihisa <nakano.yosihisa@jp.fujitsu.com> writes:

Tom Lane wrote:

After looking at the test case a bit, I have an alternate approach:
constraints on the column will have DEPENDENCY_AUTO type, while
constraints using the column will have DEPENDENCY_NORMAL type.

O.K. It seems nicer than my solution.

Please find the patch attached. I fixed the patch to decide the order
of the list by deptype of the pg_depend record.

Patch applied with some minor comment improvements. Thanks!

regards, tom lane