From 14eb87d068e46939c325c2f070c66f4dfb4f064a Mon Sep 17 00:00:00 2001 From: Amit Langote 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