Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key

Started by Alvaro Herrera9 months ago5 messages

alvherre@alvh.no-ip.org

9 months ago

1 attachment(s)

Hello,

I've been looking at this bug once again and I think I finally
understood what's going on and how to fix it.

Ref 1: /messages/by-id/20230707175859.17c91538@karst
Re: Issue attaching a table to a partitioned table with an
auto-referenced foreign key
(Guillaume Lelarge)
Ref 2: /messages/by-id/18156-a44bc7096f0683e6@postgresql.org
BUG #18156: Self-referential foreign key in partitioned table not
enforced on deletes
(Matthew Gabeler-Lee)
Ref 3: /messages/by-id/myvsiF-Attja5DcWoUWh21R12R-sfXECY2-3ynt8kaOqjw@mail.gmail.com
Self referential foreign keys in partitioned table not working as
expected
(Luca Vallisa)

First of all -- apparently we broke this in commit 5914a22f6ea5 (which
fixed the other problems that had been reported by G. Lelarge in Ref 1)
even worse than how it was before, by having the new functions just skip
processing the referenced side altogether. Previously we were at least
partially setting up the necessary triggers, at least some of the time.
So what the report by Luca is saying is, plain and simple, that the
referenced-side action triggers just do not exist, which is why no error
is thrown even on the most trivial cases, on the releases that contain
that commit (17.1, 16.5, 15.9).

The solution I came up with, is to no longer skip creating the
referenced-side objects when the FK is self-referencing. But in order
for this to work, when cloning FKs to partitions, we must process the
referencing side first rather than the referenced side first as we did
up to now; if we don't do it that way, the code there gets confused
about multiple constraint entries already existing for the same table.
Which one gets processed first shouldn't really have any other important
effect, unless I have overlooked something.

The patch also adds equivalent test cases to what was reported; if I
remove the code fix, these cases fail because they no longer report the
expected errors. (The changes to the other regression expected fails
reflect the triggers that are missing.)

We already fixed some bits in 614a406b4ff1 and siblings, but apparently
the test cases we added there were insufficient, or we would have
detected that we were making things worse in 5914a22f6ea5 :-(

Anyway, if people have a chance to give this a look, it would be
helpful.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Linux transformó mi computadora, de una `máquina para hacer cosas',
en un aparato realmente entretenido, sobre el cual cada día aprendo
algo nuevo" (Jaime Salinas)

Attachments:

0001-Fix-self-referencing-foreign-keys-on-partitioned-tab.patchtext/x-diff; charset=utf-8Download

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

Import Notes

Reply to msg id not found: Z-vD6oJjKqgeST0L@msg.df7cb.de202407121236.cksghur2ltm7@alvherre.pgsql20230707175859.17c91538@karst

Tender Wang

tndrwang@gmail.com

9 months ago

In reply to: Alvaro Herrera (#1)

Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月1日周四 20:17写道：

Hello,

I've been looking at this bug once again and I think I finally
understood what's going on and how to fix it.

Ref 1: /messages/by-id/20230707175859.17c91538@karst
Re: Issue attaching a table to a partitioned table with an
auto-referenced foreign key
(Guillaume Lelarge)
Ref 2: /messages/by-id/18156-a44bc7096f0683e6@postgresql.org
BUG #18156: Self-referential foreign key in partitioned table not
enforced on deletes
(Matthew Gabeler-Lee)
Ref 3:
/messages/by-id/myvsiF-Attja5DcWoUWh21R12R-sfXECY2-3ynt8kaOqjw@mail.gmail.com
Self referential foreign keys in partitioned table not working as
expected
(Luca Vallisa)

First of all -- apparently we broke this in commit 5914a22f6ea5 (which
fixed the other problems that had been reported by G. Lelarge in Ref 1)
even worse than how it was before, by having the new functions just skip
processing the referenced side altogether. Previously we were at least
partially setting up the necessary triggers, at least some of the time.
So what the report by Luca is saying is, plain and simple, that the
referenced-side action triggers just do not exist, which is why no error
is thrown even on the most trivial cases, on the releases that contain
that commit (17.1, 16.5, 15.9).

Hmm. I didn't get the same conclusion.
Before commit 5914a22f6ea5, the issue reported by Luca could have happened.
Look at the test below on v17.0:
psql (17.0)
Type "help" for help.

You can see from the above test that no error was reported.
But if I revert the commit 614a406b4ff1, above test would report error on
v16devel:
psql (16devel)
Type "help" for help.

postgres=# create table test (
id_1 int4 not null,
id_2 int4 not null,
parent_id_2 int4 null,
primary key (id_1, id_2),
foreign key (id_1, parent_id_2) references test (id_1, id_2)
) partition by list (id_1);
create table test_1 partition of test for values in (1);
insert into test values (1, 1, null), (1, 2, 1);
delete from test where (id_1, id_2) = (1, 1);
CREATE TABLE
CREATE TABLE
INSERT 0 2
ERROR: update or delete on table "test_1" violates foreign key constraint
"test_id_1_parent_id_2_fkey1" on table "test"
DETAIL: Key (id_1, id_2)=(1, 1) is still referenced from table "test".

Anyway, if people have a chance to give this a look, it would be
helpful.

It's midnight in my time zone. I will look at this tomorrow.

--
Thanks,
Tender Wang

Alvaro Herrera

alvherre@kurilemu.de

9 months ago

In reply to: Tender Wang (#2)

On 2025-May-01, Tender Wang wrote:

Hmm. I didn't get the same conclusion.
Before commit 5914a22f6ea5, the issue reported by Luca could have happened.

[...]

You can see from the above test that no error was reported.
But if I revert the commit 614a406b4ff1, above test would report error on
v16devel:

Yeah, I was mistaken to blame 5914a22f6ea5 for this issue when the real
culprit was 614a406b4ff1. Anyway, I pushed the proposed fix to all
branches last night, so hopefully it works correctly for all cases now.

(As context -- it took me several weeks or months to get FKs on
partitioned tables to work. People would make fun at the "spider"
diagrams I drew on whiteboards, of the relationships between
pg_constraint and pg_trigger entries. And for some reason at no point
did the idea of self-referencing FKs occurred to me. I should have
realized that the complexity was getting out of hand! At the very least
I should have pressed for some more QA help.)

Y'all are still on time to test this a bit more before next week's
releases ... if I have made things even worse I can still revert the
patch. With luck, that won't be necessary.

Regards

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Java is clearly an example of money oriented programming" (A. Stepanov)

Matthew Gabeler-Lee

fastcat@gmail.com

8 months ago

In reply to: Alvaro Herrera (#3)

On Sat, May 3, 2025 at 9:09 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

Y'all are still on time to test this a bit more before next week's
releases ... if I have made things even worse I can still revert the
patch. With luck, that won't be necessary.

I was not able to get to testing this before the release, but I was
able to test today after the release at least.

My earlier test case, and an adjusted version of it that is closer to
my production application (notably adding ON DELETE CASCADE to the
FK), now passes with 15.13.

Thank you :D

Alvaro Herrera

alvherre@kurilemu.de

8 months ago

In reply to: Matthew Gabeler-Lee (#4)

On 2025-May-08, Matthew Gabeler-Lee wrote:

My earlier test case, and an adjusted version of it that is closer to
my production application (notably adding ON DELETE CASCADE to the
FK), now passes with 15.13.

Thank you, that's a relief to know.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Porque Kim no hacía nada, pero, eso sí,
con extraordinario éxito" ("Kim", Kipling)