From c208c7cf13c6968a12e4c9b321ebeebebd931a42 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Mon, 6 Apr 2026 18:40:05 +0900 Subject: [PATCH v2] Fix deferred FK check batching introduced by commit b7b27eb41a5 That commit introduced AfterTriggerIsActive() to detect whether we are inside the after-trigger firing machinery, so that RI trigger functions can take the batched fast path. It was implemented using query_depth >= 0, which correctly identified immediate trigger firing but missed the deferred case where query_depth is -1 at COMMIT via AfterTriggerFireDeferred(). This caused deferred FK checks to fall back to the per-row fast path instead of the batched path. The correct check is whether we are inside an after-trigger firing loop specifically. Introduce afterTriggerFiringDepth, a counter incremented around the trigger-firing loops in AfterTriggerEndQuery, AfterTriggerFireDeferred, and AfterTriggerSetState, and decremented after FireAfterTriggerBatchCallbacks() returns. AfterTriggerIsActive() now returns afterTriggerFiringDepth > 0. Reported-by: Chao Li Author: Chao Li Co-authored-by: Amit Langote Discussion: https://postgr.es/m/C2133B47-79CD-40FF-B088-02D20D654806@gmail.com --- src/backend/commands/trigger.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 4d4e96a5302..5fe2585c88f 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -3940,6 +3940,13 @@ typedef struct AfterTriggerCallbackItem static AfterTriggersData afterTriggers; +/* + * Incremented before invoking afterTriggerInvokeEvents(). Used by + * AfterTriggerIsActive() to determine whether batch callbacks will fire, + * so that RI trigger functions can take the batched fast path. + */ +static int afterTriggerFiringDepth = 0; + static void AfterTriggerExecute(EState *estate, AfterTriggerEvent event, ResultRelInfo *relInfo, @@ -5113,6 +5120,7 @@ AfterTriggerBeginXact(void) Assert(afterTriggers.events.head == NULL); Assert(afterTriggers.trans_stack == NULL); Assert(afterTriggers.maxtransdepth == 0); + Assert(afterTriggerFiringDepth == 0); } @@ -5184,6 +5192,7 @@ AfterTriggerEndQuery(EState *estate) */ qs = &afterTriggers.query_stack[afterTriggers.query_depth]; + afterTriggerFiringDepth++; for (;;) { if (afterTriggerMarkEvents(&qs->events, &afterTriggers.events, true)) @@ -5234,6 +5243,7 @@ AfterTriggerEndQuery(EState *estate) AfterTriggerFreeQuery(&afterTriggers.query_stack[afterTriggers.query_depth]); afterTriggers.query_depth--; + afterTriggerFiringDepth--; } @@ -5329,6 +5339,7 @@ AfterTriggerFireDeferred(void) * Run all the remaining triggers. Loop until they are all gone, in case * some trigger queues more for us to do. */ + afterTriggerFiringDepth++; while (afterTriggerMarkEvents(events, NULL, false)) { CommandId firing_id = afterTriggers.firing_counter++; @@ -5340,6 +5351,8 @@ AfterTriggerFireDeferred(void) /* Flush any fast-path batches accumulated by the triggers just fired. */ FireAfterTriggerBatchCallbacks(); + afterTriggerFiringDepth--; + /* * We don't bother freeing the event list, since it will go away anyway * (and more efficiently than via pfree) in AfterTriggerEndXact. @@ -5404,6 +5417,8 @@ AfterTriggerEndXact(bool isCommit) /* No more afterTriggers manipulation until next transaction starts. */ afterTriggers.query_depth = -1; + + afterTriggerFiringDepth = 0; } /* @@ -6053,6 +6068,7 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt) AfterTriggerEventList *events = &afterTriggers.events; bool snapshot_set = false; + afterTriggerFiringDepth++; while (afterTriggerMarkEvents(events, NULL, true)) { CommandId firing_id = afterTriggers.firing_counter++; @@ -6086,6 +6102,7 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt) * Flush any fast-path batches accumulated by the triggers just fired. */ FireAfterTriggerBatchCallbacks(); + afterTriggerFiringDepth--; if (snapshot_set) PopActiveSnapshot(); @@ -6806,10 +6823,10 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback, * Allocate in TopTransactionContext so the item survives for the duration * of the batch, which may span multiple trigger invocations. * - * Must be called while afterTriggers is active (query_depth >= 0); - * callbacks registered outside a trigger-firing context would never fire. + * Must be called while afterTriggers is active; callbacks registered + * outside a trigger-firing context would never fire. */ - Assert(afterTriggers.query_depth >= 0); + Assert(afterTriggerFiringDepth > 0); oldcxt = MemoryContextSwitchTo(TopTransactionContext); item = palloc(sizeof(AfterTriggerCallbackItem)); item->callback = callback; @@ -6836,6 +6853,7 @@ FireAfterTriggerBatchCallbacks(void) if (afterTriggers.query_depth > 0) return; + Assert(afterTriggerFiringDepth > 0); foreach(lc, afterTriggers.batch_callbacks) { AfterTriggerCallbackItem *item = lfirst(lc); @@ -6858,5 +6876,5 @@ FireAfterTriggerBatchCallbacks(void) bool AfterTriggerIsActive(void) { - return afterTriggers.query_depth >= 0; + return afterTriggerFiringDepth > 0; } -- 2.47.3