diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 9de22a13d7..e660c10d8a 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -3433,14 +3433,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; } @@ -3484,6 +3482,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() @@ -3853,7 +3868,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) @@ -3985,23 +4000,45 @@ AfterTriggerEndQuery(EState *estate) * 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. Be careful here: firing a - * trigger could result in query_stack being repalloc'd, so we can't save - * its address across afterTriggerInvokeEvents calls. + * 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;