refactor: Split tryAttachPartitionForeignKey()

Split tryAttachPartitionForeignKey() into three functions:
AttachPartitionForeignKey(), RemoveInheritedConstraint(), and
DropForeignKeyConstraintTriggers(), so they can be reused in some
subsequent patches for the NOT ENFORCED feature.

Author: Amul Sul <amul.sul@enterprisedb.com>
Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA%40mail.gmail.com

src/backend/commands/tablecmds.c

@@ -585,6 +585,14 @@ static bool tryAttachPartitionForeignKey(List **wqueue,
 										 Oid parentInsTrigger,
 										 Oid parentUpdTrigger,
 										 Relation trigrel);
+static void AttachPartitionForeignKey(List **wqueue, Relation partition,
+									  Oid partConstrOid, Oid parentConstrOid,
+									  Oid parentInsTrigger, Oid parentUpdTrigger,
+									  Relation trigrel);
+static void RemoveInheritedConstraint(Relation conrel, Relation trigrel,
+									  Oid conoid, Oid conrelid);
+static void DropForeignKeyConstraintTriggers(Relation trigrel, Oid conoid,
+											 Oid confrelid, Oid conrelid);
 static void GetForeignKeyActionTriggers(Relation trigrel,
 										Oid conoid, Oid confrelid, Oid conrelid,
 										Oid *deleteTriggerOid,
@@ -11467,12 +11475,6 @@ tryAttachPartitionForeignKey(List **wqueue,
 	Form_pg_constraint parentConstr;
 	HeapTuple	partcontup;
 	Form_pg_constraint partConstr;
-	bool		queueValidation;
-	ScanKeyData key;
-	SysScanDesc scan;
-	HeapTuple	trigtup;
-	Oid			insertTriggerOid,
-				updateTriggerOid;
 
 	parentConstrTup = SearchSysCache1(CONSTROID,
 									  ObjectIdGetDatum(parentConstrOid));
@@ -11517,6 +11519,59 @@ tryAttachPartitionForeignKey(List **wqueue,
 		return false;
 	}
 
+	ReleaseSysCache(parentConstrTup);
+	ReleaseSysCache(partcontup);
+
+	/* Looks good!  Attach this constraint. */
+	AttachPartitionForeignKey(wqueue, partition, fk->conoid,
+							  parentConstrOid, parentInsTrigger,
+							  parentUpdTrigger, trigrel);
+
+	return true;
+}
+
+/*
+ * AttachPartitionForeignKey
+ *
+ * The subroutine for tryAttachPartitionForeignKey performs the final tasks of
+ * attaching the constraint, removing redundant triggers and entries from
+ * pg_constraint, and setting the constraint's parent.
+ */
+static void
+AttachPartitionForeignKey(List **wqueue,
+						  Relation partition,
+						  Oid partConstrOid,
+						  Oid parentConstrOid,
+						  Oid parentInsTrigger,
+						  Oid parentUpdTrigger,
+						  Relation trigrel)
+{
+	HeapTuple	parentConstrTup;
+	Form_pg_constraint parentConstr;
+	HeapTuple	partcontup;
+	Form_pg_constraint partConstr;
+	bool		queueValidation;
+	Oid			partConstrFrelid;
+	Oid			partConstrRelid;
+	Oid			insertTriggerOid,
+				updateTriggerOid;
+
+	/* Fetch the parent constraint tuple */
+	parentConstrTup = SearchSysCache1(CONSTROID,
+									  ObjectIdGetDatum(parentConstrOid));
+	if (!HeapTupleIsValid(parentConstrTup))
+		elog(ERROR, "cache lookup failed for constraint %u", parentConstrOid);
+	parentConstr = (Form_pg_constraint) GETSTRUCT(parentConstrTup);
+
+	/* Fetch the child constraint tuple */
+	partcontup = SearchSysCache1(CONSTROID,
+								 ObjectIdGetDatum(partConstrOid));
+	if (!HeapTupleIsValid(partcontup))
+		elog(ERROR, "cache lookup failed for constraint %u", partConstrOid);
+	partConstr = (Form_pg_constraint) GETSTRUCT(partcontup);
+	partConstrFrelid = partConstr->confrelid;
+	partConstrRelid = partConstr->conrelid;
+
 	/*
 	 * Will we need to validate this constraint?   A valid parent constraint
 	 * implies that all child constraints have been validated, so if this one
@@ -11528,50 +11583,15 @@ tryAttachPartitionForeignKey(List **wqueue,
 	ReleaseSysCache(parentConstrTup);
 
 	/*
-	 * Looks good!  Attach this constraint.  The action triggers in the new
+	 * The action triggers in the new partition become redundant -- the parent
-	 * partition become redundant -- the parent table already has equivalent
+	 * table already has equivalent ones, and those will be able to reach the
-	 * ones, and those will be able to reach the partition.  Remove the ones
+	 * partition.  Remove the ones in the partition.  We identify them because
-	 * in the partition.  We identify them because they have our constraint
+	 * they have our constraint OID, as well as being on the referenced rel.
-	 * OID, as well as being on the referenced rel.
 	 */
-	ScanKeyInit(&key,
+	DropForeignKeyConstraintTriggers(trigrel, partConstrOid, partConstrFrelid,
-				Anum_pg_trigger_tgconstraint,
+									 partConstrRelid);
-				BTEqualStrategyNumber, F_OIDEQ,
-				ObjectIdGetDatum(fk->conoid));
-	scan = systable_beginscan(trigrel, TriggerConstraintIndexId, true,
-							  NULL, 1, &key);
-	while ((trigtup = systable_getnext(scan)) != NULL)
-	{
-		Form_pg_trigger trgform = (Form_pg_trigger) GETSTRUCT(trigtup);
-		ObjectAddress trigger;
-
-		if (trgform->tgconstrrelid != fk->conrelid)
-			continue;
-		if (trgform->tgrelid != fk->confrelid)
-			continue;
-
-		/*
-		 * The constraint is originally set up to contain this trigger as an
-		 * implementation object, so there's a dependency record that links
-		 * the two; however, since the trigger is no longer needed, we remove
-		 * the dependency link in order to be able to drop the trigger while
-		 * keeping the constraint intact.
-		 */
-		deleteDependencyRecordsFor(TriggerRelationId,
-								   trgform->oid,
-								   false);
-		/* make dependency deletion visible to performDeletion */
-		CommandCounterIncrement();
-		ObjectAddressSet(trigger, TriggerRelationId,
-						 trgform->oid);
-		performDeletion(&trigger, DROP_RESTRICT, 0);
-		/* make trigger drop visible, in case the loop iterates */
-		CommandCounterIncrement();
-	}
 
-	systable_endscan(scan);
+	ConstraintSetParentConstraint(partConstrOid, parentConstrOid,
-
-	ConstraintSetParentConstraint(fk->conoid, parentConstrOid,
 								  RelationGetRelid(partition));
 
 	/*
@@ -11579,7 +11599,7 @@ tryAttachPartitionForeignKey(List **wqueue,
 	 * corresponding parent triggers.
 	 */
 	GetForeignKeyCheckTriggers(trigrel,
-							   fk->conoid, fk->confrelid, fk->conrelid,
+							   partConstrOid, partConstrFrelid, partConstrRelid,
 							   &insertTriggerOid, &updateTriggerOid);
 	Assert(OidIsValid(insertTriggerOid) && OidIsValid(parentInsTrigger));
 	TriggerSetParentTrigger(trigrel, insertTriggerOid, parentInsTrigger,
@@ -11593,18 +11613,67 @@ tryAttachPartitionForeignKey(List **wqueue,
 	 * attaching now has extra pg_constraint rows and action triggers that are
 	 * no longer needed.  Remove those.
 	 */
-	if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE)
+	if (get_rel_relkind(partConstrFrelid) == RELKIND_PARTITIONED_TABLE)
 	{
 		Relation	pg_constraint = table_open(ConstraintRelationId, RowShareLock);
+
+		RemoveInheritedConstraint(pg_constraint, trigrel, partConstrOid,
+								  partConstrRelid);
+
+		table_close(pg_constraint, RowShareLock);
+	}
+
+	/*
+	 * We updated this pg_constraint row above to set its parent; validating
+	 * it will cause its convalidated flag to change, so we need CCI here.  In
+	 * addition, we need it unconditionally for the rare case where the parent
+	 * table has *two* identical constraints; when reaching this function for
+	 * the second one, we must have made our changes visible, otherwise we
+	 * would try to attach both to this one.
+	 */
+	CommandCounterIncrement();
+
+	/* If validation is needed, put it in the queue now. */
+	if (queueValidation)
+	{
+		Relation	conrel;
+
+		conrel = table_open(ConstraintRelationId, RowExclusiveLock);
+
+		partcontup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(partConstrOid));
+		if (!HeapTupleIsValid(partcontup))
+			elog(ERROR, "cache lookup failed for constraint %u", partConstrOid);
+
+		/* Use the same lock as for AT_ValidateConstraint */
+		QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
+									ShareUpdateExclusiveLock);
+		ReleaseSysCache(partcontup);
+		table_close(conrel, RowExclusiveLock);
+	}
+}
+
+/*
+ * RemoveInheritedConstraint
+ *
+ * Removes the constraint and its associated trigger from the specified
+ * relation, which inherited the given constraint.
+ */
+static void
+RemoveInheritedConstraint(Relation conrel, Relation trigrel, Oid conoid,
+						  Oid conrelid)
+{
 	ObjectAddresses *objs;
 	HeapTuple	consttup;
+	ScanKeyData key;
+	SysScanDesc scan;
+	HeapTuple	trigtup;
 
 	ScanKeyInit(&key,
 				Anum_pg_constraint_conrelid,
 				BTEqualStrategyNumber, F_OIDEQ,
-					ObjectIdGetDatum(fk->conrelid));
+				ObjectIdGetDatum(conrelid));
 
-		scan = systable_beginscan(pg_constraint,
+	scan = systable_beginscan(conrel,
 							  ConstraintRelidTypidNameIndexId,
 							  true, NULL, 1, &key);
 	objs = new_object_addresses();
@@ -11612,7 +11681,7 @@ tryAttachPartitionForeignKey(List **wqueue,
 	{
 		Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(consttup);
 
-			if (conform->conparentid != fk->conoid)
+		if (conform->conparentid != conoid)
 			continue;
 		else
 		{
@@ -11632,12 +11701,12 @@ tryAttachPartitionForeignKey(List **wqueue,
 												   conform->oid,
 												   DEPENDENCY_INTERNAL,
 												   ConstraintRelationId,
-													   fk->conoid);
+												   conoid);
 			Assert(n == 1);		/* actually only one is expected */
 
 			/*
-				 * Now search for the triggers for this constraint and set
+			 * Now search for the triggers for this constraint and set them up
-				 * them up for deletion too
+			 * for deletion too
 			 */
 			ScanKeyInit(&key2,
 						Anum_pg_trigger_tgconstraint,
@@ -11659,38 +11728,58 @@ tryAttachPartitionForeignKey(List **wqueue,
 	performMultipleDeletions(objs, DROP_RESTRICT,
 							 PERFORM_DELETION_INTERNAL);
 	systable_endscan(scan);
-
-		table_close(pg_constraint, RowShareLock);
 }
 
 /*
-	 * We updated this pg_constraint row above to set its parent; validating
+ * DropForeignKeyConstraintTriggers
-	 * it will cause its convalidated flag to change, so we need CCI here.  In
+ *
-	 * addition, we need it unconditionally for the rare case where the parent
+ * The subroutine for tryAttachPartitionForeignKey handles the deletion of
-	 * table has *two* identical constraints; when reaching this function for
+ * action triggers for the foreign key constraint.
-	 * the second one, we must have made our changes visible, otherwise we
-	 * would try to attach both to this one.
  */
-	CommandCounterIncrement();
+static void
+DropForeignKeyConstraintTriggers(Relation trigrel, Oid conoid, Oid confrelid,
+								 Oid conrelid)
+{
+	ScanKeyData key;
+	SysScanDesc scan;
+	HeapTuple	trigtup;
 
-	/* If validation is needed, put it in the queue now. */
+	ScanKeyInit(&key,
-	if (queueValidation)
+				Anum_pg_trigger_tgconstraint,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(conoid));
+	scan = systable_beginscan(trigrel, TriggerConstraintIndexId, true,
+							  NULL, 1, &key);
+	while ((trigtup = systable_getnext(scan)) != NULL)
 	{
-		Relation	conrel;
+		Form_pg_trigger trgform = (Form_pg_trigger) GETSTRUCT(trigtup);
+		ObjectAddress trigger;
 
-		conrel = table_open(ConstraintRelationId, RowExclusiveLock);
+		if (trgform->tgconstrrelid != conrelid)
-		partcontup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
+			continue;
-		if (!HeapTupleIsValid(partcontup))
+		if (trgform->tgrelid != confrelid)
-			elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
+			continue;
 
-		/* Use the same lock as for AT_ValidateConstraint */
+		/*
-		QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
+		 * The constraint is originally set up to contain this trigger as an
-									ShareUpdateExclusiveLock);
+		 * implementation object, so there's a dependency record that links
-		ReleaseSysCache(partcontup);
+		 * the two; however, since the trigger is no longer needed, we remove
-		table_close(conrel, RowExclusiveLock);
+		 * the dependency link in order to be able to drop the trigger while
+		 * keeping the constraint intact.
+		 */
+		deleteDependencyRecordsFor(TriggerRelationId,
+								   trgform->oid,
+								   false);
+		/* make dependency deletion visible to performDeletion */
+		CommandCounterIncrement();
+		ObjectAddressSet(trigger, TriggerRelationId,
+						 trgform->oid);
+		performDeletion(&trigger, DROP_RESTRICT, 0);
+		/* make trigger drop visible, in case the loop iterates */
+		CommandCounterIncrement();
 	}
 
-	return true;
+	systable_endscan(scan);
 }
 
 /*