Skip to content

Commit 483d269

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 b93d7e6 commit 483d269

File tree

4 files changed

+161
-1
lines changed

4 files changed

+161
-1
lines changed

‎src/backend/catalog/pg_constraint.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -973,8 +973,12 @@ get_relation_constraint_attnos(Oid relid, const char *conname,
973973
}
974974

975975
/*
976-
* Return the OID of the constraint associated with the given index in the
976+
* Return the OID of the constraint enforced by the given index in the
977977
* given relation; or InvalidOid if no such index is catalogued.
978+
*
979+
* Much like get_constraint_index, this function is concerned only with the
980+
* one constraint that "owns" the given index. Therefore, constraints of
981+
* types other than unique, primary-key, and exclusion are ignored.
978982
*/
979983
Oid
980984
get_relation_idx_constraint_oid(Oid relationId, Oid indexId)
@@ -999,6 +1003,13 @@ get_relation_idx_constraint_oid(Oid relationId, Oid indexId)
9991003
Form_pg_constraint constrForm;
10001004

10011005
constrForm = (Form_pg_constraint) GETSTRUCT(tuple);
1006+
1007+
/* See above */
1008+
if (constrForm->contype != CONSTRAINT_PRIMARY &&
1009+
constrForm->contype != CONSTRAINT_UNIQUE &&
1010+
constrForm->contype != CONSTRAINT_EXCLUSION)
1011+
continue;
1012+
10021013
if (constrForm->conindid == indexId)
10031014
{
10041015
constraintId = constrForm->oid;

‎src/backend/commands/tablecmds.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9749,6 +9749,8 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel,
97499749
* clone those constraints to the given partition. This is to be called
97509750
* when the partition is being created or attached.
97519751
*
9752+
* This ignores self-referencing FKs; those are handled by CloneFkReferencing.
9753+
*
97529754
* This recurses to partitions, if the relation being attached is partitioned.
97539755
* Recursion is done by calling addFkRecurseReferenced.
97549756
*/
@@ -9824,6 +9826,17 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
98249826
continue;
98259827
}
98269828

