[PATCH] Add pg_get_event_trigger_ddl() function

Hello!

Should the function preserve the disabled state of an event trigger?

CREATE FUNCTION noop_evt() RETURNS event_trigger LANGUAGE plpgsql AS $$
BEGIN
END;
$$;
CREATE EVENT TRIGGER my_evt ON ddl_command_start EXECUTE FUNCTION noop_evt();
ALTER EVENT TRIGGER my_evt DISABLE;
SELECT pg_get_event_trigger_ddl('my_evt', false);

also REPLICA:

ALTER EVENT TRIGGER my_evt ENABLE REPLICA;
SELECT pg_get_event_trigger_ddl('my_evt', false);

and OWNER:

CREATE ROLE evt_owner_role SUPERUSER;
ALTER EVENT TRIGGER my_evt OWNER TO evt_owner_role;
SELECT pg_get_event_trigger_ddl('my_evt');

+				appendStringInfoString(&filters, ", ");
+
+			appendStringInfo(&filters, "'%s'", str);

Shouldn't this use quote_literal_cstr?

2026年7月18日(土) 6:26 Zsolt Parragi <zsolt.parragi@percona.com>:

Hello!

Thanks for the review!

Should the function preserve the disabled state of an event trigger?

CREATE FUNCTION noop_evt() RETURNS event_trigger LANGUAGE plpgsql AS $$
BEGIN
END;
$$;
CREATE EVENT TRIGGER my_evt ON ddl_command_start EXECUTE FUNCTION

noop_evt();

ALTER EVENT TRIGGER my_evt DISABLE;
SELECT pg_get_event_trigger_ddl('my_evt', false);

also REPLICA:

ALTER EVENT TRIGGER my_evt ENABLE REPLICA;
SELECT pg_get_event_trigger_ddl('my_evt', false);

and OWNER:

CREATE ROLE evt_owner_role SUPERUSER;
ALTER EVENT TRIGGER my_evt OWNER TO evt_owner_role;
SELECT pg_get_event_trigger_ddl('my_evt');

Correct.

+                               appendStringInfoString(&filters, ", ");
+
+                       appendStringInfo(&filters, "'%s'", str);

Shouldn't this use quote_literal_cstr?

Arguably it wouldn't strictly need to, as we'd only ever get plain ASCII
command tags here,
but it's not a critical performance choke point or anything, so might as
well use it to be
on the safe side.

Updated patch version attached.

Regards

Ian Barwick

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.

== trailing XXX (placeholder?) ==

+ * CREATE EVENT TRIGGER statement; XXX

== typo (2x the) ==

+ is false, the the corresponding <literal>ENABLE</literal> clause is

=== 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?

Best, Jim

Hi Jim

2026年7月21日(火) 0:13 Jim Jones <jim.jones@uni-muenster.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.

Regards

Ian Barwick

Hi,

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

Hi Jim

2026年7月21日(火) 0:13 Jim Jones <jim.jones@uni-muenster.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

2026年7月21日(火) 12:38 Rithvika Devisetti <devisettirithvika@gmail.com>:

Hi,

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

Hi Jim

2026年7月21日(火) 0:13 Jim Jones <jim.jones@uni-muenster.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 for checking! Revised version attached, with that and a couple of
minor text tweaks.

Regards

Ian Barwick

v4 looks good to me, I only have one nitpick question:

+        <optional>, <parameter>enable</parameter> <type>boolean</type>
+        <literal>DEFAULT</literal> true</optional> )

Shouldn't we call this something different, for example enabled?

2026年7月22日(水) 4:53 Zsolt Parragi <zsolt.parragi@percona.com>:

v4 looks good to me, I only have one nitpick question:

+        <optional>, <parameter>enable</parameter> <type>boolean</type>
+        <literal>DEFAULT</literal> true</optional> )

Shouldn't we call this something different, for example enabled?

That did cross my mind, but while "enabled" feels more natural grammatically,
"enable" corresponds to the "ENABLE" clause which is to be emitted (or omitted).
Except when it's "DISABLE"... I can't think of any better phrasing
which is reasonably
short and conveys the meaning "emit the clause which indicates the trigger is to
be disabled or enabled, and if the latter in what context".

Regards

Ian Barwick

Hi all,

On Tue, Jul 21, 2026 at 4:46 PM Ian Lawrence Barwick <barwick@gmail.com> wrote:

2026年7月21日(火) 12:38 Rithvika Devisetti <devisettirithvika@gmail.com>:

Hi,

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

Hi Jim

2026年7月21日(火) 0:13 Jim Jones <jim.jones@uni-muenster.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 for checking! Revised version attached, with that and a couple of
minor text tweaks.

Thank you for the updated patch. I reviewed and tested this patch on
my tree. The patch applied cleanly and the regression test completed
successfully without any failures. I performed functional testing of
the new pg_get_event_trigger_ddl() function and verified: the function
is installed and registered correctly, the reconstruction of CREATE
EVENT TRIGGER statements, output formats, reconstruction of single and
multiple WHEN TAG clauses, preservation of event trigger enable
states, optional owner and enable parameters, schema-qualified
function names in the generated DDL, handling of quoted event trigger
names, expected error reporting for non-existent event triggers and
also confirmed that the generated DDL is consistent with the metadata
stored in pg_event_trigger. However I did not observe any functional
issues during testing and behaved as intended. The patch looks good to
me.

Regards,
Solai

That did cross my mind, but while "enabled" feels more natural grammatically,
"enable" corresponds to the "ENABLE" clause which is to be emitted (or omitted).
Except when it's "DISABLE"... I can't think of any better phrasing
which is reasonably
short and conveys the meaning "emit the clause which indicates the trigger is to
be disabled or enabled, and if the latter in what context".

Yeah, I agree with this reasoning, I wasn't even sure to mention this or not. I only did because the ALTER EVENT TRIGGER documentation also mentions "disabled", so there's some precedent of using that form.

Keeping it "enable" is fine with me.

On 22/07/2026 08:14, Zsolt Parragi wrote:

That did cross my mind, but while "enabled" feels more natural grammatically,
"enable" corresponds to the "ENABLE" clause which is to be emitted (or omitted).
Except when it's "DISABLE"... I can't think of any better phrasing
which is reasonably
short and conveys the meaning "emit the clause which indicates the trigger is to
be disabled or enabled, and if the latter in what context".

Yeah, I agree with this reasoning, I wasn't even sure to mention this
or not. I only did because the ALTER EVENT TRIGGER documentation also
mentions "disabled", so there's some precedent of using that form.

Keeping it "enable" is fine with me.

Since there seems to be a consensus among all reviewers I'll mark the CF
entry as ready for committer.

Thanks!

Best, Jim