On Tue, 8 Sep 2020 14:27:03 +0900
Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Sep 07, 2020 at 10:35:08AM +0900, Michael Paquier wrote:
> > More comments in each script would be helpful to document to the
> > reader why those objects are defined as such and their purpose in
> > life.
>
> I have looked at that, and tweaked a couple of things as per the
> attached. Alvaro, Jehan-Guillaume, is that fine to you?
I was actually working on an answer with a new version of the patch, but you've
been faster. Thanks.
I would probably set the error message in VERSION '1.0' to
CREATE EXTENSION test_event_trigger VERSION '1.0'
But it's really nitpicking at this point.
More importantly, I wonder if this bug might be the visible tip of another
bigger one. I was wondering why this crash do not triggers without disabling
the event trigger first.
It appears that when keeping the event trigger enabled,
currentEventTriggerState might be initiated multiple times: one for the top
level "alter extension ... update", then once per DDL query triggering en event
in the extension update script. I suppose there's no crash because the very
first currentEventTriggerState initialized at top level is overrode by next
ones, each being destroyed in their own context. So at the end of the
extension script, currentEventTriggerState is just destroyed and empty.
I did not had time to investigate further more and produce a demo case though.
I might misunderstand something here.
Comment?
++
