Skip to content

Commit 614a406

Browse files
alvherrealvherre
committed
Fix self-referencing foreign keys with partitioned tables
There are a number of bugs in this area. Two of them are fixed here, namely: 1. get_relation_idx_constraint_oid does not restrict the type of constraint that's returned, so with sufficient bad luck it can return the OID of a foreign key constraint. This has the effect that a primary key in a partition can end up as a child of a foreign key, which makes no sense (it needs to be the child of the equivalent primary key.) Change the API contract so that only index-backed constraints are returned, mimicking get_constraint_index(). 2. Both CloneFkReferenced and CloneFkReferencing clone a self-referencing foreign key, so the partition ends up with a duplicate foreign key. Change the former function to ignore such constraints. Add some tests to verify that things are better now. (However, these new tests show some additional misbehavior that will be fixed later -- namely that there's a constraint marked NOT VALID.) Backpatch to 12, where these constraints are possible at all. Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20220603154232.1715b14c@karst
1 parent 3edc71e commit 614a406

File tree

4 files changed

+161
-1
lines changed

4 files changed

+161
-1
lines changed

‎src/backend/catalog/pg_constraint.c

+12-1
Original file line numberDiff line numberDiff line change
@@ -985,8 +985,12 @@ get_relation_constraint_attnos(Oid relid, const char *conname,
985985
}
986986

987987
/*
988-
* Return the OID of the constraint associated with the given index in the
988+
* Return the OID of the constraint enforced by the given index in the
989989
* given relation; or InvalidOid if no such index is catalogued.
990+
*
991+
* Much like get_constraint_index, this function is concerned only with the
992+
* one constraint that "owns" the given index. Therefore, constraints of
993+
* types other than unique, primary-key, and exclusion are ignored.
990994
*/
991995
Oid
992996
get_relation_idx_constraint_oid(Oid relationId, Oid indexId)
@@ -1011,6 +1015,13 @@ get_relation_idx_constraint_oid(Oid relationId, Oid indexId)
10111015
Form_pg_constraint constrForm;
10121016

10131017
constrForm = (Form_pg_constraint) GETSTRUCT(tuple);
1018+
1019+
/* See above */
1020+
if (constrForm->contype != CONSTRAINT_PRIMARY &&
1021+
constrForm->contype != CONSTRAINT_UNIQUE &&
1022+
constrForm->contype != CONSTRAINT_EXCLUSION)
1023+
continue;
1024+
10141025
if (constrForm->conindid == indexId)
10151026
{
10161027
constraintId = constrForm->oid;

‎src/backend/commands/tablecmds.c

+13
Original file line numberDiff line numberDiff line change
@@ -9968,6 +9968,8 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
99689968
* clone those constraints to the given partition. This is to be called
99699969
* when the partition is being created or attached.
99709970
*
9971+
* This ignores self-referencing FKs; those are handled by CloneFkReferencing.
9972+
*
99719973
* This recurses to partitions, if the relation being attached is partitioned.
99729974
* Recursion is done by calling addFkRecurseReferenced.
99739975
*/
@@ -10056,6 +10058,17 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
1005610058
continue;
1005710059
}
1005810060

