Skip to content

Commit cc733ed

Browse files
author
Álvaro Herrera
committed
Avoid bogus scans of partitions when validating FKs to partitioned tables
Validating an unvalidated foreign key that references a partitioned table would try to queue validations for each individual partition of the referenced table, but this is wrong: each individual partition would not necessarily have all the referenced rows, so errors would be raised. Avoid doing that. The pg_constraint rows that cause this to happen are only there to support the action triggers that implement the DELETE/ UPDATE actions of the FK, so no validating scan is necessary. This was an oversight in commit b663b94. An equivalent oversight exists for NOT ENFORCED constraints, which is not fixed in this commit. Author: Amul Sul <sulamul@gmail.com> Reported-by: Antonin Houska <ah@cybertec.at> Reviewed-by: jian he <jian.universality@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/26983.1748418675@localhost
1 parent 4b05ebf commit cc733ed

File tree

3 files changed

+114
-34
lines changed

3 files changed

+114
-34
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,8 @@ static void AlterConstrUpdateConstraintEntry(ATAlterConstraint *cmdcon, Relation
430430
static ObjectAddress ATExecValidateConstraint(List **wqueue,
431431
Relation rel, char *constrName,
432432
bool recurse, bool recursing, LOCKMODE lockmode);
433-
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
434-
HeapTuple contuple, LOCKMODE lockmode);
433+
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation fkrel,
434+
Oid pkrelid, HeapTuple contuple, LOCKMODE lockmode);
435435
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
436436
char *constrName, HeapTuple contuple,
437437
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -11858,16 +11858,19 @@ AttachPartitionForeignKey(List **wqueue,
1185811858
if (queueValidation)
1185911859
{
1186011860
Relation conrel;
11861+
Oid confrelid;
1186111862

1186211863
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
1186311864

1186411865
partcontup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(partConstrOid));
1186511866
if (!HeapTupleIsValid(partcontup))
1186611867
elog(ERROR, "cache lookup failed for constraint %u", partConstrOid);
1186711868

11869+
confrelid = ((Form_pg_constraint) GETSTRUCT(partcontup))->confrelid;
11870+
1186811871
/* Use the same lock as for AT_ValidateConstraint */
11869-
QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
11870-
ShareUpdateExclusiveLock);
11872+
QueueFKConstraintValidation(wqueue, conrel, partition, confrelid,
11873+
partcontup, ShareUpdateExclusiveLock);
1187111874
ReleaseSysCache(partcontup);
1187211875
table_close(conrel, RowExclusiveLock);
1187311876
}
@@ -12919,7 +12922,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
1291912922
{
1292012923
if (con->contype == CONSTRAINT_FOREIGN)
1292112924
{
12922-
QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode);
12925+
QueueFKConstraintValidation(wqueue, conrel, rel, con->confrelid,
12926+
tuple, lockmode);
1292312927
}
1292412928
else if (con->contype == CONSTRAINT_CHECK)
1292512929
{
@@ -12952,8 +12956,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
1295212956
* for the specified relation and all its children.
1295312957
*/
1295412958
static void
12955-
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
12956-
HeapTuple contuple, LOCKMODE lockmode)
12959+
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation fkrel,
12960+
Oid pkrelid, HeapTuple contuple, LOCKMODE lockmode)
1295712961
{
1295812962
Form_pg_constraint con;
1295912963
AlteredTableInfo *tab;
@@ -12964,7 +12968,17 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
1296412968
Assert(con->contype == CONSTRAINT_FOREIGN);
1296512969
Assert(!con->convalidated);
1296612970

12967-
if (rel->rd_rel->relkind == RELKIND_RELATION)
12971+
/*
12972+
* Add the validation to phase 3's queue; not needed for partitioned
12973+
* tables themselves, only for their partitions.
12974+
*
12975+
* When the referenced table (pkrelid) is partitioned, the referencing
12976+
* table (fkrel) has one pg_constraint row pointing to each partition
12977+
* thereof. These rows are there only to support action triggers and no
12978+
* table scan is needed, therefore skip this for them as well.
12979+
*/
12980+
if (fkrel->rd_rel->relkind == RELKIND_RELATION &&
12981+
con->confrelid == pkrelid)
1296812982
{
1296912983
NewConstraint *newcon;
1297012984
Constraint *fkconstraint;
@@ -12983,15 +12997,16 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
1298312997
newcon->qual = (Node *) fkconstraint;
1298412998

1298512999
/* Find or create work queue entry for this table */
12986-
tab = ATGetQueueEntry(wqueue, rel);
13000+
tab = ATGetQueueEntry(wqueue, fkrel);
1298713001
tab->constraints = lappend(tab->constraints, newcon);
1298813002
}
1298913003

1299013004
/*
1299113005
* If the table at either end of the constraint is partitioned, we need to
12992-
* recurse and handle every constraint that is a child of this constraint.
13006+
* recurse and handle every unvalidate constraint that is a child of this
13007+
* constraint.
1299313008
*/
12994-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
13009+
if (fkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
1299513010
get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
1299613011
{
1299713012
ScanKeyData pkey;
@@ -13023,16 +13038,24 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
1302313038

1302413039
childrel = table_open(childcon->conrelid, lockmode);
1302513040

13026-
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
13027-
lockmode);
13041+
/*
13042+
* NB: Note that pkrelid should be passed as-is during recursion,
13043+
* as it is required to identify the root referenced table.
13044+
*/
13045+
QueueFKConstraintValidation(wqueue, conrel, childrel, pkrelid,
13046+
childtup, lockmode);
1302813047
table_close(childrel, NoLock);
1302913048
}
1303013049

1303113050
systable_endscan(pscan);
1303213051
}
1303313052

