On Sun, Jun 1, 2025 at 6:05 PM jian he <jian.universal...@gmail.com> wrote:
>
> On Fri, May 30, 2025 at 6:32 PM Amul Sul <sula...@gmail.com> wrote:
> >
[...]
>
> + * Note that validation should be performed against the referencing
> + * root table only, not its child partitions. See
> + * QueueFKConstraintValidation() for more details.
> */
> if (rel->rd_rel->relkind == RELKIND_RELATION &&
> currcon->confrelid == pkrelid)
> {
> AlteredTableInfo *tab;
> NewConstraint *newcon;
> newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
> ....
> }
>
> in the comments "referencing" should be "referenced"?
>
Yes, fixed in the attached version.
> other than that, it looks good.
>
Thanks for the review.
Regards,
Amul
From d580da019e88aa953a53f937ccff89432d832a3c Mon Sep 17 00:00:00 2001
From: Amul Sul <sulamul@gmail.com>
Date: Mon, 2 Jun 2025 08:44:26 +0530
Subject: [PATCH v5] 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 | 55 +++++++++++++++++++----
src/test/regress/expected/foreign_key.out | 47 ++++++++++++-------
src/test/regress/sql/foreign_key.sql | 19 +++++---
3 files changed, 90 insertions(+), 31 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index acf11e83c04..0ed637fa5de 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -431,7 +431,8 @@ static ObjectAddress ATExecValidateConstraint(List **wqueue,
Relation rel, char *constrName,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode);
+ HeapTuple contuple, bool queueValidation,
+ LOCKMODE lockmode);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -11867,7 +11868,7 @@ AttachPartitionForeignKey(List **wqueue,
/* Use the same lock as for AT_ValidateConstraint */
QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
- ShareUpdateExclusiveLock);
+ true, ShareUpdateExclusiveLock);
ReleaseSysCache(partcontup);
table_close(conrel, RowExclusiveLock);
}
@@ -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;
@@ -12919,7 +12925,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
{
if (con->contype == CONSTRAINT_FOREIGN)
{
- QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode);
+ QueueFKConstraintValidation(wqueue, conrel, rel, tuple, true,
+ lockmode);
}
else if (con->contype == CONSTRAINT_CHECK)
{
@@ -12953,7 +12960,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
*/
static void
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode)
+ HeapTuple contuple, bool queueValidation,
+ LOCKMODE lockmode)
{
Form_pg_constraint con;
AlteredTableInfo *tab;
@@ -12964,7 +12972,13 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
Assert(con->contype == CONSTRAINT_FOREIGN);
Assert(!con->convalidated);
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ /*
+ * Sometimes we only want to update the pg_constraint entry without
+ * enqueueing it for validation; in such cases, queueValidation will be
+ * false.
+ */
+ if (queueValidation &&
+ rel->rd_rel->relkind == RELKIND_RELATION)
{
NewConstraint *newcon;
Constraint *fkconstraint;
@@ -13010,6 +13024,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
{
Form_pg_constraint childcon;
Relation childrel;
+ bool childValidation = true;
childcon = (Form_pg_constraint) GETSTRUCT(childtup);
@@ -13021,11 +13036,33 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
if (childcon->convalidated)
continue;
- childrel = table_open(childcon->conrelid, lockmode);
+ /*
+ * When the referenced table is partitioned, the referencing table may have
+ * multiple foreign key constraints for action triggers -- one for each child
+ * partition. These are action-trigger constraints intended to enforce the
+ * appropriate behavior when UPDATE or DELETE operations are performed on the
+ * referenced partitions. See createForeignKeyActionTriggers() for more details.
+ *
+ * These constraints should not be used for validating data in the referencing
+ * table. This is because referenced key values may be distributed across
+ * multiple partitions, and therefore, validation should be performed only
+ * through the root referenced table.
+ */
+ if (childcon->conrelid == RelationGetRelid(rel))
+ {
+ /* No need to open it again */
+ childrel = rel;
+ childValidation = false;
+ }
+ else
+ childrel = table_open(childcon->conrelid, lockmode);
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
- table_close(childrel, NoLock);
+ childValidation, lockmode);
+
+ /* We have skip opening the table if it is the same input rel */
+ if (childcon->conrelid != RelationGetRelid(rel))
+ table_close(childrel, NoLock);
}
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.43.5
