Skip to content

Commit f51ae31

Browse files
author
Álvaro Herrera
committed
Handle self-referencing FKs correctly in partitioned tables
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 614a406 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 46a8c27 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
1 parent 766d2e6 commit f51ae31

File tree

4 files changed

+107
-61
lines changed

4 files changed

+107
-61
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10608,14 +10608,14 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
1060810608
Assert(parentRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
1060910609

1061010610
/*
10611-
* Clone constraints for which the parent is on the referenced side.
10611+
* First, clone constraints where the parent is on the referencing side.
1061210612
*/
10613-
CloneFkReferenced(parentRel, partitionRel);
10613+
CloneFkReferencing(wqueue, parentRel, partitionRel);
1061410614

1061510615
/*
10616-
* Now clone constraints where the parent is on the referencing side.
10616+
* Clone constraints for which the parent is on the referenced side.
1061710617
*/
10618-
CloneFkReferencing(wqueue, parentRel, partitionRel);
10618+
CloneFkReferenced(parentRel, partitionRel);
1061910619
}
1062010620

1062110621
/*
@@ -10626,8 +10626,6 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
1062610626
* clone those constraints to the given partition. This is to be called
1062710627
* when the partition is being created or attached.
1062810628
*
10629-
* This ignores self-referencing FKs; those are handled by CloneFkReferencing.
10630-
*
1063110629
* This recurses to partitions, if the relation being attached is partitioned.
1063210630
* Recursion is done by calling addFkRecurseReferenced.
1063310631
*/
@@ -10718,17 +10716,6 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
1071810716
continue;
1071910717
}
1072010718

10721-
/*
10722-
* Don't clone self-referencing foreign keys, which can be in the
10723-
* partitioned table or in the partition-to-be.
10724-
*/
10725-
if (constrForm->conrelid == RelationGetRelid(parentRel) ||
10726-
constrForm->conrelid == RelationGetRelid(partitionRel))
10727-
{
10728-
ReleaseSysCache(tuple);
10729-
continue;
10730-
}
10731-
1073210719
/* We need the same lock level that CreateTrigger will acquire */
1073310720
fkRel = table_open(constrForm->conrelid, ShareRowExclusiveLock);
1073410721

‎src/test/regress/expected/foreign_key.out

Lines changed: 68 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,58 +2042,90 @@ CREATE TABLE part33_self_fk (
20422042
id_abc bigint
20432043
);
20442044
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
2045-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
2045+
-- verify that this constraint works
2046+
INSERT INTO parted_self_fk VALUES (1, NULL), (2, NULL), (3, NULL);
2047+
INSERT INTO parted_self_fk VALUES (10, 1), (11, 2), (12, 3) RETURNING tableoid::regclass;
2048+
tableoid
2049+
---------------
2050+
part2_self_fk
2051+
part2_self_fk
2052+
part2_self_fk
2053+
(3 rows)
2054+
2055+
INSERT INTO parted_self_fk VALUES (4, 5); -- error: referenced doesn't exist
2056+
ERROR: insert or update on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
2057+
DETAIL: Key (id_abc)=(5) is not present in table "parted_self_fk".
2058+
DELETE FROM parted_self_fk WHERE id = 1 RETURNING *; -- error: reference remains
2059+
ERROR: update or delete on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey1" on table "parted_self_fk"
2060+
DETAIL: Key (id)=(1) is still referenced from table "parted_self_fk".
2061+
SELECT cr.relname, co.conname, co.convalidated,
20462062
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
20472063
FROM pg_constraint co
20482064
JOIN pg_class cr ON cr.oid = co.conrelid
20492065
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
20502066
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
2051-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2052-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
2053-
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
2054-
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
2055-
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2056-
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2057-
part32_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2058-
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2059-
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2060-
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
2061-
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2062-
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2063-
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2064-
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2065-
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2066-
parted_self_fk | parted_self_fk_pkey | p | t | | |
2067-
(12 rows)
2067+
WHERE co.contype = 'f' AND
2068+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2069+
ORDER BY cr.relname, co.conname, p.conname;
2070+
relname | conname | convalidated | conparent | convalidated | foreignrel
2071+
----------------+-----------------------------+--------------+-----------------------------+--------------+----------------
2072+
part1_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2073+
part2_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2074+
part32_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2075+
part33_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2076+
part3_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2077+
parted_self_fk | parted_self_fk_id_abc_fkey | t | | | parted_self_fk
2078+
parted_self_fk | parted_self_fk_id_abc_fkey1 | t | parted_self_fk_id_abc_fkey | t | part1_self_fk
2079+
parted_self_fk | parted_self_fk_id_abc_fkey2 | t | parted_self_fk_id_abc_fkey | t | part2_self_fk
2080+
parted_self_fk | parted_self_fk_id_abc_fkey3 | t | parted_self_fk_id_abc_fkey | t | part3_self_fk
2081+
parted_self_fk | parted_self_fk_id_abc_fkey4 | t | parted_self_fk_id_abc_fkey3 | t | part32_self_fk
2082+
parted_self_fk | parted_self_fk_id_abc_fkey5 | t | parted_self_fk_id_abc_fkey3 | t | part33_self_fk
2083+
(11 rows)
20682084

