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: https://postgr.es/m/20230707175859.17c91538@karst
Re: Issue attaching a table to a partitioned table with an
auto-referenced foreign key
(Guillaume Lelarge)
Ref 2: https://postgr.es/m/18156-a44bc7096f068...@postgresql.org
BUG #18156: Self-referential foreign key in partitioned table not
enforced on deletes
(Matthew Gabeler-Lee)
Ref 3:
https://postgr.es/m/myvsif-attja5dcwouwh21r12r-sfxecy2-3ynt8kao...@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)
>From 02f8a416eb2f540b603320eb3aaf034f3dc73e57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@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
