Skip to content

Commit c83a387

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 ac55779 commit c83a387

File tree

4 files changed

+107
-73
lines changed

4 files changed

+107
-73
lines changed

‎src/backend/commands/tablecmds.c

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

1118811188
/*
11189-
* Clone constraints for which the parent is on the referenced side.
11189+
* First, clone constraints where the parent is on the referencing side.
1119011190
*/
11191-
CloneFkReferenced(parentRel, partitionRel);
11191+
CloneFkReferencing(wqueue, parentRel, partitionRel);
1119211192

1119311193
/*
11194-
* Now clone constraints where the parent is on the referencing side.
11194+
* Clone constraints for which the parent is on the referenced side.
1119511195
*/
11196-
CloneFkReferencing(wqueue, parentRel, partitionRel);
11196+
CloneFkReferenced(parentRel, partitionRel);
1119711197
}
1119811198

1119911199
/*
@@ -11204,8 +11204,6 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
1120411204
* clone those constraints to the given partition. This is to be called
1120511205
* when the partition is being created or attached.
1120611206
*
11207-
* This ignores self-referencing FKs; those are handled by CloneFkReferencing.
11208-
*
1120911207
* This recurses to partitions, if the relation being attached is partitioned.
1121011208
* Recursion is done by calling addFkRecurseReferenced.
1121111209
*/
@@ -11296,17 +11294,6 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
1129611294
continue;
1129711295
}
1129811296

11299-
/*
11300-
* Don't clone self-referencing foreign keys, which can be in the
11301-
* partitioned table or in the partition-to-be.
11302-
*/
11303-
if (constrForm->conrelid == RelationGetRelid(parentRel) ||
11304-
constrForm->conrelid == RelationGetRelid(partitionRel))
11305-
{
11306-
ReleaseSysCache(tuple);
11307-
continue;
11308-
}
11309-
1131011297
/* We need the same lock level that CreateTrigger will acquire */
1131111298
fkRel = table_open(constrForm->conrelid, ShareRowExclusiveLock);
1131211299

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

Lines changed: 68 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2324,70 +2324,90 @@ CREATE TABLE part33_self_fk (
23242324
id_abc bigint
23252325
);
23262326
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
2327-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
2327+
-- verify that this constraint works
2328+
INSERT INTO parted_self_fk VALUES (1, NULL), (2, NULL), (3, NULL);
2329+
INSERT INTO parted_self_fk VALUES (10, 1), (11, 2), (12, 3) RETURNING tableoid::regclass;
2330+
tableoid
2331+
---------------
2332+
part2_self_fk
2333+
part2_self_fk
2334+
part2_self_fk
2335+
(3 rows)
2336+
2337+
INSERT INTO parted_self_fk VALUES (4, 5); -- error: referenced doesn't exist
2338+
ERROR: insert or update on table "part1_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
2339+
DETAIL: Key (id_abc)=(5) is not present in table "parted_self_fk".
2340+
DELETE FROM parted_self_fk WHERE id = 1 RETURNING *; -- error: reference remains
2341+
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"
2342+
DETAIL: Key (id)=(1) is still referenced from table "parted_self_fk".
2343+
SELECT cr.relname, co.conname, co.convalidated,
23282344
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
23292345
FROM pg_constraint co
23302346
JOIN pg_class cr ON cr.oid = co.conrelid
23312347
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
23322348
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
2333-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2334-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
2335-
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
2336-
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
2337-
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2338-
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2339-
part32_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2340-
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2341-
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2342-
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
2343-
part1_self_fk | part1_self_fk_id_not_null | n | t | | |
2344-
part2_self_fk | parted_self_fk_id_not_null | n | t | | |
2345-
part32_self_fk | part3_self_fk_id_not_null | n | t | | |
2346-
part33_self_fk | part33_self_fk_id_not_null | n | t | | |
2347-
part3_self_fk | part3_self_fk_id_not_null | n | t | | |
2348-
parted_self_fk | parted_self_fk_id_not_null | n | t | | |
2349-
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2350-
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2351-
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2352-
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2353-
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2354-
parted_self_fk | parted_self_fk_pkey | p | t | | |
2355-
(18 rows)
2349+
WHERE co.contype = 'f' AND
2350+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2351+
ORDER BY cr.relname, co.conname, p.conname;
2352+
relname | conname | convalidated | conparent | convalidated | foreignrel
2353+
----------------+--------------------------------+--------------+------------------------------+--------------+----------------
2354+
part1_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2355+
part2_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2356+
part32_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2357+
part33_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2358+
part3_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2359+
parted_self_fk | parted_self_fk_id_abc_fkey | t | | | parted_self_fk
2360+
parted_self_fk | parted_self_fk_id_abc_fkey_1 | t | parted_self_fk_id_abc_fkey | t | part1_self_fk
2361+
parted_self_fk | parted_self_fk_id_abc_fkey_2 | t | parted_self_fk_id_abc_fkey | t | part2_self_fk
2362+
parted_self_fk | parted_self_fk_id_abc_fkey_3 | t | parted_self_fk_id_abc_fkey | t | part3_self_fk
2363+
parted_self_fk | parted_self_fk_id_abc_fkey_3_1 | t | parted_self_fk_id_abc_fkey_3 | t | part33_self_fk
2364+
parted_self_fk | parted_self_fk_id_abc_fkey_4 | t | parted_self_fk_id_abc_fkey_3 | t | part32_self_fk
2365+
(11 rows)
23562366