20692085
-- detach and re-attach multiple times just to ensure everything is kosher
20702086
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
2087+
INSERT INTO part2_self_fk VALUES (16, 9); -- error: referenced doesn't exist
2088+
ERROR: insert or update on table "part2_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
2089+
DETAIL: Key (id_abc)=(9) is not present in table "parted_self_fk".
2090+
DELETE FROM parted_self_fk WHERE id = 2 RETURNING *; -- error: reference remains
2091+
ERROR: update or delete on table "part1_self_fk" violates foreign key constraint "part2_self_fk_id_abc_fkey" on table "part2_self_fk"
2092+
DETAIL: Key (id)=(2) is still referenced from table "part2_self_fk".
20712093
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2094+
INSERT INTO parted_self_fk VALUES (16, 9); -- error: referenced doesn't exist
2095+
ERROR: insert or update on table "part2_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
2096+
DETAIL: Key (id_abc)=(9) is not present in table "parted_self_fk".
2097+
DELETE FROM parted_self_fk WHERE id = 3 RETURNING *; -- error: reference remains
2098+
ERROR: update or delete on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey1" on table "parted_self_fk"
2099+
DETAIL: Key (id)=(3) is still referenced from table "parted_self_fk".
20722100
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
20732101
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2074-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
2102+
ALTER TABLE parted_self_fk DETACH PARTITION part3_self_fk;
2103+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (30) TO (40);
2104+
ALTER TABLE part3_self_fk DETACH PARTITION part33_self_fk;
2105+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
2106+
SELECT cr.relname, co.conname, co.convalidated,
20752107
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
20762108
FROM pg_constraint co
20772109
JOIN pg_class cr ON cr.oid = co.conrelid
20782110
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
20792111
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
2080-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2081-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
2082-
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
2083-
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
2084-
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2085-
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2086-
part32_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2087-
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2088-
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2089-
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
2090-
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2091-
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2092-
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2093-
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2094-
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2095-
parted_self_fk | parted_self_fk_pkey | p | t | | |
2096-
(12 rows)
2112+
WHERE co.contype = 'f' AND
2113+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2114+
ORDER BY cr.relname, co.conname, p.conname;
2115+
relname | conname | convalidated | conparent | convalidated | foreignrel
2116+
----------------+-----------------------------+--------------+-----------------------------+--------------+----------------
2117+
part1_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2118+
part2_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2119+
part32_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2120+
part33_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2121+
part3_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2122+
parted_self_fk | parted_self_fk_id_abc_fkey | t | | | parted_self_fk
2123+
parted_self_fk | parted_self_fk_id_abc_fkey1 | t | parted_self_fk_id_abc_fkey | t | part1_self_fk
2124+
parted_self_fk | parted_self_fk_id_abc_fkey2 | t | parted_self_fk_id_abc_fkey | t | part2_self_fk
2125+
parted_self_fk | parted_self_fk_id_abc_fkey3 | t | parted_self_fk_id_abc_fkey | t | part3_self_fk
2126+
parted_self_fk | parted_self_fk_id_abc_fkey4 | t | parted_self_fk_id_abc_fkey3 | t | part32_self_fk
2127+
parted_self_fk | parted_self_fk_id_abc_fkey5 | t | parted_self_fk_id_abc_fkey3 | t | part33_self_fk
2128+
(11 rows)
20972129