9829+
/*
9830+
* Don't clone self-referencing foreign keys, which can be in the
9831+
* partitioned table or in the partition-to-be.
9832+
*/
9833+
if (constrForm->conrelid == RelationGetRelid(parentRel) ||
9834+
constrForm->conrelid == RelationGetRelid(partitionRel))
9835+
{
9836+
ReleaseSysCache(tuple);
9837+
continue;
9838+
}
9839+
98279840
/*
98289841
* Because we're only expanding the key space at the referenced side,
98299842
* we don't need to prevent any operation in the referencing table, so

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1921,6 +1921,87 @@ drop table other_partitioned_fk;
19211921
reset role;
19221922
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
19231923
drop role regress_other_partitioned_fk_owner;
1924+
--
1925+
-- Test self-referencing foreign key with partition.
1926+
-- This should create only one fk constraint per partition
1927+
--
1928+
CREATE TABLE parted_self_fk (
1929+
id bigint NOT NULL PRIMARY KEY,
1930+
id_abc bigint,
1931+
FOREIGN KEY (id_abc) REFERENCES parted_self_fk(id)
1932+
)
1933+
PARTITION BY RANGE (id);
1934+
CREATE TABLE part1_self_fk (
1935+
id bigint NOT NULL PRIMARY KEY,
1936+
id_abc bigint
1937+
);
1938+
ALTER TABLE parted_self_fk ATTACH PARTITION part1_self_fk FOR VALUES FROM (0) TO (10);
1939+
CREATE TABLE part2_self_fk PARTITION OF parted_self_fk FOR VALUES FROM (10) TO (20);
1940+
CREATE TABLE part3_self_fk ( -- a partitioned partition
1941+
id bigint NOT NULL PRIMARY KEY,
1942+
id_abc bigint
1943+
) PARTITION BY RANGE (id);
1944+
CREATE TABLE part32_self_fk PARTITION OF part3_self_fk FOR VALUES FROM (20) TO (30);
1945+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (20) TO (40);
1946+
CREATE TABLE part33_self_fk (
1947+
id bigint NOT NULL PRIMARY KEY,
1948+
id_abc bigint
1949+
);
1950+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
1951+
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1952+
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
1953+
FROM pg_constraint co
1954+
JOIN pg_class cr ON cr.oid = co.conrelid
1955+
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
1956+
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1957+
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1958+
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1959+
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
1960+
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
1961+
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1962+
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1963+
part32_self_fk | parted_self_fk_id_abc_fkey | f | f | parted_self_fk_id_abc_fkey | t | parted_self_fk
1964+
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1965+
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1966+
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
1967+
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
1968+
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
1969+
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
1970+
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
1971+
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
1972+
parted_self_fk | parted_self_fk_pkey | p | t | | |
1973+
(12 rows)
1974+
1975+
-- detach and re-attach multiple times just to ensure everything is kosher
1976+
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1977+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1978+
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1979+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1980+
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1981+
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
1982+
FROM pg_constraint co
1983+
JOIN pg_class cr ON cr.oid = co.conrelid
1984+
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
1985+
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1986+
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1987+
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1988+
relname | conname | contype | convalidated | conparent | convalidated | foreignrel
1989+
----------------+----------------------------+---------+--------------+----------------------------+--------------+----------------
1990+
part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1991+
part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1992+
part32_self_fk | parted_self_fk_id_abc_fkey | f | f | parted_self_fk_id_abc_fkey | t | parted_self_fk
1993+
part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1994+
part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk
1995+
parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk
1996+
part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t |
1997+
part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t |
1998+
part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t |
1999+
part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t |
2000+
part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t |
2001+
parted_self_fk | parted_self_fk_pkey | p | t | | |
2002+
(12 rows)
2003+
2004+
-- Leave this table around, for pg_upgrade/pg_dump tests
19242005
-- Test creating a constraint at the parent that already exists in partitions.
19252006
-- There should be no duplicated constraints, and attempts to drop the
19262007
-- constraint in partitions should raise appropriate errors.

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,6 +1390,61 @@ reset role;
13901390
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
13911391
drop role regress_other_partitioned_fk_owner;
13921392

1393+
--
1394+
-- Test self-referencing foreign key with partition.
1395+
-- This should create only one fk constraint per partition
1396+
--
1397+
CREATE TABLE parted_self_fk (
1398+
id bigint NOT NULL PRIMARY KEY,
1399+
id_abc bigint,
1400+
FOREIGN KEY (id_abc) REFERENCES parted_self_fk(id)
1401+
)
1402+
PARTITION BY RANGE (id);
1403+
CREATE TABLE part1_self_fk (
1404+
id bigint NOT NULL PRIMARY KEY,
1405+
id_abc bigint
1406+
);
1407+
ALTER TABLE parted_self_fk ATTACH PARTITION part1_self_fk FOR VALUES FROM (0) TO (10);
1408+
CREATE TABLE part2_self_fk PARTITION OF parted_self_fk FOR VALUES FROM (10) TO (20);
1409+
CREATE TABLE part3_self_fk ( -- a partitioned partition
1410+
id bigint NOT NULL PRIMARY KEY,
1411+
id_abc bigint
1412+
) PARTITION BY RANGE (id);
1413+
CREATE TABLE part32_self_fk PARTITION OF part3_self_fk FOR VALUES FROM (20) TO (30);
1414+
ALTER TABLE parted_self_fk ATTACH PARTITION part3_self_fk FOR VALUES FROM (20) TO (40);
1415+
CREATE TABLE part33_self_fk (
1416+
id bigint NOT NULL PRIMARY KEY,
1417+
id_abc bigint
1418+
);
1419+
ALTER TABLE part3_self_fk ATTACH PARTITION part33_self_fk FOR VALUES FROM (30) TO (40);
1420+
1421+
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1422+
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
1423+
FROM pg_constraint co
1424+
JOIN pg_class cr ON cr.oid = co.conrelid
1425+
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
1426+
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1427+
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1428+
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1429+
1430+
-- detach and re-attach multiple times just to ensure everything is kosher
1431+
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1432+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1433+
ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk;
1434+
ALTER TABLE parted_self_fk ATTACH PARTITION part2_self_fk FOR VALUES FROM (10) TO (20);
1435+
1436+
SELECT cr.relname, co.conname, co.contype, co.convalidated,
1437+
p.conname AS conparent, p.convalidated, cf.relname AS foreignrel
1438+
FROM pg_constraint co
1439+
JOIN pg_class cr ON cr.oid = co.conrelid
1440+
LEFT JOIN pg_class cf ON cf.oid = co.confrelid
1441+
LEFT JOIN pg_constraint p ON p.oid = co.conparentid
1442+
WHERE cr.oid IN (SELECT relid FROM pg_partition_tree('parted_self_fk'))
1443+
ORDER BY co.contype, cr.relname, co.conname, p.conname;
1444+
1445+
-- Leave this table around, for pg_upgrade/pg_dump tests
1446+
1447+
13931448
-- Test creating a constraint at the parent that already exists in partitions.
13941449
-- There should be no duplicated constraints, and attempts to drop the
13951450
-- constraint in partitions should raise appropriate errors.

0 commit comments

Comments
 (0)