Re: Event trigger bugs (was Re: Repeated crashes in GENERATED ... AS IDENTITY tests)

I wrote:
> Hence, two questions:
> * Should EventTriggerTableRewrite do
> if (!currentEventTriggerState ||
> currentEventTriggerState->commandCollectionInhibited)
> return;
> like most of the other functions, or should it just check for null
> currentEventTriggerState?

After closer inspection I've concluded that it should not look
at commandCollectionInhibited; that disables collection of some
data, but it's not related to whether we ought to fire triggers,
AFAICS.

> * Of the three other callers of EventTriggerCommonSetup, only one
> has such a guard now. But aren't EventTriggerDDLCommandStart and
> EventTriggerDDLCommandEnd equally vulnerable to this type of race
> condition? What should they check exactly? Perhaps
> EventTriggerCommonSetup itself should check this?

And the answer here is that DDLCommandStart must fire regardless of the
existence of currentEventTriggerState, because we don't set that up when
there are only ddl_command_start triggers. But it seems like
EventTriggerDDLCommandEnd should quit if it's not set up. That function
itself wouldn't crash, but presumably the called triggers would call
pg_event_trigger_ddl_commands which would fail. Better to pretend the
triggers aren't active yet.

(I see Andrew Gierth came to the same conclusion.)

regards, tom lane