1303413053
/*
13035-
* Now update the catalog, while we have the door open.
13054+
* Now mark the pg_constraint row as validated (even if we didn't check,
13055+
* notably the ones for partitions on the referenced side).
13056+
*
13057+
* We rely on transaction abort to roll back this change if phase 3
13058+
* ultimately finds violating rows. This is a bit ugly.
1303613059
*/
1303713060
copyTuple = heap_copytuple(contuple);
1303813061
copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);

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

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1895,29 +1895,67 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
18951895
(5 rows)
18961896

18971897
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
1898-
-- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
1898+
-- NOT VALID foreign key on a non-partitioned table referencing a partitioned
1899+
-- table
18991900
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
19001901
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
1902+
CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
19011903
CREATE TABLE fk_notpartitioned_fk (b int, a int);
1902-
ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
1903-
-- Constraint will be invalid.
1904-
SELECT conname, convalidated FROM pg_constraint
1904+
INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
1905+
INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
1906+
ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
1907+
FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
1908+
-- All constraints will be invalid.
1909+
SELECT conname, conenforced, convalidated FROM pg_constraint
19051910
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
1906-
conname | convalidated
1907-
---------------------------------+--------------
1908-
fk_notpartitioned_fk_a_b_fkey | f
1909-
fk_notpartitioned_fk_a_b_fkey_1 | f
1910-
(2 rows)
1911+
conname | conenforced | convalidated
1912+
---------------------------------+-------------+--------------
1913+
fk_notpartitioned_fk_a_b_fkey | t | f
1914+
fk_notpartitioned_fk_a_b_fkey_1 | t | f
1915+
fk_notpartitioned_fk_a_b_fkey_2 | t | f
1916+
(3 rows)
19111917

