v3-0004-Store-batch-callbacks-at-the-appropriate-level-ra.patch

From 14eb87d068e46939c325c2f070c66f4dfb4f064a Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Wed, 8 Apr 2026 23:22:50 +0900
Subject: [PATCH v3 4/4] Store batch callbacks at the appropriate level rather
 than depth-matching

Instead of tagging each AfterTriggerCallbackItem with a firing depth
and matching at invocation time, store callbacks directly at the level
where they should fire: in AfterTriggersQueryData.batch_callbacks for
immediate constraints (fired by AfterTriggerEndQuery) and in
AfterTriggersData.batch_callbacks for deferred constraints (fired by
AfterTriggerFireDeferred and AfterTriggerSetState).

RegisterAfterTriggerBatchCallback() routes the callback to the current
query-level list when query_depth >= 0, and to the top-level list
otherwise (deferred firing at COMMIT).

FireAfterTriggerBatchCallbacks() is simplified to just iterate and
invoke the passed list.  Memory cleanup is handled by the caller:
AfterTriggerFreeQuery() for query-level callbacks and
AfterTriggerEndXact() for the top-level list.

This eliminates the firing_depth field from AfterTriggerCallbackItem
and the depth-matched iteration logic, replacing it with natural
list-level scoping.  The firing_depth counter in AfterTriggersData
is retained solely for AfterTriggerIsActive().
---
 src/backend/commands/trigger.c | 90 ++++++++++++----------------------
 1 file changed, 32 insertions(+), 58 deletions(-)

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index bbc2405cc4a..993a00aec8c 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3902,8 +3902,8 @@ typedef struct AfterTriggersData
 	 */
 	int			firing_depth;
 
-	List	   *batch_callbacks;	/* List of AfterTriggerCallbackItem,
-									 * possibly from multiple firing depths */
+	List	   *batch_callbacks;	/* List of AfterTriggerCallbackItem;
+									 * for deferred constraints */
 } AfterTriggersData;
 
 struct AfterTriggersQueryData
@@ -3911,6 +3911,7 @@ struct AfterTriggersQueryData
 	AfterTriggerEventList events;	/* events pending from this query */
 	Tuplestorestate *fdw_tuplestore;	/* foreign tuples for said events */
 	List	   *tables;			/* list of AfterTriggersTableData, see below */
+	List       *batch_callbacks;    /* List of AfterTriggerCallbackItem */
 };
 
 struct AfterTriggersTransData
@@ -3944,8 +3945,6 @@ struct AfterTriggersTableData
 typedef struct AfterTriggerCallbackItem
 {
 	AfterTriggerBatchCallback callback;
-	int			firing_depth;	/* afterTriggerFiringDepth when registered;
-								 * callback fires only at this depth */
 	void	   *arg;
 } AfterTriggerCallbackItem;
 
@@ -3984,7 +3983,7 @@ static SetConstraintState SetConstraintStateAddItem(SetConstraintState state,
 													Oid tgoid, bool tgisdeferred);
 static void cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent);
 
-static void FireAfterTriggerBatchCallbacks(void);
+static void FireAfterTriggerBatchCallbacks(List *callbacks);
 
 /*
  * Get the FDW tuplestore for the current trigger query level, creating it
@@ -5237,17 +5236,15 @@ AfterTriggerEndQuery(EState *estate)
 	/*
 	 * Fire batch callbacks before releasing query-level storage and before
 	 * decrementing query_depth.  Callbacks may do real work (index probes,
-	 * error reporting) and rely on query_depth still reflecting the current
-	 * batch level so that nested calls from SPI inside AFTER triggers are
-	 * correctly suppressed by FireAfterTriggerBatchCallbacks's depth guard.
+	 * error reporting).
 	 */
-	FireAfterTriggerBatchCallbacks();
+	FireAfterTriggerBatchCallbacks(qs->batch_callbacks);
+	afterTriggers.firing_depth--;
 
 	/* Release query-level-local storage, including tuplestores if any */
 	AfterTriggerFreeQuery(&afterTriggers.query_stack[afterTriggers.query_depth]);
 
 	afterTriggers.query_depth--;
-	afterTriggers.firing_depth--;
 }
 
 
@@ -5304,6 +5301,9 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
 	 */
 	qs->tables = NIL;
 	list_free_deep(tables);
+
+	list_free_deep(qs->batch_callbacks);
+	qs->batch_callbacks = NIL;
 }
 
 
@@ -5353,8 +5353,7 @@ AfterTriggerFireDeferred(void)
 	}
 
 	/* Flush any fast-path batches accumulated by the triggers just fired. */