10061+
/*
10062+
* Don't clone self-referencing foreign keys, which can be in the
10063+
* partitioned table or in the partition-to-be.
10064+
*/
10065+
if (constrForm->conrelid == RelationGetRelid(parentRel) ||
10066+
constrForm->conrelid == RelationGetRelid(partitionRel))
10067+
{
10068+
ReleaseSysCache(tuple);
10069+
continue;
10070+
}
10071+
1005910072
/*
1006010073
* Because we're only expanding the key space at the referenced side,
1006110074
* we don't need to prevent any operation in the referencing table, so

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

+81
Original file line numberDiff line numberDiff line change
@@ -1992,6 +1992,87 @@ drop table other_partitioned_fk;
19921992
reset role;
19931993
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
19941994
drop role regress_other_partitioned_fk_owner;
1995+
--
1996+
-- Test self-referencing foreign key with partition.
1997+
-- This should create only one fk constraint per partition
1998+
--
1999+
CREATE TABLE parted_self_fk (
2000+
id bigint NOT NULL PRIMARY KEY,
2001+
id_abc bigint,
2002+
FOREIGN KEY (id_abc) REFERENCES parted_self_fk(id)
2003+
)
2004+
PARTITION BY RANGE (id);
2005+
CREATE TABLE part1_self_fk (
2006+
id bigint NOT NULL PRIMARY KEY,
2007+
id_abc bigint
2008+
);
2009+
ALTER TABLE parted_self_fk ATTACH PARTITION part1_self_fk FOR VALUES FROM (0) TO (10);
2010+
CREATE TABLE part2_self_fk PARTITION OF parted_self_fk FOR VALUES FROM (10) TO (20);
2011+
CREATE TABLE part3_self_fk ( -- a partitioned partition
2012+
id bigint NOT NULL PRIMARY KEY,
2013+
id_abc bigint
2014+
) PARTITION BY RANGE (id);
2015+
CREATE TABLE part32_self_fk PARTITION OF part3_self_fk FOR VALUES FROM (20) TO (30);
2016+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (20) TO (40);
2017+
CREATE TABLE part33_self_fk (
2018+
id bigint NOT NULL PRIMARY KEY,
2019+
id_abc bigint
2020+
);
2021+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
2022+
SELECT cr.relname, co.conname, co.contype, co.convalidated,
2023+
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
2024+
FROM pg_constraint co
2025+
JOIN pg_class cr ON cr.oid = co.conrelid
2026+
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
2027+
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
2028+
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2029+
ORDER BY co.contype, cr.relname, co.conname, p.conname;
2030+
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
2031+
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
2032+
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2033+
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2034+
part32_self_fk | parted_self_fk_id_abc_fkey | f | f | parted_self_fk_id_abc_fkey | t | parted_self_fk
2035+
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2036+
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2037+
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
2038+
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2039+
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2040+
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2041+
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2042+
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2043+
parted_self_fk | parted_self_fk_pkey | p | t | | |
2044+
(12 rows)
2045+
2046+
-- detach and re-attach multiple times just to ensure everything is kosher
2047+
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
2048+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2049+
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
2050+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
2051+
SELECT cr.relname, co.conname, co.contype, co.convalidated,
2052+
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
2053+
FROM pg_constraint co
2054+
JOIN pg_class cr ON cr.oid = co.conrelid
2055+
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
2056+
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
2057+
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
2058+
ORDER BY co.contype, cr.relname, co.conname, p.conname;
2059+
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
2060+
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
2061+
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2062+
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2063+
part32_self_fk | parted_self_fk_id_abc_fkey | f | f | parted_self_fk_id_abc_fkey | t | parted_self_fk
2064+
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2065+
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
2066+
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
2067+
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2068+
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2069+
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2070+
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2071+
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2072+
parted_self_fk | parted_self_fk_pkey | p | t | | |
2073+
(12 rows)
2074+
2075+
-- Leave this table around, for pg_upgrade/pg_dump tests
19952076
-- Test creating a constraint at the parent that already exists in partitions.
19962077
-- There should be no duplicated constraints, and attempts to drop the
19972078
-- constraint in partitions should raise appropriate errors.

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

+55
Original file line numberDiff line numberDiff line change
@@ -1441,6 +1441,61 @@ reset role;
14411441
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
14421442
drop role regress_other_partitioned_fk_owner;
14431443

1444+
--
1445+
-- Test self-referencing foreign key with partition.
1446+
-- This should create only one fk constraint per partition
1447+
--
1448+
CREATE TABLE parted_self_fk (
1449+
id bigint NOT NULL PRIMARY KEY,
1450+
id_abc bigint,
1451+
FOREIGN KEY (id_abc) REFERENCES parted_self_fk(id)
1452+
)
1453+
PARTITION BY RANGE (id);
1454+
CREATE TABLE part1_self_fk (
1455+
id bigint NOT NULL PRIMARY KEY,
1456+
id_abc bigint
1457+
);
1458+
ALTER TABLE parted_self_fk ATTACH PARTITION part1_self_fk FOR VALUES FROM (0) TO (10);
1459+
CREATE TABLE part2_self_fk PARTITION OF parted_self_fk FOR VALUES FROM (10) TO (20);
1460+
CREATE TABLE part3_self_fk ( -- a partitioned partition
1461+
id bigint NOT NULL PRIMARY KEY,
1462+
id_abc bigint
1463+
) PARTITION BY RANGE (id);
1464+
CREATE TABLE part32_self_fk PARTITION OF part3_self_fk FOR VALUES FROM (20) TO (30);
1465+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (20) TO (40);
1466+
CREATE TABLE part33_self_fk (
1467+
id bigint NOT NULL PRIMARY KEY,
1468+
id_abc bigint
1469+
);
1470+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
1471+
1472+
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1473+
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
1474+
FROM pg_constraint co
1475+
JOIN pg_class cr ON cr.oid = co.conrelid
1476+
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
1477+
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1478+
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1479+
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1480+
1481+
-- detach and re-attach multiple times just to ensure everything is kosher
1482+
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1483+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1484+
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1485+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1486+
1487+
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1488+
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
1489+
FROM pg_constraint co
1490+
JOIN pg_class cr ON cr.oid = co.conrelid
1491+
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
1492+
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1493+
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1494+
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1495+
1496+
-- Leave this table around, for pg_upgrade/pg_dump tests
1497+
1498+
14441499
-- Test creating a constraint at the parent that already exists in partitions.
14451500
-- There should be no duplicated constraints, and attempts to drop the
14461501
-- constraint in partitions should raise appropriate errors.

0 commit comments

Comments
 (0)