23572367
-- detach and re-attach multiple times just to ensure everything is kosher
23582368
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
2369+
INSERT INTO part2_self_fk VALUES (16, 9); -- error: referenced doesn't exist
2370+
ERROR: insert or update on table "part2_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
2371+
DETAIL: Key (id_abc)=(9) is not present in table "parted_self_fk".
2372+
DELETE FROM parted_self_fk WHERE id = 2 RETURNING *; -- error: reference remains
2373+
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"
2374+
DETAIL: Key (id)=(2) is still referenced from table "part2_self_fk".
23592375
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2376+
INSERT INTO parted_self_fk VALUES (16, 9); -- error: referenced doesn't exist
2377+
ERROR: insert or update on table "part2_self_fk" violates foreign key constraint "parted_self_fk_id_abc_fkey"
2378+
DETAIL: Key (id_abc)=(9) is not present in table "parted_self_fk".
2379+
DELETE FROM parted_self_fk WHERE id = 3 RETURNING *; -- error: reference remains
2380+
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"
2381+
DETAIL: Key (id)=(3) is still referenced from table "parted_self_fk".
23602382
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
23612383
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2362-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
2384+
ALTER TABLE parted_self_fk DETACH PARTITION part3_self_fk;
2385+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (30) TO (40);
2386+
ALTER TABLE part3_self_fk DETACH PARTITION part33_self_fk;
2387+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
2388+
SELECT cr.relname, co.conname, co.convalidated,
23632389
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
23642390
FROM pg_constraint co
23652391
JOIN pg_class cr ON cr.oid = co.conrelid
23662392
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
23672393
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
2368-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2369-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
2370-
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
2371-
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
2372-
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2373-
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2374-
part32_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2375-
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2376-
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2377-
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
2378-
part1_self_fk | part1_self_fk_id_not_null | n | t | | |
2379-
part2_self_fk | parted_self_fk_id_not_null | n | t | | |
2380-
part32_self_fk | part3_self_fk_id_not_null | n | t | | |
2381-
part33_self_fk | part33_self_fk_id_not_null | n | t | | |
2382-
part3_self_fk | part3_self_fk_id_not_null | n | t | | |
2383-
parted_self_fk | parted_self_fk_id_not_null | n | t | | |
2384-
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2385-
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2386-
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2387-
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2388-
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2389-
parted_self_fk | parted_self_fk_pkey | p | t | | |
2390-
(18 rows)
2394+
WHERE co.contype = 'f' AND
2395+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2396+
ORDER BY cr.relname, co.conname, p.conname;
2397+
relname | conname | convalidated | conparent | convalidated | foreignrel
2398+
----------------+--------------------------------+--------------+------------------------------+--------------+----------------
2399+
part1_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2400+
part2_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2401+
part32_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2402+
part33_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2403+
part3_self_fk | parted_self_fk_id_abc_fkey | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2404+
parted_self_fk | parted_self_fk_id_abc_fkey | t | | | parted_self_fk
2405+
parted_self_fk | parted_self_fk_id_abc_fkey_1 | t | parted_self_fk_id_abc_fkey | t | part1_self_fk
2406+
parted_self_fk | parted_self_fk_id_abc_fkey_2 | t | parted_self_fk_id_abc_fkey | t | part2_self_fk
2407+
parted_self_fk | parted_self_fk_id_abc_fkey_3 | t | parted_self_fk_id_abc_fkey | t | part3_self_fk
2408+
parted_self_fk | parted_self_fk_id_abc_fkey_3_1 | t | parted_self_fk_id_abc_fkey_3 | t | part33_self_fk
2409+
parted_self_fk | parted_self_fk_id_abc_fkey_4 | t | parted_self_fk_id_abc_fkey_3 | t | part32_self_fk
2410+
(11 rows)
23912411

