From 02f8a416eb2f540b603320eb3aaf034f3dc73e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= 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