Fix detach of a partition that has a toplevel FK to a partitioned table

In common cases, foreign keys are defined on the toplevel partitioned
table; but if instead one is defined on a partition and references a
partitioned table, and the referencing partition is detached, we would
examine the pg_constraint row on the partition being detached, and fail
to realize that the sub-constraints must be left alone.  This causes the
ALTER TABLE DETACH process to fail with

 ERROR:  could not find ON INSERT check triggers of foreign key constraint NNN

This is similar but not quite the same as what was fixed by
53af9491a043.  This bug doesn't affect branches earlier than 15, because
the detach procedure was different there, so we only backpatch down to
15.

Fix by skipping such modifying constraints that are children of other
constraints being detached.

Author: Amul Sul <sulamul@gmail.com>
Diagnosys-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b97GuPh6wQPbxQS-Zpy16Oh+0aMv-w64QcGrLhCOZZ6p+g@mail.gmail.com

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2f58f869b3ccc17f793264144fddab741b5b94cd..66c1385f1bec37d1576065ef84ea871a519f9733 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -18800,6 +18800,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
        HeapTuple       tuple,
                                newtuple;
        Relation        trigrel = NULL;
+       List       *fkoids = NIL;
 
        if (concurrent)
        {
@@ -18820,6 +18821,27 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
        fks = copyObject(RelationGetFKeyList(partRel));
        if (fks != NIL)
                trigrel = table_open(TriggerRelationId, RowExclusiveLock);
+
+       /*
+        * It's possible that the partition being detached has a foreign key that
+        * references a partitioned table.  When that happens, there are multiple
+        * pg_constraint rows for the partition: one points to the partitioned
+        * table itself, while the others point to each of its partitions.  Only
+        * the topmost one is to be considered here; the child constraints must be
+        * left alone, because conceptually those aren't coming from our parent
+        * partitioned table, but from this partition itself.
+        *
+        * We implement this by collecting all the constraint OIDs in a first scan
+        * of the FK array, and skipping in the loop below those constraints whose
+        * parents are listed here.
+        */
+       foreach(cell, fks)
+       {
+               ForeignKeyCacheInfo *fk = (ForeignKeyCacheInfo *) lfirst(cell);
+
+               fkoids = lappend_oid(fkoids, fk->conoid);
+       }
+
        foreach(cell, fks)
        {
                ForeignKeyCacheInfo *fk = lfirst(cell);
@@ -18833,9 +18855,13 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
                        elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
                conform = (Form_pg_constraint) GETSTRUCT(contup);
 
-               /* consider only the inherited foreign keys */
+               /*
+                * Consider only inherited foreign keys, and only if their parents
+                * aren't in the list.
+                */
                if (conform->contype != CONSTRAINT_FOREIGN ||
-                       !OidIsValid(conform->conparentid))
+                       !OidIsValid(conform->conparentid) ||
+                       list_member_oid(fkoids, conform->conparentid))
                {
                        ReleaseSysCache(contup);
                        continue;

diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index eac62806c1e71bc5ff5deaa2796c9cabceb92f22..ff6b4e23fe899ae2f8cad7539eb6f243a63550dd 100644 (file)
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2339,6 +2339,13 @@ UPDATE pk SET a = 4002 WHERE a = 4000;
 DELETE FROM pk WHERE a = 4002;
 UPDATE pk SET a = 4502 WHERE a = 4500;
 DELETE FROM pk WHERE a = 4502;
+-- Also, detaching a partition that has the FK itself should work
+-- https://postgr.es/m/CAAJ_b97GuPh6wQPbxQS-Zpy16Oh+0aMv-w64QcGrLhCOZZ6p+g@mail.gmail.com
+CREATE TABLE ffk (a int, b int REFERENCES pk) PARTITION BY list (a);
+CREATE TABLE ffk1 PARTITION OF ffk FOR VALUES IN (1);
+ALTER TABLE ffk1 ADD FOREIGN KEY (a) REFERENCES pk;
+ALTER TABLE ffk DETACH PARTITION ffk1;
+DROP TABLE ffk, ffk1;
 CREATE SCHEMA fkpart4;
 SET search_path TO fkpart4;
 -- dropping/detaching PARTITIONs is prevented if that would break

diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 4128e4e404962038ffae33cbab67ba70ce8b4ce9..98e4d2bf481dbdd70ebdbff331eb6235fac1d93e 100644 (file)
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1672,6 +1672,14 @@ DELETE FROM pk WHERE a = 4002;
 UPDATE pk SET a = 4502 WHERE a = 4500;
 DELETE FROM pk WHERE a = 4502;
 
+-- Also, detaching a partition that has the FK itself should work
+-- https://postgr.es/m/CAAJ_b97GuPh6wQPbxQS-Zpy16Oh+0aMv-w64QcGrLhCOZZ6p+g@mail.gmail.com
+CREATE TABLE ffk (a int, b int REFERENCES pk) PARTITION BY list (a);
+CREATE TABLE ffk1 PARTITION OF ffk FOR VALUES IN (1);
+ALTER TABLE ffk1 ADD FOREIGN KEY (a) REFERENCES pk;
+ALTER TABLE ffk DETACH PARTITION ffk1;
+DROP TABLE ffk, ffk1;
+
 CREATE SCHEMA fkpart4;
 SET search_path TO fkpart4;
 -- dropping/detaching PARTITIONs is prevented if that would break