23922412
-- Leave this table around, for pg_upgrade/pg_dump tests
23932413
-- 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
@@ -2528,11 +2528,13 @@ select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
25282528
---------+-------------------------+------------------------+-----------
25292529
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
25302530
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
2531+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
2532+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
25312533
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
25322534
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
25332535
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
25342536
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
2535-
(6 rows)
2537+
(8 rows)
25362538

25372539
alter table parent disable trigger all;
25382540
select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
@@ -2543,11 +2545,13 @@ select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
25432545
---------+-------------------------+------------------------+-----------
25442546
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | D
25452547
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | D
2548+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | D
2549+
child1 | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | D
25462550
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | D
25472551
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | D
25482552
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | D
25492553
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | D
2550-
(6 rows)
2554+
(8 rows)
25512555

25522556
drop table parent, child1;
25532557
-- 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
@@ -1673,29 +1673,52 @@ CREATE TABLE part33_self_fk (
16731673
);
16741674
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
16751675

1676-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1676+
-- verify that this constraint works
1677+
INSERT INTO parted_self_fk VALUES (1, NULL), (2, NULL), (3, NULL);
1678+
INSERT INTO parted_self_fk VALUES (10, 1), (11, 2), (12, 3) RETURNING tableoid::regclass;
1679+
1680+
INSERT INTO parted_self_fk VALUES (4, 5); -- error: referenced doesn't exist
1681+
DELETE FROM parted_self_fk WHERE id = 1 RETURNING *; -- error: reference remains
1682+
1683+
SELECT cr.relname, co.conname, co.convalidated,
16771684
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
16781685
FROM pg_constraint co
16791686
JOIN pg_class cr ON cr.oid = co.conrelid
16801687
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
16811688
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1682-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1683-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1689+
WHERE co.contype = 'f' AND
1690+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1691+
ORDER BY cr.relname, co.conname, p.conname;
16841692

16851693
-- detach and re-attach multiple times just to ensure everything is kosher
16861694
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1695+
1696+
INSERT INTO part2_self_fk VALUES (16, 9); -- error: referenced doesn't exist
1697+
DELETE FROM parted_self_fk WHERE id = 2 RETURNING *; -- error: reference remains
1698+
16871699
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1700+
1701+
INSERT INTO parted_self_fk VALUES (16, 9); -- error: referenced doesn't exist
1702+
DELETE FROM parted_self_fk WHERE id = 3 RETURNING *; -- error: reference remains
1703+
16881704
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
16891705
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
16901706

1691-
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1707+
ALTER TABLE parted_self_fk DETACH PARTITION part3_self_fk;
1708+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (30) TO (40);
1709+
1710+
ALTER TABLE part3_self_fk DETACH PARTITION part33_self_fk;
1711+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
1712+
1713+
SELECT cr.relname, co.conname, co.convalidated,
16921714
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
16931715
FROM pg_constraint co
16941716
JOIN pg_class cr ON cr.oid = co.conrelid
16951717
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
16961718
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1697-
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1698-
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1719+
WHERE co.contype = 'f' AND
1720+
cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1721+
ORDER BY cr.relname, co.conname, p.conname;
16991722

17001723
-- Leave this table around, for pg_upgrade/pg_dump tests
17011724

0 commit comments

Comments
 (0)