projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: ac55779)
Handle self-referencing FKs correctly in partitioned tables
authorÁlvaro Herrera <alvherre@kurilemu.de>
Fri, 2 May 2025 19:25:50 +0000 (21:25 +0200)
committerÁlvaro Herrera <alvherre@kurilemu.de>
Fri, 2 May 2025 19:25:50 +0000 (21:25 +0200)
For self-referencing foreign keys in partitioned tables, we weren't
handling creation of pg_constraint rows during CREATE TABLE PARTITION AS
as well as ALTER TABLE ATTACH PARTITION.  This is an old bug -- mostly,
we broke this in 614a406b4ff1 while trying to fix it (so 12.13, 13.9,
14.6 and 15.0 and up all behave incorrectly).  This commit reverts part
of that with additional fixes for full correctness, and installs more
tests to verify the parts we broke, not just the catalog contents but
also the user-visible behavior.

Backpatch to all live branches.  In branches 13 and 14, commit
46a8c27a7226 changed the behavior during DETACH to drop a FK
constraint rather than trying to repair it, because the complete fix of
repairing catalog constraints was problematic due to lack of previous
fixes.  For this reason, the test behavior in those branches is a bit
different.  However, as best as I can tell, the fix works correctly
there.

In release notes we have to recommend that all self-referencing foreign
keys on partitioned tables be recreated if partitions have been created
or attached after the FK was created, keeping in mind that violating
rows might already be present on the referencing side.

Reported-by: Guillaume Lelarge <guillaume@lelarge.info>
Reported-by: Matthew Gabeler-Lee <fastcat@gmail.com>
Reported-by: Luca Vallisa <luca.vallisa@gmail.com>
Discussion: https://postgr.es/m/CAECtzeWHCA+6tTcm2Oh2+g7fURUJpLZb-=pRXgeWJ-Pi+VU=_w@mail.gmail.com
Discussion: https://postgr.es/m/18156-a44bc7096f0683e6@postgresql.org
Discussion: https://postgr.es/m/CAAT=myvsiF-Attja5DcWoUWh21R12R-sfXECY2-3ynt8kaOqjw@mail.gmail.com

src/backend/commands/tablecmds.c patch | blob | blame | history
src/test/regress/expected/foreign_key.out patch | blob | blame | history
src/test/regress/expected/triggers.out patch | blob | blame | history
src/test/regress/sql/foreign_key.sql patch | blob | blame | history

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2705cf11330ddded7ed3305f7ba06e3d53297c9f..54ad38247aa32e34507f13ab817cb9da694580d7 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -11186,14 +11186,14 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
    Assert(parentRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
 
    /*
-    * Clone constraints for which the parent is on the referenced side.
+    * First, clone constraints where the parent is on the referencing side.
     */
-   CloneFkReferenced(parentRel, partitionRel);
+   CloneFkReferencing(wqueue, parentRel, partitionRel);
 
    /*
-    * Now clone constraints where the parent is on the referencing side.
+    * Clone constraints for which the parent is on the referenced side.
     */
-   CloneFkReferencing(wqueue, parentRel, partitionRel);
+   CloneFkReferenced(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 d05c6b1ac5c6af85de898867801fb49e041aa1eb..4f3f280a439bc9d2227978f258960381e2285af8 100644 (file)
--- 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 c598dc78518745a1025605bc18524c4c9c769a58..f245d7f154924b93137766ad2231008e2446230a 100644 (file)
--- 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 25b09a39a7694a03526c06640a3db27bfd4a57d9..8159e363022423ec621ed02c87be784b4472cd9c 100644 (file)
--- 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
 
This is the main PostgreSQL git repository.