Re: [PATCH] Add pg_get_event_trigger_ddl() function

Hi,

On Mon, Jul 20, 2026 at 6:49 PM Ian Lawrence Barwick <barwick(at)gmail(dot)com>
wrote:

> Hi Jim
>
> 2026年7月21日(火) 0:13 Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>:
> >
> > Hi Ian
> >
> > Thanks for the patch! Here a few comments on v2:
> >
> > == shadow variable ==
> >
> > The function pg_get_event_trigger_ddl_internal has a bool parameter
> > named owner, and its body has a char* with the same name.
>
> Good catch - I note the compiler-level shadow check is only effective
> for variables of the same type, which hadn't occurred to me before.
>
> > == trailing XXX (placeholder?) ==
> >
> > + * CREATE EVENT TRIGGER statement; XXX
>
> Whoops, fixed.
>
> > == typo (2x the) ==
> >
> > + is false, the the corresponding <literal>ENABLE</literal> clause is
>
> Classic typo invisible to the author, fixed.
>
> > === switch without default ==
> >
> > I realise that "switch (evtForm->evtenabled)" already tests all possible
> > values of evtenable, but I'm wondering if we really should let it
> > silently finish the buffer like "ALTER EVENT TRIGGER foo ;" if evtenable
> > ever gets a different value. I'd argue that an error message would be
> > better than a malformed DDL. What do you think?
>
> I did wonder - the corresponding switch statement in pg_dump falls through
> to
> "ENABLE" as the default value, but only calls that if "evtenabled" is
> not 'O' (the
> default), so the addition of some new trigger firing state would result
> in "ALTER EVENT TRIGGER foo ENABLE;" if the code was not updated,
> which is easy to overlook with this kind of setup. On the other hand, I
> can't
> imagine what kind of new trigger firing state might be added, so maybe
> it's more of a theoretical risk. In any case I added an error message, and
> also updated the tests to check each existing firing state is
> correctly rendered.

While looking at that, I did update the comment in
> "src/include/commands/trigger.h"
> to note that the "TRIGGER_*" definitions also apply to pg_event_trigger.
>

Patch applies clean to me, and tests passed. I see a grammatical typo below,
there is an additional "be".

cannot be actually be used in a valid statement

Thanks,

Rithvika