Hello
I find this coding somewhat confusing. Now you have a function
"QueueFkConstraintValidation" which may queue a FK queue constraint
validation, if the flag "queueValidation" is given, but it may also do
something else -- makes no sense IMO. I think it's better to split this
function in two; one does indeed validate, the other is a simple helper
that only sets the catalog flag, and does its own recursion. This
causes a bit of duplicate code, but is simpler to follow. (We could
also refactor the duplicate code by adding another helper routine, but
I'm not sure it's worth the trouble.)
I would propose something like this instead, what do you think?
I hate that pg_constraint.conparentid doesn't have a syscache or at
least an index :-(
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"El miedo atento y previsor es la madre de la seguridad" (E. Burke)
>From 16437c21eca0bb15e34ba6c7c951daabb280fcec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@kurilemu.de>
Date: Mon, 2 Jun 2025 18:19:13 +0200
Subject: [PATCH v6] Skip adding action-based foreign key constraints to the
phase 3 validation queue.
When the referenced table is partitioned, each partition will have its
own action trigger constraint to handle the appropriate behavior for
UPDATE or DELETE operations. However, these constraints should not be
used to validate data in the referencing table, as the referenced data
may be distributed across multiple partitions. Attempting validation
against a single partition could result in errors. Therefore,
validation of referencing table data should always be performed
against the referenced root partitioned table.
---
src/backend/commands/tablecmds.c | 113 ++++++++++++++++++++--
src/test/regress/expected/foreign_key.out | 47 ++++++---
src/test/regress/sql/foreign_key.sql | 19 ++--
3 files changed, 148 insertions(+), 31 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index acf11e83c04..84e4476e5c6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -432,6 +432,7 @@ static ObjectAddress ATExecValidateConstraint(List **wqueue,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
HeapTuple contuple, LOCKMODE lockmode);
+static void markChildFKConstraintValidated(Relation conrel, HeapTuple contuple, Relation rel);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -12464,8 +12465,13 @@ ATExecAlterConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
/*
* Tell Phase 3 to check that the constraint is satisfied by existing
* rows.
+ *
+ * Note that validation should be performed against the referenced root
+ * table only, not its child partitions. See
+ * QueueFKConstraintValidation() for more details.
*/
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ if (rel->rd_rel->relkind == RELKIND_RELATION &&
+ currcon->confrelid == pkrelid)
{
AlteredTableInfo *tab;
NewConstraint *newcon;
@@ -12964,6 +12970,11 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
Assert(con->contype == CONSTRAINT_FOREIGN);
Assert(!con->convalidated);
+ /*
+ * If we're processing a plain relation, it needs to be scanned by phase 3
+ * in order for the constraint to be validated. Find or create its work
+ * queue entry, and attach the constraint information to it.
+ */
if (rel->rd_rel->relkind == RELKIND_RELATION)
{
NewConstraint *newcon;
@@ -12988,8 +12999,15 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
}
/*
- * If the table at either end of the constraint is partitioned, we need to
- * recurse and handle every constraint that is a child of this constraint.
+ * If the referencing table is partitioned, we need to scan every
+ * partition to report any potentially violating rows.
+ *
+ * If the referenced table is partitioned, we don't need to scan the
+ * partitions, but we need to mark those child constraint rows as
+ * validated. (This is just so that catalog state is consistent.)
+ *
+ * XXX this requires a seqscan of pg_constraint for each partitioning
+ * level.
*/
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
@@ -13009,23 +13027,100 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
while (HeapTupleIsValid(childtup = systable_getnext(pscan)))
{
Form_pg_constraint childcon;
- Relation childrel;
childcon = (Form_pg_constraint) GETSTRUCT(childtup);
/*
- * If the child constraint has already been validated, no further
+ * If the child constraint is already marked validated, no further
* action is required for it or its descendants, as they are all
* valid.
*/
if (childcon->convalidated)
continue;
- childrel = table_open(childcon->conrelid, lockmode);
+ if (childcon->conrelid == RelationGetRelid(rel))
+ {
+ markChildFKConstraintValidated(conrel, childtup, rel);
+ }
+ else
+ {
+ Relation childrel;
- QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
- table_close(childrel, NoLock);
+ childrel = table_open(childcon->conrelid, lockmode);
+
+ QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
+ lockmode);
+
+ /* We have skip opening the table if it is the same input rel */
+ table_close(childrel, NoLock);
+ }
+ }
+
+ systable_endscan(pscan);
+ }
+
+ /*
+ * Now update the catalog, while we have the door open.
+ */
+ copyTuple = heap_copytuple(contuple);
+ copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
+ copy_con->convalidated = true;
+ CatalogTupleUpdate(conrel, ©Tuple->t_self, copyTuple);
+
+ InvokeObjectPostAlterHook(ConstraintRelationId, con->oid, 0);
+
+ heap_freetuple(copyTuple);
+}
+
+/*
+ * Helper for QueueFKConstraintValidation: recurse on the referenced-side
+ * relation, when it is a partitioned table.
+ */
+static void
+markChildFKConstraintValidated(Relation conrel, HeapTuple contuple, Relation rel)
+{
+ Form_pg_constraint con;
+ HeapTuple copyTuple;
+ Form_pg_constraint copy_con;
+
+ con = (Form_pg_constraint) GETSTRUCT(contuple);
+ Assert(con->contype == CONSTRAINT_FOREIGN);
+ Assert(!con->convalidated);
+ Assert(con->conrelid = RelationGetRelid(rel));
+
+ /*
+ * If this child refenced table is also partitioned, then we must process
+ * the constraints on its own partitions as well. Descend the hierarchy
+ * using the conparentid link.
+ *
+ * XXX this requires a seqscan of pg_constraint for each partitioning
+ * level.
+ */
+ if (get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
+ {
+ ScanKeyData pkey;
+ SysScanDesc pscan;
+ HeapTuple childtup;
+
+ ScanKeyInit(&pkey,
+ Anum_pg_constraint_conparentid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(con->oid));
+
+ pscan = systable_beginscan(conrel, ConstraintParentIndexId,
+ true, NULL, 1, &pkey);
+
+ while (HeapTupleIsValid(childtup = systable_getnext(pscan)))
+ {
+ /*
+ * If the child constraint has already been validated, no further
+ * action is required for it or its descendants, as they are all
+ * valid.
+ */
+ if (((Form_pg_constraint) GETSTRUCT(childtup))->convalidated)
+ continue;
+
+ markChildFKConstraintValidated(conrel, childtup, rel);
}
systable_endscan(pscan);
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 4f3f280a439..8050a3c4aa3 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1895,29 +1895,44 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
(5 rows)
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | f
- fk_notpartitioned_fk_a_b_fkey_1 | f
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | f
+ fk_notpartitioned_fk_a_b_fkey_1 | t | f
+ fk_notpartitioned_fk_a_b_fkey_2 | t | f
+ fk_notpartitioned_fk_a_b_fkey2 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_1 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_2 | f | f
+(6 rows)
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | t
- fk_notpartitioned_fk_a_b_fkey_1 | t
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | t
+ fk_notpartitioned_fk_a_b_fkey_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey_2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_2 | t | t
+(6 rows)
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
-- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 8159e363022..aaeb0f49694 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1389,20 +1389,27 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
--
2.39.5
