Remove buggy and dead code from CreateTriggerFiringOn

david-rowley · david-rowley · commit 16d69ec29b11 · 2022-09-06T15:51:44.000+12:00
Here we remove some dead code from CreateTriggerFiringOn() which was attempting to find the relevant child partition index corresponding to the given indexOid. As it turned out, thanks to -Wshadow=compatible-local, this code was buggy as the code which was finding the child indexes assigned those to a shadowed variable that directly went out of scope. The code which thought it was looking at the List of child indexes was always referencing an empty List. On further investigation, this code is dead. We never call CreateTriggerFiringOn() passing a valid indexOid in a way that the function would actually ever execute the code in question. So, for lack of a way to test if a fix actually works, let's just remove the dead code instead. As a reminder, if there is ever a need to resurrect this code, an Assert() has been added to remind future feature developers that they might need to write some code to find the corresponding child index. Reported-by: Justin Pryzby Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/20220819211824.GX26426@telsasoft.com
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
@@ -1147,8 +1147,6 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 	if (partition_recurse)
 	{
 		PartitionDesc partdesc = RelationGetPartitionDesc(rel, true);
-		List	   *idxs = NIL;
-		List	   *childTbls = NIL;
 		int			i;
 		MemoryContext oldcxt,
 					perChildCxt;
@@ -1158,53 +1156,23 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 											ALLOCSET_SMALL_SIZES);
 
 		/*
-		 * When a trigger is being created associated with an index, we'll
-		 * need to associate the trigger in each child partition with the
-		 * corresponding index on it.
+		 * We don't currently expect to be called with a valid indexOid.  If
+		 * that ever changes then we'll need to quite code here to find the
+		 * corresponding child index.
 		 */
-		if (OidIsValid(indexOid))
-		{
-			ListCell   *l;
-			List	   *idxs = NIL;
-
-			idxs = find_inheritance_children(indexOid, ShareRowExclusiveLock);
-			foreach(l, idxs)
-				childTbls = lappend_oid(childTbls,
-										IndexGetRelation(lfirst_oid(l),
-														 false));
-		}
+		Assert(!OidIsValid(indexOid));
 
 		oldcxt = MemoryContextSwitchTo(perChildCxt);
 
 		/* Iterate to create the trigger on each existing partition */
 		for (i = 0; i < partdesc->nparts; i++)
 		{
-			Oid			indexOnChild = InvalidOid;
-			ListCell   *l;
-			ListCell   *l2;
 			CreateTrigStmt *childStmt;
 			Relation	childTbl;
 			Node	   *qual;
 
 			childTbl = table_open(partdesc->oids[i], ShareRowExclusiveLock);
 
-			/* Find which of the child indexes is the one on this partition */
-			if (OidIsValid(indexOid))
-			{
-				forboth(l, idxs, l2, childTbls)
-				{
-					if (lfirst_oid(l2) == partdesc->oids[i])
-					{
-						indexOnChild = lfirst_oid(l);
-						break;
-					}
-				}
-				if (!OidIsValid(indexOnChild))
-					elog(ERROR, "failed to find index matching index \"%s\" in partition \"%s\"",
-						 get_rel_name(indexOid),
-						 get_rel_name(partdesc->oids[i]));
-			}
-
 			/*
 			 * Initialize our fabricated parse node by copying the original
 			 * one, then resetting fields that we pass separately.
@@ -1224,7 +1192,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 
 			CreateTriggerFiringOn(childStmt, queryString,
 								  partdesc->oids[i], refRelOid,
-								  InvalidOid, indexOnChild,
+								  InvalidOid, InvalidOid,
 								  funcoid, trigoid, qual,
 								  isInternal, true, trigger_fires_when);
 
@@ -1235,8 +1203,6 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 
 		MemoryContextSwitchTo(oldcxt);
 		MemoryContextDelete(perChildCxt);
-		list_free(idxs);
-		list_free(childTbls);
 	}
 
 	/* Keep lock on target rel until end of xact */
