From 068209d5c70fa11f7d2c07edf62b76428edec143 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 17 Jan 2019 18:55:02 -0300 Subject: [PATCH 3/3] fix dupe FK constraints --- src/backend/commands/tablecmds.c | 43 +++++++++++----- src/test/regress/expected/foreign_key.out | 83 +++++++++++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 38 ++++++++++++++ 3 files changed, 152 insertions(+), 12 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b1b7fe11ac..9893c35217 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7751,31 +7751,50 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, if (recurse && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { PartitionDesc partdesc; + Relation pg_constraint; + List *cloned = NIL; + ListCell *cell; + pg_constraint = heap_open(ConstraintRelationId, RowExclusiveLock); partdesc = RelationGetPartitionDesc(rel); for (i = 0; i < partdesc->nparts; i++) { Oid partitionId = partdesc->oids[i]; Relation partition = heap_open(partitionId, lockmode); - AlteredTableInfo *childtab; - ObjectAddress childAddr; CheckTableNotInUse(partition, "ALTER TABLE"); - /* Find or create work queue entry for this table */ - childtab = ATGetQueueEntry(wqueue, partition); - - childAddr = - ATAddForeignKeyConstraint(wqueue, childtab, partition, - fkconstraint, constrOid, - recurse, true, lockmode); - - /* Record this constraint as dependent on the parent one */ - recordDependencyOn(&childAddr, &address, DEPENDENCY_INTERNAL_AUTO); + CloneFkReferencing(pg_constraint, rel, partition, + list_make1_oid(constrOid), + &cloned); heap_close(partition, NoLock); } + heap_close(pg_constraint, RowExclusiveLock); + + foreach(cell, cloned) + { + ClonedConstraint *cc = (ClonedConstraint *) lfirst(cell); + Relation partition = heap_open(cc->relid, lockmode); + AlteredTableInfo *childtab; + NewConstraint *newcon; + + /* Find or create work queue entry for this partition */ + childtab = ATGetQueueEntry(wqueue, partition); + + newcon = (NewConstraint *) palloc0(sizeof(NewConstraint)); + newcon->name = cc->constraint->conname; + newcon->contype = CONSTR_FOREIGN; + newcon->refrelid = cc->refrelid; + newcon->refindid = cc->conindid; + newcon->conid = cc->conid; + newcon->qual = (Node *) fkconstraint; + + childtab->constraints = lappend(childtab->constraints, newcon); + + heap_close(partition, lockmode); + } } /* diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 52164e89d2..32a3611c94 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1781,3 +1781,86 @@ INSERT INTO fk_notpartitioned_pk VALUES (1600, 601), (1600, 1601); ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2 FOR VALUES IN (1600); -- leave these tables around intentionally +-- Test creating a constraint at the parent that already exists in partitions. +-- There should be no duplicated constraints, and attempts to drop the +-- constraint in partitions should raise appropriate errors. +create schema fkpart0 + create table pkey (a int primary key) + create table fk_part (a int) partition by list (a) + create table fk_part_1 partition of fk_part + (foreign key (a) references fkpart0.pkey) for values in (1) + create table fk_part_23 partition of fk_part + (foreign key (a) references fkpart0.pkey) for values in (2, 3) + partition by list (a) + create table fk_part_23_2 partition of fk_part_23 for values in (2); +alter table fkpart0.fk_part add foreign key (a) references fkpart0.pkey; +\d fkpart0.fk_part_1 \\ -- should have only one FK + Table "fkpart0.fk_part_1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: fkpart0.fk_part FOR VALUES IN (1) +Foreign-key constraints: + "fk_part_1_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a) + +alter table fkpart0.fk_part_1 drop constraint fk_part_1_a_fkey; +ERROR: cannot drop inherited constraint "fk_part_1_a_fkey" of relation "fk_part_1" +\d fkpart0.fk_part_23 \\ -- should have only one FK + Table "fkpart0.fk_part_23" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: fkpart0.fk_part FOR VALUES IN (2, 3) +Partition key: LIST (a) +Foreign-key constraints: + "fk_part_23_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a) +Number of partitions: 1 (Use \d+ to list them.) + +\d fkpart0.fk_part_23_2 \\ -- should have only one FK + Table "fkpart0.fk_part_23_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: fkpart0.fk_part_23 FOR VALUES IN (2) +Foreign-key constraints: + "fk_part_23_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a) + +alter table fkpart0.fk_part_23 drop constraint fk_part_23_a_fkey; +ERROR: cannot drop inherited constraint "fk_part_23_a_fkey" of relation "fk_part_23" +alter table fkpart0.fk_part_23_2 drop constraint fk_part_23_a_fkey; +ERROR: cannot drop inherited constraint "fk_part_23_a_fkey" of relation "fk_part_23_2" +create table fkpart0.fk_part_4 partition of fkpart0.fk_part for values in (4); +\d fkpart0.fk_part_4 + Table "fkpart0.fk_part_4" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: fkpart0.fk_part FOR VALUES IN (4) +Foreign-key constraints: + "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a) + +alter table fkpart0.fk_part_4 drop constraint fk_part_a_fkey; +ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_4" +create table fkpart0.fk_part_56 partition of fkpart0.fk_part + for values in (5,6) partition by list (a); +create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56 + for values in (5); +\d fkpart0.fk_part_56 + Table "fkpart0.fk_part_56" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: fkpart0.fk_part FOR VALUES IN (5, 6) +Partition key: LIST (a) +Foreign-key constraints: + "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a) +Number of partitions: 1 (Use \d+ to list them.) + +alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey; +ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_56" +alter table fkpart0.fk_part_56_5 drop constraint fk_part_a_fkey; +ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_56_5" +\set VERBOSITY terse \\ -- suppress cascade details +drop schema fkpart0 cascade; +NOTICE: drop cascades to 2 other objects +\set VERBOSITY default diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index f387004855..c120af7397 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1289,3 +1289,41 @@ ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2 FOR VALUES IN (1600); -- leave these tables around intentionally + +-- Test creating a constraint at the parent that already exists in partitions. +-- There should be no duplicated constraints, and attempts to drop the +-- constraint in partitions should raise appropriate errors. +create schema fkpart0 + create table pkey (a int primary key) + create table fk_part (a int) partition by list (a) + create table fk_part_1 partition of fk_part + (foreign key (a) references fkpart0.pkey) for values in (1) + create table fk_part_23 partition of fk_part + (foreign key (a) references fkpart0.pkey) for values in (2, 3) + partition by list (a) + create table fk_part_23_2 partition of fk_part_23 for values in (2); + +alter table fkpart0.fk_part add foreign key (a) references fkpart0.pkey; +\d fkpart0.fk_part_1 \\ -- should have only one FK +alter table fkpart0.fk_part_1 drop constraint fk_part_1_a_fkey; + +\d fkpart0.fk_part_23 \\ -- should have only one FK +\d fkpart0.fk_part_23_2 \\ -- should have only one FK +alter table fkpart0.fk_part_23 drop constraint fk_part_23_a_fkey; +alter table fkpart0.fk_part_23_2 drop constraint fk_part_23_a_fkey; + +create table fkpart0.fk_part_4 partition of fkpart0.fk_part for values in (4); +\d fkpart0.fk_part_4 +alter table fkpart0.fk_part_4 drop constraint fk_part_a_fkey; + +create table fkpart0.fk_part_56 partition of fkpart0.fk_part + for values in (5,6) partition by list (a); +create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56 + for values in (5); +\d fkpart0.fk_part_56 +alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey; +alter table fkpart0.fk_part_56_5 drop constraint fk_part_a_fkey; + +\set VERBOSITY terse \\ -- suppress cascade details +drop schema fkpart0 cascade; +\set VERBOSITY default -- 2.11.0