From 02f8a416eb2f540b603320eb3aaf034f3dc73e57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Thu, 1 May 2025 13:14:01 +0200
Subject: [PATCH] Fix self-referencing foreign keys on partitioned tables

---
 src/backend/commands/tablecmds.c          |  23 +----
 src/test/regress/expected/foreign_key.out | 116 +++++++++++++---------
 src/test/regress/expected/triggers.out    |   8 +-
 src/test/regress/sql/foreign_key.sql      |  35 +++++--
 4 files changed, 108 insertions(+), 74 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2705cf11330..54ad38247aa 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -11185,15 +11185,15 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
 	/* This only works for declarative partitioning */
 	Assert(parentRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
 
+	/*
+	 * First, clone constraints where the parent is on the referencing side.
+	 */
+	CloneFkReferencing(wqueue, parentRel, partitionRel);
+
 	/*
 	 * Clone constraints for which the parent is on the referenced side.
 	 */
 	CloneFkReferenced(parentRel, partitionRel);
-
-	/*
-	 * Now clone constraints where the parent is on the referencing side.
-	 */
-	CloneFkReferencing(wqueue, parentRel, partitionRel);
 }
 
 /*
@@ -11204,8 +11204,6 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
  * clone those constraints to the given partition.  This is to be called
  * when the partition is being created or attached.
  *
- * This ignores self-referencing FKs; those are handled by CloneFkReferencing.
- *
  * This recurses to partitions, if the relation being attached is partitioned.
  * Recursion is done by calling addFkRecurseReferenced.
  */
@@ -11296,17 +11294,6 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
 			continue;
 		}
 
-		/*
-		 * Don't clone self-referencing foreign keys, which can be in the
-		 * partitioned table or in the partition-to-be.
-		 */
-		if (constrForm->conrelid == RelationGetRelid(parentRel) ||
-			constrForm->conrelid == RelationGetRelid(partitionRel))
-		{
-			ReleaseSysCache(tuple);
-			continue;
-		}
-
 		/* We need the same lock level that CreateTrigger will acquire */
 		fkRel = table_open(constrForm->conrelid, ShareRowExclusiveLock);
 
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index d05c6b1ac5c..4f3f280a439 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2324,70 +2324,90 @@ CREATE TABLE part33_self_fk (
 	id_abc bigint
 );
 ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
-SELECT cr.relname, co.conname, co.contype, co.convalidated,
+-- verify that this constraint works
+INSERT INTO parted_self_fk VALUES (1, NULL), (2, NULL), (3, NULL);
+INSERT INTO parted_self_fk VALUES (10, 1), (11, 2), (12, 3) RETURNING tableoid::regclass;
+   tableoid    
+---------------
+ part2_self_fk
+ part2_self_fk
+ part2_self_fk
+(3 rows)
+
+INSERT INTO parted_self_fk VALUES (4, 5);	-- error: referenced doesn't exist
+ERROR:  insert or update on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
+DETAIL:  Key (id_abc)=(5) is not present in table "parted_self_fk".
+DELETE FROM parted_self_fk WHERE id = 1 RETURNING *;	-- error: reference remains
+ERROR:  update or delete on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey_1" on table "parted_self_fk"
+DETAIL:  Key (id)=(1) is still referenced from table "parted_self_fk".
+SELECT cr.relname, co.conname, co.convalidated,
        p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
 FROM pg_constraint co
 JOIN pg_class cr ON cr.oid = co.conrelid
 LEFT JOIN pg_class cf ON cf.oid = co.confrelid
 LEFT JOIN pg_constraint p ON p.oid = co.conparentid
-WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
-ORDER BY co.contype, cr.relname, co.conname, p.conname;
-    relname     |          conname           | contype | convalidated |         conparent          | convalidated |   foreignrel   
-----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
- part1_self_fk  | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- part2_self_fk  | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- part32_self_fk | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- part33_self_fk | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- part3_self_fk  | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- parted_self_fk | parted_self_fk_id_abc_fkey | f       | t            |                            |              | parted_self_fk
- part1_self_fk  | part1_self_fk_id_not_null  | n       | t            |                            |              | 
- part2_self_fk  | parted_self_fk_id_not_null | n       | t            |                            |              | 
- part32_self_fk | part3_self_fk_id_not_null  | n       | t            |                            |              | 
- part33_self_fk | part33_self_fk_id_not_null | n       | t            |                            |              | 
- part3_self_fk  | part3_self_fk_id_not_null  | n       | t            |                            |              | 
- parted_self_fk | parted_self_fk_id_not_null | n       | t            |                            |              | 
- part1_self_fk  | part1_self_fk_pkey         | p       | t            | parted_self_fk_pkey        | t            | 
- part2_self_fk  | part2_self_fk_pkey         | p       | t            | parted_self_fk_pkey        | t            | 
- part32_self_fk | part32_self_fk_pkey        | p       | t            | part3_self_fk_pkey         | t            | 
- part33_self_fk | part33_self_fk_pkey        | p       | t            | part3_self_fk_pkey         | t            | 
- part3_self_fk  | part3_self_fk_pkey         | p       | t            | parted_self_fk_pkey        | t            | 
- parted_self_fk | parted_self_fk_pkey        | p       | t            |                            |              | 
-(18 rows)
+WHERE co.contype = 'f' AND
+      cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
+ORDER BY cr.relname, co.conname, p.conname;
+    relname     |            conname             | convalidated |          conparent           | convalidated |   foreignrel   
+----------------+--------------------------------+--------------+------------------------------+--------------+----------------
+ part1_self_fk  | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ part2_self_fk  | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ part32_self_fk | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ part33_self_fk | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ part3_self_fk  | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey     | t            |                              |              | parted_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_1   | t            | parted_self_fk_id_abc_fkey   | t            | part1_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_2   | t            | parted_self_fk_id_abc_fkey   | t            | part2_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_3   | t            | parted_self_fk_id_abc_fkey   | t            | part3_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_3_1 | t            | parted_self_fk_id_abc_fkey_3 | t            | part33_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_4   | t            | parted_self_fk_id_abc_fkey_3 | t            | part32_self_fk
+(11 rows)
 
 -- detach and re-attach multiple times just to ensure everything is kosher
 ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
+INSERT INTO part2_self_fk VALUES (16, 9);	-- error: referenced doesn't exist
+ERROR:  insert or update on table "part2_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
+DETAIL:  Key (id_abc)=(9) is not present in table "parted_self_fk".
+DELETE FROM parted_self_fk WHERE id = 2 RETURNING *;	-- error: reference remains
+ERROR:  update or delete on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey_5" on table "part2_self_fk"
+DETAIL:  Key (id)=(2) is still referenced from table "part2_self_fk".
 ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
+INSERT INTO parted_self_fk VALUES (16, 9);	-- error: referenced doesn't exist
+ERROR:  insert or update on table "part2_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
+DETAIL:  Key (id_abc)=(9) is not present in table "parted_self_fk".
+DELETE FROM parted_self_fk WHERE id = 3 RETURNING *;	-- error: reference remains
+ERROR:  update or delete on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey_1" on table "parted_self_fk"
+DETAIL:  Key (id)=(3) is still referenced from table "parted_self_fk".
 ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
 ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
-SELECT cr.relname, co.conname, co.contype, co.convalidated,
+ALTER TABLE parted_self_fk DETACH PARTITION part3_self_fk;
+ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (30) TO (40);
+ALTER TABLE part3_self_fk DETACH PARTITION part33_self_fk;
+ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
+SELECT cr.relname, co.conname, co.convalidated,
        p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
 FROM pg_constraint co
 JOIN pg_class cr ON cr.oid = co.conrelid
 LEFT JOIN pg_class cf ON cf.oid = co.confrelid
 LEFT JOIN pg_constraint p ON p.oid = co.conparentid
-WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
-ORDER BY co.contype, cr.relname, co.conname, p.conname;
-    relname     |          conname           | contype | convalidated |         conparent          | convalidated |   foreignrel   
-----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
- part1_self_fk  | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- part2_self_fk  | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- part32_self_fk | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- part33_self_fk | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- part3_self_fk  | parted_self_fk_id_abc_fkey | f       | t            | parted_self_fk_id_abc_fkey | t            | parted_self_fk
- parted_self_fk | parted_self_fk_id_abc_fkey | f       | t            |                            |              | parted_self_fk
- part1_self_fk  | part1_self_fk_id_not_null  | n       | t            |                            |              | 
- part2_self_fk  | parted_self_fk_id_not_null | n       | t            |                            |              | 
- part32_self_fk | part3_self_fk_id_not_null  | n       | t            |                            |              | 
- part33_self_fk | part33_self_fk_id_not_null | n       | t            |                            |              | 
- part3_self_fk  | part3_self_fk_id_not_null  | n       | t            |                            |              | 
- parted_self_fk | parted_self_fk_id_not_null | n       | t            |                            |              | 
- part1_self_fk  | part1_self_fk_pkey         | p       | t            | parted_self_fk_pkey        | t            | 
- part2_self_fk  | part2_self_fk_pkey         | p       | t            | parted_self_fk_pkey        | t            | 
- part32_self_fk | part32_self_fk_pkey        | p       | t            | part3_self_fk_pkey         | t            | 
- part33_self_fk | part33_self_fk_pkey        | p       | t            | part3_self_fk_pkey         | t            | 
- part3_self_fk  | part3_self_fk_pkey         | p       | t            | parted_self_fk_pkey        | t            | 
- parted_self_fk | parted_self_fk_pkey        | p       | t            |                            |              | 
-(18 rows)
+WHERE co.contype = 'f' AND
+      cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
+ORDER BY cr.relname, co.conname, p.conname;
+    relname     |            conname             | convalidated |          conparent           | convalidated |   foreignrel   
+----------------+--------------------------------+--------------+------------------------------+--------------+----------------
+ part1_self_fk  | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ part2_self_fk  | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ part32_self_fk | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ part33_self_fk | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ part3_self_fk  | parted_self_fk_id_abc_fkey     | t            | parted_self_fk_id_abc_fkey   | t            | parted_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey     | t            |                              |              | parted_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_1   | t            | parted_self_fk_id_abc_fkey   | t            | part1_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_2   | t            | parted_self_fk_id_abc_fkey   | t            | part2_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_3   | t            | parted_self_fk_id_abc_fkey   | t            | part3_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_3_1 | t            | parted_self_fk_id_abc_fkey_3 | t            | part33_self_fk
+ parted_self_fk | parted_self_fk_id_abc_fkey_4   | t            | parted_self_fk_id_abc_fkey_3 | t            | part32_self_fk
+(11 rows)
 
 -- Leave this table around, for pg_upgrade/pg_dump tests
 -- Test creating a constraint at the parent that already exists in partitions.
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index c598dc78518..f245d7f1549 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2528,11 +2528,13 @@ select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
 ---------+-------------------------+------------------------+-----------
  child1  | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins"    | O
  child1  | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd"    | O
