Re: Repeated crashes in GENERATED ... AS IDENTITY tests

>>>>> "Alvaro" == Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:

Alvaro> I can't look further into this now -- maybe next week if nobody
Alvaro> has beaten me into it. My guess is that the identity stuff is
Alvaro> not setting state like event triggers expect.

I think this is unrelated to the identity stuff. I suspect a race
condition: the event trigger code is implicitly assuming that the event
trigger cache can't pick up new entries in between
EventTriggerStartCompleteQuery and actually firing a table rewrite
operation.

If initially there are no event triggers that require
currentEventTriggerState, then EventTriggerStartCompleteQuery won't
allocate one. But that's done before actually executing the command, so
if we accept invalidations on the cache afterwards, a newly added
trigger might show up, so by the time we reach EventTriggerTableRewrite
we think we have work to do, but currentEventTriggerState is still null.

I haven't been able to reproduce it yet, so this is conjecture, but I
think it's correct. It being a relatively narrow race explains the
relative rarity of the failure.

For the other event trigger types, SQL_DROP checks for
currentEventTriggerState's validity, so it'll simply fail to run the
trigger if one wasn't already present at command start; DDL_COMMAND_END
doesn't actually access currentEventTriggerState at all unless the
trigger function calls pg_event_trigger_ddl_commands, which will falsely
return an error in the race case but won't crash.

So the obvious simple fix would be to have EventTriggerTableRewrite
likewise do nothing if currentEventTriggerState is not set (and it would
be more consistent to do the same for command_end triggers too).

--
Andrew (irc:RhodiumToad)