20982130
-- Leave this table around, for pg_upgrade/pg_dump tests
20992131
-- Test creating a constraint at the parent that already exists in partitions.

‎src/test/regress/expected/triggers.out

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2796,11 +2796,13 @@ select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
27962796
---------+-------------------------+------------------------+-----------
27972797
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
27982798
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
2799+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
2800+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
27992801
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
28002802
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
28012803
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
28022804
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
2803-
(6 rows)
2805+
(8 rows)
28042806

28052807
alter table parent disable trigger all;
28062808
select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
@@ -2811,11 +2813,13 @@ select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
28112813
---------+-------------------------+------------------------+-----------
28122814
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | D
28132815
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | D
2816+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | D
2817+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | D
28142818
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | D
28152819
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | D
28162820
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | D
28172821
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | D
2818-
(6 rows)
2822+
(8 rows)
28192823

28202824
drop table parent, child1;
28212825
-- Verify that firing state propagates correctly on creation, too

‎src/test/regress/sql/foreign_key.sql

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,29 +1490,52 @@ CREATE TABLE part33_self_fk (
14901490
);
14911491
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
14921492

1493-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1493+
-- verify that this constraint works
1494+
INSERT INTO parted_self_fk VALUES (1, NULL), (2, NULL), (3, NULL);
1495+
INSERT INTO parted_self_fk VALUES (10, 1), (11, 2), (12, 3) RETURNING tableoid::regclass;
1496+
1497+
INSERT INTO parted_self_fk VALUES (4, 5); -- error: referenced doesn't exist
1498+
DELETE FROM parted_self_fk WHERE id = 1 RETURNING *; -- error: reference remains
1499+
1500+
SELECT cr.relname, co.conname, co.convalidated,
14941501
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
14951502
FROM pg_constraint co
14961503
JOIN pg_class cr ON cr.oid = co.conrelid
14971504
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
14981505
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1499-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1500-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1506+
WHERE co.contype = 'f' AND
1507+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1508+
ORDER BY cr.relname, co.conname, p.conname;
15011509

15021510
-- detach and re-attach multiple times just to ensure everything is kosher
15031511
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1512+
1513+
INSERT INTO part2_self_fk VALUES (16, 9); -- error: referenced doesn't exist
1514+
DELETE FROM parted_self_fk WHERE id = 2 RETURNING *; -- error: reference remains
1515+
15041516
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1517+
1518+
INSERT INTO parted_self_fk VALUES (16, 9); -- error: referenced doesn't exist
1519+
DELETE FROM parted_self_fk WHERE id = 3 RETURNING *; -- error: reference remains
1520+
15051521
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
15061522
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
15071523

1508-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1524+
ALTER TABLE parted_self_fk DETACH PARTITION part3_self_fk;
1525+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (30) TO (40);
1526+
1527+
ALTER TABLE part3_self_fk DETACH PARTITION part33_self_fk;
1528+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
1529+
1530+
SELECT cr.relname, co.conname, co.convalidated,
15091531
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
15101532
FROM pg_constraint co
15111533
JOIN pg_class cr ON cr.oid = co.conrelid
15121534
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
15131535
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1514-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1515-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1536+
WHERE co.contype = 'f' AND
1537+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1538+
ORDER BY cr.relname, co.conname, p.conname;
15161539

15171540
-- Leave this table around, for pg_upgrade/pg_dump tests
15181541

0 commit comments

Comments
 (0)