-	FireAfterTriggerBatchCallbacks();
-
+	FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks);
 	afterTriggers.firing_depth--;
 
 	/*
@@ -5424,14 +5423,8 @@ AfterTriggerEndXact(bool isCommit)
 
 	afterTriggers.firing_depth = 0;
 
-	Assert(afterTriggers.batch_callbacks == NIL || !isCommit);
-
-	/* On abort, discard any pending callbacks without firing them. */
-	if (!isCommit)
-	{
-		list_free_deep(afterTriggers.batch_callbacks);
-		afterTriggers.batch_callbacks = NIL;
-	}
+	list_free_deep(afterTriggers.batch_callbacks);
+	afterTriggers.batch_callbacks = NIL;
 }
 
 /*
@@ -5732,6 +5725,7 @@ AfterTriggerEnlargeQueryState(void)
 		qs->events.tailfree = NULL;
 		qs->fdw_tuplestore = NULL;
 		qs->tables = NIL;
+		qs->batch_callbacks = NIL;
 
 		++init_depth;
 	}
@@ -6114,7 +6108,9 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
 		/*
 		 * Flush any fast-path batches accumulated by the triggers just fired.
 		 */
-		FireAfterTriggerBatchCallbacks();
+		FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks);
+		list_free_deep(afterTriggers.batch_callbacks);
+		afterTriggers.batch_callbacks = NIL;
 		afterTriggers.firing_depth--;
 
 		if (snapshot_set)
@@ -6843,60 +6839,38 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
 	oldcxt = MemoryContextSwitchTo(TopTransactionContext);
 	item = palloc(sizeof(AfterTriggerCallbackItem));
 	item->callback = callback;
-	item->firing_depth = afterTriggers.firing_depth;
 	item->arg = arg;
-	afterTriggers.batch_callbacks =
-		lappend(afterTriggers.batch_callbacks, item);
+	if (afterTriggers.query_depth >= 0)
+	{
+		AfterTriggersQueryData *qs =
+			&afterTriggers.query_stack[afterTriggers.query_depth];
+		qs->batch_callbacks = lappend(qs->batch_callbacks, item);
+	}
+	else
+		afterTriggers.batch_callbacks =
+			lappend(afterTriggers.batch_callbacks, item);
 	MemoryContextSwitchTo(oldcxt);
 }
 
 /*
  * FireAfterTriggerBatchCallbacks
- *		Invoke callbacks registered at the current firing depth.
- *
- * Each callback is tagged with the afterTriggerFiringDepth at registration
- * time.  Only callbacks matching the current depth are invoked; the rest
- * are retained for when their own depth fires.  This ensures that nested
- * trigger-firing contexts (e.g., SPI calls inside AFTER triggers) only
- * fire the callbacks they registered, leaving outer-level callbacks intact
- * until their firing depth is reached.
+ *		Invoke all callbacks in the given list.
  *
- * The list is updated before any callbacks are invoked so that if a
- * callback throws an ERROR the list is already in a consistent state.
+ * Memory cleanup of the list and its items is handled by the caller
+ * (AfterTriggerFreeQuery for query-level callbacks, AfterTriggerEndXact
+ * for top-level deferred callbacks).
  */
 static void
-FireAfterTriggerBatchCallbacks(void)
+FireAfterTriggerBatchCallbacks(List *callbacks)
 {
-	List	   *remaining = NIL;
-	List	   *to_fire = NIL;
 	ListCell   *lc;
 
-	/* remaining and to_fire lists must survive until callbacks complete */
-	MemoryContext oldcxt = MemoryContextSwitchTo(TopTransactionContext);
-
-	foreach(lc, afterTriggers.batch_callbacks)
-	{
-		AfterTriggerCallbackItem *item = lfirst(lc);
-
-		if (item->firing_depth == afterTriggers.firing_depth)
-			to_fire = lappend(to_fire, item);
-		else
-			remaining = lappend(remaining, item);
-	}
-
-	list_free(afterTriggers.batch_callbacks);
-	afterTriggers.batch_callbacks = remaining;
-	MemoryContextSwitchTo(oldcxt);
-
-	/* Now fire; if one throws, the list is already clean */
-	foreach(lc, to_fire)
+	foreach(lc, callbacks)
 	{
 		AfterTriggerCallbackItem *item = lfirst(lc);
 
 		item->callback(item->arg);
-		pfree(item);
 	}
-	list_free(to_fire);
 }
 
 /*
-- 
2.47.3