+ child1  | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
+ child1  | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
  parent  | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins"    | O
  parent  | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd"    | O
  parent  | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
  parent  | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
-(6 rows)
+(8 rows)
 
 alter table parent disable trigger all;
 select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
@@ -2543,11 +2545,13 @@ select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
 ---------+-------------------------+------------------------+-----------
  child1  | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins"    | D
  child1  | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd"    | D
+ child1  | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | D
+ child1  | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | D
  parent  | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins"    | D
  parent  | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd"    | D
  parent  | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | D
  parent  | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | D
-(6 rows)
+(8 rows)
 
 drop table parent, child1;
 -- Verify that firing state propagates correctly on creation, too
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 25b09a39a76..8159e363022 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1673,29 +1673,52 @@ CREATE TABLE part33_self_fk (
 );
 ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
 
-SELECT cr.relname, co.conname, co.contype, co.convalidated,
+-- verify that this constraint works
+INSERT INTO parted_self_fk VALUES (1, NULL), (2, NULL), (3, NULL);
+INSERT INTO parted_self_fk VALUES (10, 1), (11, 2), (12, 3) RETURNING tableoid::regclass;
+
+INSERT INTO parted_self_fk VALUES (4, 5);	-- error: referenced doesn't exist
+DELETE FROM parted_self_fk WHERE id = 1 RETURNING *;	-- error: reference remains
+
+SELECT cr.relname, co.conname, co.convalidated,
        p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
 FROM pg_constraint co
 JOIN pg_class cr ON cr.oid = co.conrelid
 LEFT JOIN pg_class cf ON cf.oid = co.confrelid
 LEFT JOIN pg_constraint p ON p.oid = co.conparentid
-WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
-ORDER BY co.contype, cr.relname, co.conname, p.conname;
+WHERE co.contype = 'f' AND
+      cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
+ORDER BY cr.relname, co.conname, p.conname;
 
 -- detach and re-attach multiple times just to ensure everything is kosher
 ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
+
+INSERT INTO part2_self_fk VALUES (16, 9);	-- error: referenced doesn't exist
+DELETE FROM parted_self_fk WHERE id = 2 RETURNING *;	-- error: reference remains
+
 ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
+
+INSERT INTO parted_self_fk VALUES (16, 9);	-- error: referenced doesn't exist
+DELETE FROM parted_self_fk WHERE id = 3 RETURNING *;	-- error: reference remains
+
 ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
 ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
 
-SELECT cr.relname, co.conname, co.contype, co.convalidated,
+ALTER TABLE parted_self_fk DETACH PARTITION part3_self_fk;
+ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (30) TO (40);
+
+ALTER TABLE part3_self_fk DETACH PARTITION part33_self_fk;
+ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
+
+SELECT cr.relname, co.conname, co.convalidated,
        p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
 FROM pg_constraint co
 JOIN pg_class cr ON cr.oid = co.conrelid
 LEFT JOIN pg_class cf ON cf.oid = co.confrelid
 LEFT JOIN pg_constraint p ON p.oid = co.conparentid
-WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
-ORDER BY co.contype, cr.relname, co.conname, p.conname;
+WHERE co.contype = 'f' AND
+      cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
+ORDER BY cr.relname, co.conname, p.conname;
 
 -- Leave this table around, for pg_upgrade/pg_dump tests
 
-- 
2.39.5

