Skip to content

Commit b3a9c53

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 fd0af49 commit b3a9c53

File tree

4 files changed

+126
-69
lines changed

4 files changed

+126
-69
lines changed

‎src/backend/commands/tablecmds.c

+4-17
Original file line numberDiff line numberDiff line change
@@ -9577,14 +9577,14 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
95779577
Assert(parentRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
95789578

95799579
/*
9580-
* Clone constraints for which the parent is on the referenced side.
9580+
* First, clone constraints where the parent is on the referencing side.
95819581
*/
9582-
CloneFkReferenced(parentRel, partitionRel);
9582+
CloneFkReferencing(wqueue, parentRel, partitionRel);
95839583

95849584
/*
9585-
* Now clone constraints where the parent is on the referencing side.
9585+
* Clone constraints for which the parent is on the referenced side.
95869586
*/
9587-
CloneFkReferencing(wqueue, parentRel, partitionRel);
9587+
CloneFkReferenced(parentRel, partitionRel);
95889588
}
95899589

95909590
/*
@@ -9595,8 +9595,6 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
95959595
* clone those constraints to the given partition. This is to be called
95969596
* when the partition is being created or attached.
95979597
*
9598-
* This ignores self-referencing FKs; those are handled by CloneFkReferencing.
9599-
*
96009598
* This recurses to partitions, if the relation being attached is partitioned.
96019599
* Recursion is done by calling addFkRecurseReferenced.
96029600
*/
@@ -9672,17 +9670,6 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
96729670
continue;
96739671
}
96749672

