upstream/postgresql
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-05-16 03:19:53 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix deferred FK check batching introduced by commit b7b27eb41a

Browse source 
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 <li.evan.chao@gmail.com>
Author: Chao Li <li.evan.chao@gmail.com>
Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/C2133B47-79CD-40FF-B088-02D20D654806@gmail.com
This commit is contained in:
Amit Langote 2026-04-07 08:36:49 +09:00
parent 9897957805
commit 5c54c3ed1b
1 changed files with 22 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
src/backend/commands/trigger.c
View file

@ -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;
}