Skip to content

Commit 2c80a65

Browse files
alvherrealvherre
committed
Fix failure to create FKs correctly in partitions
On a multi-level partioned table, when adding a partition not directly connected to the root table, foreign key constraints referencing the root were not cloned to the new partition, leading to the FK being possibly inadvertently violated later on. This was caused by fuzzy thinking in CloneFkReferenced (commit f56f8f8): it was skipping constraints marked as having parents on the theory that cloning those would create duplicates; but that's only correct for the top level of the partitioning hierarchy. For levels below that one, such constraints must still be considered and only skipped if later on we see that we'd create duplicates. Apparently, I (Álvaro) wrote the comments right but the code implemented something slightly different. Author: Jehan-Guillaume de Rorthais Discussion: https://postgr.es/m/20200206004948.238352db@firost
1 parent ce054a8 commit 2c80a65

File tree

3 files changed

+59
-11
lines changed

3 files changed

+59
-11
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8549,13 +8549,13 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
85498549
List *clone = NIL;
85508550

85518551
/*
8552-
* Search for any constraints where this partition is in the referenced
8553-
* side. However, we must ignore any constraint whose parent constraint
8554-
* is also going to be cloned, to avoid duplicates. So do it in two
8555-
* steps: first construct the list of constraints to clone, then go over
8556-
* that list cloning those whose parents are not in the list. (We must
8557-
* not rely on the parent being seen first, since the catalog scan could
8558-
* return children first.)
8552+
* Search for any constraints where this partition's parent is in the
8553+
* referenced side. However, we must not clone any constraint whose
8554+
* parent constraint is also going to be cloned, to avoid duplicates. So
8555+
* do it in two steps: first construct the list of constraints to clone,
8556+
* then go over that list cloning those whose parents are not in the list.
8557+
* (We must not rely on the parent being seen first, since the catalog
8558+
* scan could return children first.)
85598559
*/
85608560
pg_constraint = table_open(ConstraintRelationId, RowShareLock);
85618561
ScanKeyInit(&key[0],
@@ -8571,10 +8571,6 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
85718571
{
85728572
Form_pg_constraint constrForm = (Form_pg_constraint) GETSTRUCT(tuple);
85738573

8574-
/* Only try to clone the top-level constraint; skip child ones. */
8575-
if (constrForm->conparentid != InvalidOid)
8576-
continue;
8577-
85788574
clone = lappend_oid(clone, constrForm->oid);
85798575
}
85808576
systable_endscan(scan);
@@ -8604,6 +8600,16 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
86048600
elog(ERROR, "cache lookup failed for constraint %u", constrOid);
86058601
constrForm = (Form_pg_constraint) GETSTRUCT(tuple);
86068602

8603+
/*
8604+
* As explained above: don't try to clone a constraint for which we're
8605+
* going to clone the parent.
8606+
*/
8607+
if (list_member_oid(clone, constrForm->conparentid))
8608+
{
8609+
ReleaseSysCache(tuple);
8610+
continue;
8611+
}
8612+
86078613
/*
86088614
* Because we're only expanding the key space at the referenced side,
86098615
* we don't need to prevent any operation in the referencing table, so

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2444,3 +2444,27 @@ DROP SCHEMA fkpart8 CASCADE;
24442444
NOTICE: drop cascades to 2 other objects
24452445
DETAIL: drop cascades to table fkpart8.tbl1
24462446
drop cascades to table fkpart8.tbl2
2447+
-- ensure FK referencing a multi-level partitioned table are
2448+
-- enforce reference to sub-children.
2449+
CREATE SCHEMA fkpart9
2450+
CREATE TABLE pk (a INT PRIMARY KEY) PARTITION BY RANGE (a)
2451+
CREATE TABLE fk (
2452+
fk_a INT REFERENCES pk(a) ON DELETE CASCADE
2453+
)
2454+
CREATE TABLE pk1 PARTITION OF pk FOR VALUES FROM (30) TO (50) PARTITION BY RANGE (a)
2455+
CREATE TABLE pk11 PARTITION OF pk1 FOR VALUES FROM (30) TO (40);
2456+
INSERT INTO fkpart9.pk VALUES (35);
2457+
INSERT INTO fkpart9.fk VALUES (35);
2458+
DELETE FROM fkpart9.pk WHERE a=35;
2459+
SELECT fk.fk_a, pk.a
2460+
FROM fkpart9.fk
2461+
LEFT JOIN fkpart9.pk ON fk.fk_a = pk.a
2462+
WHERE fk.fk_a=35;
2463+
fk_a | a
2464+
------+---
2465+
(0 rows)
2466+
2467+
DROP SCHEMA fkpart9 CASCADE;
2468+
NOTICE: drop cascades to 2 other objects
2469+
DETAIL: drop cascades to table fkpart9.pk
2470+
drop cascades to table fkpart9.fk

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,3 +1722,21 @@ INSERT INTO fkpart8.tbl2 VALUES(1);
17221722
ALTER TABLE fkpart8.tbl2 DROP CONSTRAINT tbl2_f1_fkey;
17231723
COMMIT;
17241724
DROP SCHEMA fkpart8 CASCADE;
1725+
1726+
-- ensure FK referencing a multi-level partitioned table are
1727+
-- enforce reference to sub-children.
1728+
CREATE SCHEMA fkpart9
1729+
CREATE TABLE pk (a INT PRIMARY KEY) PARTITION BY RANGE (a)
1730+
CREATE TABLE fk (
1731+
fk_a INT REFERENCES pk(a) ON DELETE CASCADE
1732+
)
1733+
CREATE TABLE pk1 PARTITION OF pk FOR VALUES FROM (30) TO (50) PARTITION BY RANGE (a)
1734+
CREATE TABLE pk11 PARTITION OF pk1 FOR VALUES FROM (30) TO (40);
1735+
INSERT INTO fkpart9.pk VALUES (35);
1736+
INSERT INTO fkpart9.fk VALUES (35);
1737+
DELETE FROM fkpart9.pk WHERE a=35;
1738+
SELECT fk.fk_a, pk.a
1739+
FROM fkpart9.fk
1740+
LEFT JOIN fkpart9.pk ON fk.fk_a = pk.a
1741+
WHERE fk.fk_a=35;
1742+
DROP SCHEMA fkpart9 CASCADE;

0 commit comments

Comments
 (0)