v11-0007-Merge-the-parent-and-child-constraints-with-diff.patch

From 9271dc59ce3f03bf285f6d136220dad4f8b10a11 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Fri, 17 Jan 2025 17:03:59 +0530
Subject: [PATCH v11 7/7] Merge the parent and child constraints with differing
 enforcibility.

If an enforced parent constraint is attached to a non-enforced child
constraint, the child constraint will be made enforced, with
validation applied if the parent constraint is validated as well.
Otherwise, a new enforced constraint (with validation, if the parent
constraint is validated) would need to be created on the child table,
which would be unnecessary if a similar constraint already exists and
can be attached.

On the other hand, having a not-enforced parent constraint with an
enforced child constraint does not cause any issues, and no changes
are required.

----
NOTE: This patch is intended to reduce the diff noise from the main
patch and is not meant to be committed separately. It should be
squashed with the main patch that adds ENFORCED/NOT ENFORCED.
----
---
 src/backend/commands/tablecmds.c          | 157 ++++++++++++++++++++--
 src/test/regress/expected/foreign_key.out |  36 ++++-
 src/test/regress/sql/foreign_key.sql      |  14 ++
 3 files changed, 191 insertions(+), 16 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 00cf56d3fc9..0ba8e5422c3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -11435,7 +11435,6 @@ tryAttachPartitionForeignKey(List **wqueue,
 	if (OidIsValid(partConstr->conparentid) ||
 		partConstr->condeferrable != parentConstr->condeferrable ||
 		partConstr->condeferred != parentConstr->condeferred ||
-		partConstr->conenforced != parentConstr->conenforced ||
 		partConstr->confupdtype != parentConstr->confupdtype ||
 		partConstr->confdeltype != parentConstr->confdeltype ||
 		partConstr->confmatchtype != parentConstr->confmatchtype)
@@ -11480,6 +11479,8 @@ AttachPartitionForeignKey(List **wqueue,
 	Oid			partConstrFrelid;
 	Oid			partConstrRelid;
 	bool		parentConstrIsEnforced;
+	bool		partConstrIsEnforced;
+	bool		partConstrParentIsSet;
 	Oid			insertTriggerOid,
 				updateTriggerOid;
 
@@ -11497,9 +11498,79 @@ AttachPartitionForeignKey(List **wqueue,
 	if (!HeapTupleIsValid(partcontup))
 		elog(ERROR, "cache lookup failed for constraint %u", partConstrOid);
 	partConstr = (Form_pg_constraint) GETSTRUCT(partcontup);
+	partConstrIsEnforced = partConstr->conenforced;
 	partConstrFrelid = partConstr->confrelid;
 	partConstrRelid = partConstr->conrelid;
 
+	/*
+	 * When the enforcibility of both constraints is the same, no special
+	 * action is taken, and the flow remains unchanged. However, there are two
+	 * scenarios to consider when it differs:
+	 *
+	 * 1. Parent constraint is not-enforced and child constraint is enforced:
+	 *
+	 * This is allowed because the not-enforced parent constraint does not
+	 * have triggers, so no redundancy issues arise with the child constraint,
+	 * even if it is enforced. In this case, we allow the child constraint to
+	 * remain enforced and keep its trigger intact. This ensures that
+	 * referential integrity checks for the child continue as before, even
+	 * with the parent constraint not enforced. The relationship between the
+	 * two constraints is maintained by setting the parent constraint, which
+	 * enables us to locate the child constraint. This is important if the
+	 * parent constraint is later changed to enforced, at which point the
+	 * necessary trigger will be created for the parent. Any potential
+	 * redundancy from these triggers will be addressed accordingly.
+	 *
+	 * 2. Parent constraint is enforced and child constraint is not-enforced:
+	 *
+	 * This is not allowed, as it would violate referential integrity. In such
+	 * cases, the child constraint will first be made enforced before merging
+	 * it with the enforced parent constraint. Then, action triggers and
+	 * constraint redundancy will be handled in the usual manner, similar to
+	 * how two enforced constraints are merged.
+	 */
+	if (!parentConstrIsEnforced && partConstrIsEnforced)
+	{
+		ReleaseSysCache(partcontup);
+		ReleaseSysCache(parentConstrTup);
+
+		ConstraintSetParentConstraint(partConstrOid, parentConstrOid,
+									  RelationGetRelid(partition));
+		CommandCounterIncrement();
+
+		return;
+	}
+	else if (parentConstrIsEnforced && !partConstrIsEnforced)
+	{
+		Constraint *cmdcon = makeNode(Constraint);
+		Relation	conrel;
+		List	   *otherrelids = NIL;
+
+		cmdcon->contype = partConstr->contype;
+		cmdcon->conname = NameStr(partConstr->conname);
+		cmdcon->deferrable = partConstr->condeferrable;
+		cmdcon->initdeferred = partConstr->condeferred;
+		cmdcon->is_enforced = true;
+
+		conrel = table_open(ConstraintRelationId, RowExclusiveLock);
+
+		ATExecAlterConstrRecurse(cmdcon, conrel, trigrel, partConstr->conrelid,
+								 partConstr->confrelid, partcontup, &otherrelids,
+								 AccessExclusiveLock, InvalidOid, InvalidOid,
+								 InvalidOid, InvalidOid);
+
+		table_close(conrel, RowExclusiveLock);
+
+		CommandCounterIncrement();
+	}
+
+	/*
+	 * The constraint parent shouldn't be set beforehand, or if it's already
+	 * set, it should be the specified parent.
+	 */
+	partConstrParentIsSet = OidIsValid(partConstr->conparentid);
+	Assert(!partConstrParentIsSet || partConstr->conparentid == parentConstrOid);
+
 	/*
 	 * Will we need to validate this constraint?   A valid parent constraint
 	 * implies that all child constraints have been validated, so if this one
@@ -11519,8 +11590,10 @@ AttachPartitionForeignKey(List **wqueue,
 	DropForeignKeyConstraintTriggers(trigrel, partConstrOid, partConstrFrelid,
 									 partConstrRelid);
 
-	ConstraintSetParentConstraint(partConstrOid, parentConstrOid,
-								  RelationGetRelid(partition));
+	/* Skip if the parent is already set */
+	if (!partConstrParentIsSet)
+		ConstraintSetParentConstraint(partConstrOid, parentConstrOid,
+									  RelationGetRelid(partition));
 
 	/*
 	 * Like the constraint, attach partition's "check" triggers to the
@@ -11566,7 +11639,7 @@ AttachPartitionForeignKey(List **wqueue,
 	CommandCounterIncrement();
 
 	/* If validation is needed, put it in the queue now. */
-	if (queueValidation)
+	if (wqueue && queueValidation)
 	{
 		Relation	conrel;
 
@@ -12162,6 +12235,17 @@ ATExecAlterConstrEnforceability(Constraint *cmdcon, Relation conrel,
 
 		/* Drop all the triggers */
 		DropForeignKeyConstraintTriggers(tgrel, conoid, InvalidOid, InvalidOid);
+
+		/*
+		 * If the referenced table is partitioned, the child constraint we're
+		 * changing to not-enforced may have additional pg_constraint rows and
+		 * action triggers that remain untouched while this child constraint
+		 * is attached to the not-enforced parent. These must now be removed.
+		 * For more details, see AttachPartitionForeignKey().
+		 */
+		if (OidIsValid(currcon->conparentid) &&
+			get_rel_relkind(currcon->confrelid) == RELKIND_PARTITIONED_TABLE)
+			RemoveInheritedConstraint(conrel, tgrel, currcon->conrelid, conoid);
 	}
 	else						/* Create triggers */
 	{
@@ -12220,13 +12304,42 @@ ATExecAlterConstrEnforceability(Constraint *cmdcon, Relation conrel,
 									   true, NULL, 1, &pkey);
 
 			while (HeapTupleIsValid(childtup = systable_getnext(pscan)))
-				ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, fkrelid, pkrelid,
-										 childtup, otherrelids, lockmode,
-										 ReferencedDelTriggerOid,
-										 ReferencedUpdTriggerOid,
-										 ReferencingInsTriggerOid,
-										 ReferencingUpdTriggerOid);
+			{
+				Form_pg_constraint childcon;
 
+				childcon = (Form_pg_constraint) GETSTRUCT(childtup);
+
+				if (childcon->conenforced)
+				{
+
+					/*
+					 * The child constraint is attached to the parent
+					 * constraint, which is already enforced. Now, as the
+					 * parent constraint is being modified to be enforced, some
+					 * constraints and action triggers on the child table may
+					 * become redundant and need to be removed.
+					 */
+					if (currcon->confrelid == pkrelid)
+					{
+						Relation rel = table_open(childcon->conrelid, lockmode);
+
+						AttachPartitionForeignKey(NULL, rel, childcon->oid,
+												  conoid,
+												  ReferencingInsTriggerOid,
+												  ReferencingUpdTriggerOid,
+												  tgrel);
+
+						table_close(rel, NoLock);
+					}
+				}
+				else
+					ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, fkrelid,
+											 pkrelid, childtup, otherrelids,
+											 lockmode, ReferencedDelTriggerOid,
+											 ReferencedUpdTriggerOid,
+											 ReferencingInsTriggerOid,
+											 ReferencingUpdTriggerOid);
+			}
 			systable_endscan(pscan);
 		}
 	}
@@ -20387,7 +20500,9 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 	{
 		ForeignKeyCacheInfo *fk = lfirst(cell);
 		HeapTuple	contup;
+		HeapTuple	parentContup;
 		Form_pg_constraint conform;
+		Oid			parentConstrIsEnforced;
 
 		contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
 		if (!HeapTupleIsValid(contup))
@@ -20406,12 +20521,34 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 			continue;
 		}
 
+		/* Get the enforcibility of the parent constraint */
+		parentContup = SearchSysCache1(CONSTROID,
+									   ObjectIdGetDatum(conform->conparentid));
+		if (!HeapTupleIsValid(parentContup))
+			elog(ERROR, "cache lookup failed for constraint %u",
+				 conform->conparentid);
+		parentConstrIsEnforced =
+			((Form_pg_constraint) GETSTRUCT(parentContup))->conenforced;
+		ReleaseSysCache(parentContup);
+
 		/*
 		 * The constraint on this table must be marked no longer a child of
 		 * the parent's constraint, as do its check triggers.
 		 */
 		ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);
 
