From 9a9fe65eeb0bc3074793474b9d85b10c3e260e78 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 13 Sep 2018 13:26:18 -0300 Subject: [PATCH v2] fix ALTER TYPE --- src/backend/commands/tablecmds.c | 71 +++++++++++-------------------- src/test/regress/expected/foreign_key.out | 12 ++++++ src/test/regress/sql/foreign_key.sql | 11 +++++ 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e96512e051..03c0b739b3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9527,33 +9527,12 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, { char *defstring = pg_get_constraintdef_command(foundObject.objectId); - /* - * Put NORMAL dependencies at the front of the list and - * AUTO dependencies at the back. This makes sure that - * foreign-key constraints depending on this column will - * be dropped before unique or primary-key constraints of - * the column; which we must have because the FK - * constraints depend on the indexes belonging to the - * unique constraints. - */ - if (foundDep->deptype == DEPENDENCY_NORMAL) - { - tab->changedConstraintOids = - lcons_oid(foundObject.objectId, - tab->changedConstraintOids); - tab->changedConstraintDefs = - lcons(defstring, - tab->changedConstraintDefs); - } - else - { - tab->changedConstraintOids = - lappend_oid(tab->changedConstraintOids, - foundObject.objectId); - tab->changedConstraintDefs = - lappend(tab->changedConstraintDefs, - defstring); - } + tab->changedConstraintOids = + lappend_oid(tab->changedConstraintOids, + foundObject.objectId); + tab->changedConstraintDefs = + lappend(tab->changedConstraintDefs, + defstring); } break; @@ -9893,10 +9872,18 @@ static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) { ObjectAddress obj; + ObjectAddresses *objects; ListCell *def_item; ListCell *oid_item; /* + * Collect all the constraints and indexes to drop so we can process them + * in a single call. That way we don't have to worry about dependencies + * among them. + */ + objects = new_object_addresses(); + + /* * Re-parse the index and constraint definitions, and attach them to the * appropriate work queue entries. We do this before dropping because in * the case of a FOREIGN KEY constraint, we might not yet have exclusive @@ -9937,6 +9924,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) conislocal = con->conislocal; ReleaseSysCache(tup); + ObjectAddressSet(obj, ConstraintRelationId, lfirst_oid(oid_item)); + add_exact_object_address(&obj, objects); + /* * If the constraint is inherited (only), we don't want to inject a * new definition here; it'll get recreated when ATAddCheckConstraint @@ -9960,31 +9950,18 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) ATPostAlterTypeParse(oldId, relid, InvalidOid, (char *) lfirst(def_item), wqueue, lockmode, tab->rewrite); + + ObjectAddressSet(obj, RelationRelationId, lfirst_oid(oid_item)); + add_exact_object_address(&obj, objects); } /* - * Now we can drop the existing constraints and indexes --- constraints - * first, since some of them might depend on the indexes. In fact, we - * have to delete FOREIGN KEY constraints before UNIQUE constraints, but - * we already ordered the constraint list to ensure that would happen. It - * should be okay to use DROP_RESTRICT here, since nothing else should be - * depending on these objects. + * It should be okay to use DROP_RESTRICT here, since nothing else should + * be depending on these objects. */ - foreach(oid_item, tab->changedConstraintOids) - { - obj.classId = ConstraintRelationId; - obj.objectId = lfirst_oid(oid_item); - obj.objectSubId = 0; - performDeletion(&obj, DROP_RESTRICT, PERFORM_DELETION_INTERNAL); - } + performMultipleDeletions(objects, DROP_RESTRICT, PERFORM_DELETION_INTERNAL); - foreach(oid_item, tab->changedIndexOids) - { - obj.classId = RelationRelationId; - obj.objectId = lfirst_oid(oid_item); - obj.objectSubId = 0; - performDeletion(&obj, DROP_RESTRICT, PERFORM_DELETION_INTERNAL); - } + free_object_addresses(objects); /* * The objects will get recreated during subsequent passes over the work diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index b90c4926e2..fc3bbe4deb 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1518,6 +1518,18 @@ DETAIL: Key (a, b)=(2500, 2502) is still referenced from table "fk_partitioned_ ALTER TABLE fk_partitioned_fk DROP CONSTRAINT fk_partitioned_fk_a_fkey; -- done. DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk; +-- Altering a type referenced by a foreign key needs to drop/recreate the FK. +-- Ensure that works. +CREATE TABLE fk_notpartitioned_pk (a INT, PRIMARY KEY(a), CHECK (a > 0)); +CREATE TABLE fk_partitioned_fk (a INT REFERENCES fk_notpartitioned_pk(a) PRIMARY KEY) PARTITION BY RANGE(a); +CREATE TABLE fk_partitioned_fk_1 PARTITION OF fk_partitioned_fk FOR VALUES FROM (MINVALUE) TO (MAXVALUE); +INSERT INTO fk_notpartitioned_pk VALUES (1); +INSERT INTO fk_partitioned_fk VALUES (1); +ALTER TABLE fk_notpartitioned_pk ALTER COLUMN a TYPE bigint; +DELETE FROM fk_notpartitioned_pk WHERE a = 1; +ERROR: update or delete on table "fk_notpartitioned_pk" violates foreign key constraint "fk_partitioned_fk_a_fkey" on table "fk_partitioned_fk" +DETAIL: Key (a)=(1) is still referenced from table "fk_partitioned_fk". +DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk; -- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE -- actions CREATE TABLE fk_notpartitioned_pk (a int, b int, primary key (a, b)); diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index f3e7058583..d2cecdf4eb 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1139,6 +1139,17 @@ ALTER TABLE fk_partitioned_fk DROP CONSTRAINT fk_partitioned_fk_a_fkey; -- done. DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk; +-- Altering a type referenced by a foreign key needs to drop/recreate the FK. +-- Ensure that works. +CREATE TABLE fk_notpartitioned_pk (a INT, PRIMARY KEY(a), CHECK (a > 0)); +CREATE TABLE fk_partitioned_fk (a INT REFERENCES fk_notpartitioned_pk(a) PRIMARY KEY) PARTITION BY RANGE(a); +CREATE TABLE fk_partitioned_fk_1 PARTITION OF fk_partitioned_fk FOR VALUES FROM (MINVALUE) TO (MAXVALUE); +INSERT INTO fk_notpartitioned_pk VALUES (1); +INSERT INTO fk_partitioned_fk VALUES (1); +ALTER TABLE fk_notpartitioned_pk ALTER COLUMN a TYPE bigint; +DELETE FROM fk_notpartitioned_pk WHERE a = 1; +DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk; + -- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE -- actions CREATE TABLE fk_notpartitioned_pk (a int, b int, primary key (a, b)); -- 2.11.0