diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index ba256964b38..2d61dc4818d 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -3300,14 +3300,12 @@ static void afterTriggerFreeEventList(AfterTriggerEventList *events) { AfterTriggerEventChunk *chunk; - AfterTriggerEventChunk *next_chunk; - for (chunk = events->head; chunk != NULL; chunk = next_chunk) + while ((chunk = events->head) != NULL) { - next_chunk = chunk->next; + events->head = chunk->next; pfree(chunk); } - events->head = NULL; events->tail = NULL; events->tailfree = NULL; } @@ -3351,6 +3349,23 @@ afterTriggerRestoreEventList(AfterTriggerEventList *events, } } +/* ---------- + * afterTriggerDeleteHeadEventChunk() + * + * Remove the first chunk of events from the given event list. + * ---------- + */ +static void +afterTriggerDeleteHeadEventChunk(AfterTriggerEventList *events) +{ + AfterTriggerEventChunk *target = events->head; + + Assert(target && target->next); + + events->head = target->next; + pfree(target); +} + /* ---------- * AfterTriggerExecute() @@ -3661,7 +3676,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, /* * If it's last chunk, must sync event list's tailfree too. Note * that delete_ok must NOT be passed as true if there could be - * stacked AfterTriggerEventList values pointing at this event + * additional AfterTriggerEventList values pointing at this event * list, since we'd fail to fix their copies of tailfree. */ if (chunk == events->tail) @@ -3810,24 +3825,50 @@ AfterTriggerEndQuery(EState *estate) * IMMEDIATE: all events we have decided to defer will be available for it * to fire. * - * We loop in case a trigger queues more events at the same query level - * (is that even possible?). Be careful here: firing a trigger could - * result in query_stack being repalloc'd, so we can't save its address - * across afterTriggerInvokeEvents calls. + * We loop in case a trigger queues more events at the same query level. + * Ordinary trigger functions, including all PL/pgSQL trigger functions, + * will instead fire any triggers in a dedicated query level. Foreign key + * enforcement triggers do add to the current query level, thanks to their + * passing fire_triggers = false to SPI_execute_snapshot(). Other + * C-language triggers might do likewise. * * If we find no firable events, we don't have to increment * firing_counter. */ + events = &afterTriggers->query_stack[afterTriggers->query_depth]; + for (;;) { - events = &afterTriggers->query_stack[afterTriggers->query_depth]; if (afterTriggerMarkEvents(events, &afterTriggers->events, true)) { CommandId firing_id = afterTriggers->firing_counter++; + AfterTriggerEventChunk *oldtail = events->tail; - /* OK to delete the immediate events after processing them */ - if (afterTriggerInvokeEvents(events, firing_id, estate, true)) + if (afterTriggerInvokeEvents(events, firing_id, estate, false)) break; /* all fired */ + + /* + * Firing a trigger could result in query_stack being repalloc'd, + * so we must recalculate ptr after each afterTriggerInvokeEvents + * call. Furthermore, it's unsafe to pass delete_ok = true here, + * because that could cause afterTriggerInvokeEvents to try to + * access *events after the stack has been repalloc'd. + */ + events = &afterTriggers->query_stack[afterTriggers->query_depth]; + + /* + * We'll need to scan the events list again. To reduce the cost + * of doing so, get rid of completely-fired chunks. We know that + * all events were marked IN_PROGRESS or DONE at the conclusion of + * afterTriggerMarkEvents, so any still-interesting events must + * have been added after that, and so must be in the chunk that + * was then the tail chunk, or in later chunks. So, zap all + * chunks before oldtail. This is approximately the same set of + * events we would have gotten rid of by passing delete_ok = true. + */ + Assert(oldtail != NULL); + while (events->head != oldtail) + afterTriggerDeleteHeadEventChunk(events); } else break;