+		/*
+		 * Unsetting the parent is sufficient when the parent constraint is
+		 * not enforced and the child constraint is enforced, as we link them
+		 * by setting the constraint parent, while leaving the rest unchanged.
+		 * For more details, see AttachPartitionForeignKey().
+		 */
+		if (!parentConstrIsEnforced && fk->conenforced)
+		{
+			ReleaseSysCache(contup);
+			continue;
+		}
+
 		/*
 		 * Also, look up the partition's "check" triggers corresponding to the
 		 * enforced constraint being detached and detach them from the parent
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index c8b91f45146..ec2058c06d5 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1643,8 +1643,33 @@ ALTER TABLE fk_partitioned_fk_3 DROP COLUMN fdrop1, DROP COLUMN fdrop2,
 	DROP COLUMN fdrop3, DROP COLUMN fdrop4;
 CREATE TABLE fk_partitioned_fk_3_0 PARTITION OF fk_partitioned_fk_3 FOR VALUES WITH (MODULUS 5, REMAINDER 0);
 CREATE TABLE fk_partitioned_fk_3_1 PARTITION OF fk_partitioned_fk_3 FOR VALUES WITH (MODULUS 5, REMAINDER 1);
+-- Merge the not-enforced parent constraint with the enforced and not-enforced child constraints.
+ALTER TABLE fk_partitioned_fk_3_0 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk ENFORCED;
+ALTER TABLE fk_partitioned_fk_3_1 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT ENFORCED;
+ALTER TABLE fk_partitioned_fk_3 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT ENFORCED;
+SELECT conname, conenforced, convalidated, conrelid::regclass FROM pg_constraint
+WHERE conrelid::regclass::text like 'fk_partitioned_fk_3%' ORDER BY oid;
+            conname             | conenforced | convalidated |       conrelid        
+--------------------------------+-------------+--------------+-----------------------
+ fk_partitioned_fk_3_0_a_b_fkey | t           | t            | fk_partitioned_fk_3_0
+ fk_partitioned_fk_3_1_a_b_fkey | f           | f            | fk_partitioned_fk_3_1
+ fk_partitioned_fk_3_a_b_fkey   | f           | f            | fk_partitioned_fk_3
+(3 rows)
+
+-- Merging an enforced parent constraint (validated) with a not-enforced child
+-- constraint will implicitly change the child constraint to enforced and apply
+-- the validation as well.
 ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_3
   FOR VALUES FROM (2000,2000) TO (3000,3000);
+SELECT conname, conenforced, convalidated, conrelid::regclass FROM pg_constraint
+WHERE conrelid::regclass::text like 'fk_partitioned_fk_3%' ORDER BY oid;
+            conname             | conenforced | convalidated |       conrelid        
+--------------------------------+-------------+--------------+-----------------------
+ fk_partitioned_fk_3_0_a_b_fkey | t           | t            | fk_partitioned_fk_3_0
+ fk_partitioned_fk_3_1_a_b_fkey | t           | t            | fk_partitioned_fk_3_1
+ fk_partitioned_fk_3_a_b_fkey   | t           | t            | fk_partitioned_fk_3
+(3 rows)
+
 -- Creating a foreign key with ONLY on a partitioned table referencing
 -- a non-partitioned table fails.
 ALTER TABLE ONLY fk_partitioned_fk ADD FOREIGN KEY (a, b)
@@ -1665,16 +1690,16 @@ INSERT INTO fk_partitioned_fk_2 (a,b) VALUES (1500, 1501);
 ERROR:  insert or update on table "fk_partitioned_fk_2" violates foreign key constraint "fk_partitioned_fk_2_a_b_fkey"
 DETAIL:  Key (a, b)=(1500, 1501) is not present in table "fk_notpartitioned_pk".
 INSERT INTO fk_partitioned_fk (a,b) VALUES (2500, 2502);
-ERROR:  insert or update on table "fk_partitioned_fk_3_1" violates foreign key constraint "fk_partitioned_fk_a_b_fkey"
+ERROR:  insert or update on table "fk_partitioned_fk_3_1" violates foreign key constraint "fk_partitioned_fk_3_1_a_b_fkey"
 DETAIL:  Key (a, b)=(2500, 2502) is not present in table "fk_notpartitioned_pk".
 INSERT INTO fk_partitioned_fk_3 (a,b) VALUES (2500, 2502);
-ERROR:  insert or update on table "fk_partitioned_fk_3_1" violates foreign key constraint "fk_partitioned_fk_a_b_fkey"
+ERROR:  insert or update on table "fk_partitioned_fk_3_1" violates foreign key constraint "fk_partitioned_fk_3_1_a_b_fkey"
 DETAIL:  Key (a, b)=(2500, 2502) is not present in table "fk_notpartitioned_pk".
 INSERT INTO fk_partitioned_fk (a,b) VALUES (2501, 2503);
-ERROR:  insert or update on table "fk_partitioned_fk_3_0" violates foreign key constraint "fk_partitioned_fk_a_b_fkey"
+ERROR:  insert or update on table "fk_partitioned_fk_3_0" violates foreign key constraint "fk_partitioned_fk_3_0_a_b_fkey"
 DETAIL:  Key (a, b)=(2501, 2503) is not present in table "fk_notpartitioned_pk".
 INSERT INTO fk_partitioned_fk_3 (a,b) VALUES (2501, 2503);
-ERROR:  insert or update on table "fk_partitioned_fk_3_0" violates foreign key constraint "fk_partitioned_fk_a_b_fkey"
+ERROR:  insert or update on table "fk_partitioned_fk_3_0" violates foreign key constraint "fk_partitioned_fk_3_0_a_b_fkey"
 DETAIL:  Key (a, b)=(2501, 2503) is not present in table "fk_notpartitioned_pk".
 -- but if we insert the values that make them valid, then they work
 INSERT INTO fk_notpartitioned_pk VALUES (500, 501), (1500, 1501),
@@ -1685,7 +1710,7 @@ INSERT INTO fk_partitioned_fk (a,b) VALUES (2500, 2502);
 INSERT INTO fk_partitioned_fk (a,b) VALUES (2501, 2503);
 -- this update fails because there is no referenced row
 UPDATE fk_partitioned_fk SET a = a + 1 WHERE a = 2501;
-ERROR:  insert or update on table "fk_partitioned_fk_3_1" violates foreign key constraint "fk_partitioned_fk_a_b_fkey"
+ERROR:  insert or update on table "fk_partitioned_fk_3_1" violates foreign key constraint "fk_partitioned_fk_3_1_a_b_fkey"
 DETAIL:  Key (a, b)=(2502, 2503) is not present in table "fk_notpartitioned_pk".
 -- but we can fix it thusly:
 INSERT INTO fk_notpartitioned_pk (a,b) VALUES (2502, 2503);
@@ -2054,7 +2079,6 @@ ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2 FOR VALUES IN
  a      | integer |           |          | 
 Partition of: fk_partitioned_fk FOR VALUES IN (1500, 1502)
 Foreign-key constraints:
-    "fk_partitioned_fk_2_a_b_fkey" FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk(a, b) ON UPDATE CASCADE ON DELETE CASCADE NOT ENFORCED
     TABLE "fk_partitioned_fk" CONSTRAINT "fk_partitioned_fk_a_b_fkey" FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk(a, b) ON UPDATE CASCADE ON DELETE CASCADE
 
 DROP TABLE fk_partitioned_fk_2;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index f3971c620c3..baf0fb86296 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1241,9 +1241,23 @@ ALTER TABLE fk_partitioned_fk_3 DROP COLUMN fdrop1, DROP COLUMN fdrop2,
 	DROP COLUMN fdrop3, DROP COLUMN fdrop4;
 CREATE TABLE fk_partitioned_fk_3_0 PARTITION OF fk_partitioned_fk_3 FOR VALUES WITH (MODULUS 5, REMAINDER 0);
 CREATE TABLE fk_partitioned_fk_3_1 PARTITION OF fk_partitioned_fk_3 FOR VALUES WITH (MODULUS 5, REMAINDER 1);
+-- Merge the not-enforced parent constraint with the enforced and not-enforced child constraints.
+ALTER TABLE fk_partitioned_fk_3_0 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk ENFORCED;
+ALTER TABLE fk_partitioned_fk_3_1 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT ENFORCED;
+ALTER TABLE fk_partitioned_fk_3 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT ENFORCED;
+
+SELECT conname, conenforced, convalidated, conrelid::regclass FROM pg_constraint
+WHERE conrelid::regclass::text like 'fk_partitioned_fk_3%' ORDER BY oid;
+
+-- Merging an enforced parent constraint (validated) with a not-enforced child
+-- constraint will implicitly change the child constraint to enforced and apply
+-- the validation as well.
 ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_3
   FOR VALUES FROM (2000,2000) TO (3000,3000);
 
+SELECT conname, conenforced, convalidated, conrelid::regclass FROM pg_constraint
+WHERE conrelid::regclass::text like 'fk_partitioned_fk_3%' ORDER BY oid;
+
 -- Creating a foreign key with ONLY on a partitioned table referencing
 -- a non-partitioned table fails.
 ALTER TABLE ONLY fk_partitioned_fk ADD FOREIGN KEY (a, b)
-- 
2.43.5