19121918
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
19131919
-- All constraints are now valid.
1914-
SELECT conname, convalidated FROM pg_constraint
1920+
SELECT conname, conenforced, convalidated FROM pg_constraint
19151921
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
1916-
conname | convalidated
1917-
---------------------------------+--------------
1918-
fk_notpartitioned_fk_a_b_fkey | t
1919-
fk_notpartitioned_fk_a_b_fkey_1 | t
1920-
(2 rows)
1922+
conname | conenforced | convalidated
1923+
---------------------------------+-------------+--------------
1924+
fk_notpartitioned_fk_a_b_fkey | t | t
1925+
fk_notpartitioned_fk_a_b_fkey_1 | t | t
1926+
fk_notpartitioned_fk_a_b_fkey_2 | t | t
1927+
(3 rows)
1928+
1929+
-- test a self-referential FK
1930+
ALTER TABLE fk_partitioned_pk ADD CONSTRAINT selffk FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
1931+
CREATE TABLE fk_partitioned_pk_3 PARTITION OF fk_partitioned_pk FOR VALUES FROM (2000,2000) TO (3000,3000)
1932+
PARTITION BY RANGE (a);
1933+
CREATE TABLE fk_partitioned_pk_3_1 PARTITION OF fk_partitioned_pk_3 FOR VALUES FROM (2000) TO (2100);
1934+
SELECT conname, conenforced, convalidated FROM pg_constraint
1935+
WHERE conrelid = 'fk_partitioned_pk'::regclass AND contype = 'f'
1936+
ORDER BY oid::regclass::text;
1937+
conname | conenforced | convalidated
1938+
------------+-------------+--------------
1939+
selffk | t | f
1940+
selffk_1 | t | f
1941+
selffk_2 | t | f
1942+
selffk_3 | t | f
1943+
selffk_3_1 | t | f
1944+
(5 rows)
1945+
1946+
ALTER TABLE fk_partitioned_pk_2 VALIDATE CONSTRAINT selffk;
1947+
ALTER TABLE fk_partitioned_pk VALIDATE CONSTRAINT selffk;
1948+
SELECT conname, conenforced, convalidated FROM pg_constraint
1949+
WHERE conrelid = 'fk_partitioned_pk'::regclass AND contype = 'f'
1950+
ORDER BY oid::regclass::text;
1951+
conname | conenforced | convalidated
1952+
------------+-------------+--------------
1953+
selffk | t | t
1954+
selffk_1 | t | t
1955+
selffk_2 | t | t
1956+
selffk_3 | t | t
1957+
selffk_3_1 | t | t
1958+
(5 rows)
19211959

19221960
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
19231961
-- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE

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

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,22 +1389,41 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
13891389

13901390
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
13911391

1392-
-- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
1392+
-- NOT VALID foreign key on a non-partitioned table referencing a partitioned
1393+
-- table
13931394
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
13941395
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
1396+
CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
13951397
CREATE TABLE fk_notpartitioned_fk (b int, a int);
1396-
ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
1398+
INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
1399+
INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
1400+
ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
1401+
FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
13971402

1398-
-- Constraint will be invalid.
1399-
SELECT conname, convalidated FROM pg_constraint
1403+
-- All constraints will be invalid.
1404+
SELECT conname, conenforced, convalidated FROM pg_constraint
14001405
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
14011406

14021407
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
14031408

14041409
-- All constraints are now valid.
1405-
SELECT conname, convalidated FROM pg_constraint
1410+
SELECT conname, conenforced, convalidated FROM pg_constraint
14061411
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
14071412

1413+
-- test a self-referential FK
1414+
ALTER TABLE fk_partitioned_pk ADD CONSTRAINT selffk FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
1415+
CREATE TABLE fk_partitioned_pk_3 PARTITION OF fk_partitioned_pk FOR VALUES FROM (2000,2000) TO (3000,3000)
1416+
PARTITION BY RANGE (a);
1417+
CREATE TABLE fk_partitioned_pk_3_1 PARTITION OF fk_partitioned_pk_3 FOR VALUES FROM (2000) TO (2100);
1418+
SELECT conname, conenforced, convalidated FROM pg_constraint
1419+
WHERE conrelid = 'fk_partitioned_pk'::regclass AND contype = 'f'
1420+
ORDER BY oid::regclass::text;
1421+
ALTER TABLE fk_partitioned_pk_2 VALIDATE CONSTRAINT selffk;
1422+
ALTER TABLE fk_partitioned_pk VALIDATE CONSTRAINT selffk;
1423+
SELECT conname, conenforced, convalidated FROM pg_constraint
1424+
WHERE conrelid = 'fk_partitioned_pk'::regclass AND contype = 'f'
1425+
ORDER BY oid::regclass::text;
1426+
14081427
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
14091428

14101429
-- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE

0 commit comments

Comments
 (0)