9675-
/*
9676-
* Don't clone self-referencing foreign keys, which can be in the
9677-
* partitioned table or in the partition-to-be.
9678-
*/
9679-
if (constrForm->conrelid == RelationGetRelid(parentRel) ||
9680-
constrForm->conrelid == RelationGetRelid(partitionRel))
9681-
{
9682-
ReleaseSysCache(tuple);
9683-
continue;
9684-
}
9685-
96869673
/*
96879674
* Because we're only expanding the key space at the referenced side,
96889675
* we don't need to prevent any operation in the referencing table, so

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

+76-36
Original file line numberDiff line numberDiff line change
@@ -1970,58 +1970,98 @@ CREATE TABLE part33_self_fk (
19701970
id_abc bigint
19711971
);
19721972
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
1973-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1973+
-- verify that this constraint works
1974+
INSERT INTO parted_self_fk VALUES (1, NULL), (2, NULL), (3, NULL);
1975+
INSERT INTO parted_self_fk VALUES (10, 1), (11, 2), (12, 3) RETURNING tableoid::regclass;
1976+
tableoid
1977+
---------------
1978+
part2_self_fk
1979+
part2_self_fk
1980+
part2_self_fk
1981+
(3 rows)
1982+
1983+
INSERT INTO parted_self_fk VALUES (4, 5); -- error: referenced doesn't exist
1984+
ERROR: insert or update on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
1985+
DETAIL: Key (id_abc)=(5) is not present in table "parted_self_fk".
1986+
DELETE FROM parted_self_fk WHERE id = 1 RETURNING *; -- error: reference remains
1987+
ERROR: update or delete on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey1" on table "parted_self_fk"
1988+
DETAIL: Key (id)=(1) is still referenced from table "parted_self_fk".
1989+
SELECT cr.relname, co.conname, co.convalidated,
19741990
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
19751991
FROM pg_constraint co
19761992
JOIN pg_class cr ON cr.oid = co.conrelid
19771993
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
19781994
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1979-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1980-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1981-
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
1982-
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
1983-
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1984-
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1985-
part32_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1986-
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1987-
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1988-
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
1989-
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
1990-
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
1991-
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
1992-
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
1993-
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
1994-
parted_self_fk | parted_self_fk_pkey | p | t | | |
1995-
(12 rows)
1995+
WHERE co.contype = 'f' AND
1996+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1997+
ORDER BY cr.relname, co.conname, p.conname;
1998+
relname | conname | convalidated | conparent | convalidated | foreignrel
1999+
----------------+-----------------------------+--------------+-----------------------------+--------------+----------------
2000+
part1_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2001+
part2_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2002+
part32_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2003+
part33_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2004+
part3_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2005+
parted_self_fk | parted_self_fk_id_abc_fkey | t | | | parted_self_fk
2006+
parted_self_fk | parted_self_fk_id_abc_fkey1 | t | parted_self_fk_id_abc_fkey | t | part1_self_fk
2007+
parted_self_fk | parted_self_fk_id_abc_fkey2 | t | parted_self_fk_id_abc_fkey | t | part2_self_fk
2008+
parted_self_fk | parted_self_fk_id_abc_fkey3 | t | parted_self_fk_id_abc_fkey | t | part3_self_fk
2009+
parted_self_fk | parted_self_fk_id_abc_fkey4 | t | parted_self_fk_id_abc_fkey3 | t | part32_self_fk
2010+
parted_self_fk | parted_self_fk_id_abc_fkey5 | t | parted_self_fk_id_abc_fkey3 | t | part33_self_fk
2011+
(11 rows)
19962012

19972013
-- detach and re-attach multiple times just to ensure everything is kosher
19982014
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
2015+
\d+ part2_self_fk
2016+
Table "public.part2_self_fk"
2017+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
2018+
--------+--------+-----------+----------+---------+---------+--------------+-------------
2019+
id | bigint | | not null | | plain | |
2020+
id_abc | bigint | | | | plain | |
2021+
Indexes:
2022+
"part2_self_fk_pkey" PRIMARY KEY, btree (id)
2023+
2024+
INSERT INTO part2_self_fk VALUES (16, 9); -- good, but it'll prevent the attach below
2025+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2026+
ERROR: insert or update on table "part2_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
2027+
DETAIL: Key (id_abc)=(9) is not present in table "parted_self_fk".
2028+
DELETE FROM part2_self_fk WHERE id = 16;
19992029
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2030+
INSERT INTO parted_self_fk VALUES (16, 9); -- error: referenced doesn't exist
2031+
ERROR: insert or update on table "part2_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
2032+
DETAIL: Key (id_abc)=(9) is not present in table "parted_self_fk".
2033+
DELETE FROM parted_self_fk WHERE id = 3 RETURNING *; -- error: reference remains
2034+
ERROR: update or delete on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey1" on table "parted_self_fk"
2035+
DETAIL: Key (id)=(3) is still referenced from table "parted_self_fk".
20002036
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
20012037
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2002-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
2038+
ALTER TABLE parted_self_fk DETACH PARTITION part3_self_fk;
2039+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (30) TO (40);
2040+
ALTER TABLE part3_self_fk DETACH PARTITION part33_self_fk;
2041+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
2042+
SELECT cr.relname, co.conname, co.convalidated,
20032043
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
20042044
FROM pg_constraint co
20052045
JOIN pg_class cr ON cr.oid = co.conrelid
20062046
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
20072047
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
2008-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2009-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
2010-
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
2011-
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
2012-
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2013-
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2014-
part32_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2015-
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2016-
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2017-
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
2018-
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2019-
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2020-
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2021-
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2022-
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2023-
parted_self_fk | parted_self_fk_pkey | p | t | | |
2024-
(12 rows)
2048+
WHERE co.contype = 'f' AND
2049+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2050+
ORDER BY cr.relname, co.conname, p.conname;
2051+
relname | conname | convalidated | conparent | convalidated | foreignrel
2052+
----------------+-----------------------------+--------------+-----------------------------+--------------+----------------
2053+
part1_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2054+
part2_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2055+
part32_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2056+
part33_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2057+
part3_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2058+
parted_self_fk | parted_self_fk_id_abc_fkey | t | | | parted_self_fk
2059+
parted_self_fk | parted_self_fk_id_abc_fkey1 | t | parted_self_fk_id_abc_fkey | t | part1_self_fk
2060+
parted_self_fk | parted_self_fk_id_abc_fkey2 | t | parted_self_fk_id_abc_fkey | t | part2_self_fk
2061+
parted_self_fk | parted_self_fk_id_abc_fkey3 | t | parted_self_fk_id_abc_fkey | t | part3_self_fk
2062+
parted_self_fk | parted_self_fk_id_abc_fkey4 | t | parted_self_fk_id_abc_fkey3 | t | part32_self_fk
2063+
parted_self_fk | parted_self_fk_id_abc_fkey5 | t | parted_self_fk_id_abc_fkey3 | t | part33_self_fk
2064+
(11 rows)
20252065

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

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

+14-10
Original file line numberDiff line numberDiff line change
@@ -2713,23 +2713,27 @@ select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
27132713
tgfoid::regproc, tgenabled
27142714
from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
27152715
order by tgrelid::regclass::text, tgfoid;
2716-
tgrelid | tgname | tgfoid | tgenabled
2717-
---------+-------------------------+---------------------+-----------
2718-
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
2719-
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
2720-
(2 rows)
2716+
tgrelid | tgname | tgfoid | tgenabled
2717+
---------+-------------------------+------------------------+-----------
2718+
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
2719+
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
2720+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
2721+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
2722+
(4 rows)
27212723

27222724
-- Before v15, this has no effect because parent has no triggers:
27232725
alter table parent disable trigger all;
27242726
select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
27252727
tgfoid::regproc, tgenabled
27262728
from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
27272729
order by tgrelid::regclass::text, tgfoid;
2728-
tgrelid | tgname | tgfoid | tgenabled
2729-
---------+-------------------------+---------------------+-----------
2730-
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
2731-
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
2732-
(2 rows)
2730+
tgrelid | tgname | tgfoid | tgenabled
2731+
---------+-------------------------+------------------------+-----------
2732+
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
2733+
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
2734+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
2735+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
2736+
(4 rows)
27332737

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

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

+32-6
Original file line numberDiff line numberDiff line change
@@ -1438,29 +1438,55 @@ CREATE TABLE part33_self_fk (
14381438
);
14391439
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
14401440

1441-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1441+
-- verify that this constraint works
1442+
INSERT INTO parted_self_fk VALUES (1, NULL), (2, NULL), (3, NULL);
1443+
INSERT INTO parted_self_fk VALUES (10, 1), (11, 2), (12, 3) RETURNING tableoid::regclass;
1444+
1445+
INSERT INTO parted_self_fk VALUES (4, 5); -- error: referenced doesn't exist
1446+
DELETE FROM parted_self_fk WHERE id = 1 RETURNING *; -- error: reference remains
1447+
1448+
SELECT cr.relname, co.conname, co.convalidated,
14421449
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
14431450
FROM pg_constraint co
14441451
JOIN pg_class cr ON cr.oid = co.conrelid
14451452
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
14461453
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1447-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1448-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1454+
WHERE co.contype = 'f' AND
1455+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1456+
ORDER BY cr.relname, co.conname, p.conname;
14491457

14501458
-- detach and re-attach multiple times just to ensure everything is kosher
14511459
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1460+
1461+
\d+ part2_self_fk
1462+
1463+
INSERT INTO part2_self_fk VALUES (16, 9); -- good, but it'll prevent the attach below
1464+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1465+
1466+
DELETE FROM part2_self_fk WHERE id = 16;
14521467
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1468+
1469+
INSERT INTO parted_self_fk VALUES (16, 9); -- error: referenced doesn't exist
1470+
DELETE FROM parted_self_fk WHERE id = 3 RETURNING *; -- error: reference remains
1471+
14531472
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
14541473
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
14551474

1456-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1475+
ALTER TABLE parted_self_fk DETACH PARTITION part3_self_fk;
1476+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (30) TO (40);
1477+
1478+
ALTER TABLE part3_self_fk DETACH PARTITION part33_self_fk;
1479+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
1480+
1481+
SELECT cr.relname, co.conname, co.convalidated,
14571482
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
14581483
FROM pg_constraint co
14591484
JOIN pg_class cr ON cr.oid = co.conrelid
14601485
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
14611486
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1462-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1463-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1487+
WHERE co.contype = 'f' AND
1488+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1489+
ORDER BY cr.relname, co.conname, p.conname;
14641490

14651491
-- Leave this table around, for pg_upgrade/pg_dump tests
14661492

0 commit comments

Comments
 (0)