]> granicus.if.org Git - postgresql/commitdiff
Fix creation of duplicate foreign keys on partitions
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Fri, 18 Jan 2019 17:49:40 +0000 (14:49 -0300)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Fri, 18 Jan 2019 18:00:45 +0000 (15:00 -0300)
When creating a foreign key in a partitioned table, if some partitions
already have equivalent constraints, we wastefully create duplicates of
the constraints instead of attaching to the existing ones.  That's
inconsistent with the de-duplication that is applied when a table is
attached as a partition.  To fix, reuse the FK-cloning code instead of
having a separate code path.

Backpatch to Postgres 11.  This is a subtle behavior change, but surely
a welcome one since there's no use in having duplicate foreign keys.

Discovered by Álvaro Herrera while thinking about a different problem
reported by Jesper Pedersen (bug #15587).

Author: Álvaro Herrera
Discussion: https://postgr.es/m/201901151935.zfadrzvyof4k@alvherre.pgsql

src/backend/commands/tablecmds.c
src/test/regress/expected/foreign_key.out
src/test/regress/sql/foreign_key.sql

index 1e108fd61b585cd1892ced5a9c2a10ab59206690..1a1ac698e54552906a998e5eb261d96fa1bcfd03 100644 (file)
@@ -7658,30 +7658,49 @@ 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 */
+                       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);
 
-                       childAddr =
-                               ATAddForeignKeyConstraint(wqueue, childtab, partition,
-                                                                                 fkconstraint, constrOid,
-                                                                                 recurse, true, lockmode);
+                       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;
 
-                       /* Record this constraint as dependent on the parent one */
-                       recordDependencyOn(&childAddr, &address, DEPENDENCY_INTERNAL_AUTO);
+                       childtab->constraints = lappend(childtab->constraints, newcon);
 
-                       heap_close(partition, NoLock);
+                       heap_close(partition, lockmode);
                }
        }
 
index 52d540e8969cd1efe613d7b07c646c2a2d2614af..f85c2ef78a8e8d4ed2865df7424dcb5f59065067 100644 (file)
@@ -1842,3 +1842,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
+      Partitioned 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
+      Partitioned 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
index d1f3d6e525f0dd97fc3722d7961625167b4d54ab..c3a7cfcc019819820828ff7e42fb70cba96b225e 100644 (file)
@@ -1340,3 +1340,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