Collect ALTER PUBLICATION commands for event triggers

Hi

2026年7月24日(金) 11:07 Fujii Masao <masao.fujii@gmail.com>:

Hi,

I found two cases where ALTER PUBLICATION fires ddl_command_end,
but pg_event_trigger_ddl_commands() does not report the corresponding
ALTER PUBLICATION command.

Attached are two patches.

0001 patch: Collect ALTER PUBLICATION mapping drops

When an ALTER PUBLICATION command only removes publication membership mappings,
the publication itself is not collected as a ddl_command_end object.

For example:
ALTER PUBLICATION p DROP TABLE t1;
ALTER PUBLICATION p SET TABLE t2;

sql_drop reports the removed publication relation,
but pg_event_trigger_ddl_commands() reports no corresponding
ALTER PUBLICATION entry. This seems like a bug to me. Thoughts?

It certainly seems inconsistent with the behaviour of e.g. ALTER TABLE
... DROP COLUMN.
The documentation [*] indicates the sql_drop event should be followed by
a ddl_command_end event for the main ALTER command, so there's no obvious
reason why it isn't happening here.

[*] https://www.postgresql.org/docs/devel/event-trigger-definition.html#EVENT-TRIGGER-SQL_DROP

0001 patch fixes the issue by detecting when table or schema membership
mappings are removed and collecting the publication itself once as the
ALTER PUBLICATION command.

As a side-effect of this fix, SET commands that both remove and add
memberships may now produce both a publication-level entry and
the existing per-membership entries for additions. This is intentional,
since they represent different parts of the command.

Seems reasonable.

I think this should be backpatched to all supported branches. But,
if the above behavioral change is considered too risky for backpatching,
though, I'm fine with applying it only to master. Thoughts?

This seems like the kind of issue which doesn't get backpatched because
it changes behaviour and no-one has complained about it (AFAIK),
but that's just my Friday afternoon vibe opinion and more qualified
opinions may be available.

0002: Collect ALTER PUBLICATION SET ALL changes

ALTER PUBLICATION ... SET ALL TABLES and SET ALL SEQUENCES have
a similar issue. These commands fire ddl_command_end,
but pg_event_trigger_ddl_commands() can return no entry.

0002 patch fixes the issue by making AlterPublicationAllFlags() report
whether the publication state changed. If it did, the publication itself is
collected as the ALTER PUBLICATION command.

As a side-effect of this fix, SET ALL TABLES ... EXCEPT (...) may now produce
both a publication-level entry and the existing per-membership entries for
added exclusions. This is intentional, since they represent different parts
of the command.

I think this should be backpatched to v19, where
ALTER PUBLICATION SET ALL TABLES and SET ALL SEQUENCES were introduced.

Seems reasonable.

Would it be an idea to add SET ALL TABLES ... EXCEPT (...) to the regression
test as well, to clarify the expected behaviour?

Regards

Ian Barwick

On Jul 24, 2026, at 10:06, Fujii Masao <masao.fujii@gmail.com> wrote:

Hi,

I found two cases where ALTER PUBLICATION fires ddl_command_end,
but pg_event_trigger_ddl_commands() does not report the corresponding
ALTER PUBLICATION command.

Attached are two patches.

0001 patch: Collect ALTER PUBLICATION mapping drops

When an ALTER PUBLICATION command only removes publication membership mappings,
the publication itself is not collected as a ddl_command_end object.

For example:
ALTER PUBLICATION p DROP TABLE t1;
ALTER PUBLICATION p SET TABLE t2;

sql_drop reports the removed publication relation,
but pg_event_trigger_ddl_commands() reports no corresponding
ALTER PUBLICATION entry. This seems like a bug to me. Thoughts?

0001 patch fixes the issue by detecting when table or schema membership
mappings are removed and collecting the publication itself once as the
ALTER PUBLICATION command.

As a side-effect of this fix, SET commands that both remove and add
memberships may now produce both a publication-level entry and
the existing per-membership entries for additions. This is intentional,
since they represent different parts of the command.

I think this should be backpatched to all supported branches. But,
if the above behavioral change is considered too risky for backpatching,
though, I'm fine with applying it only to master. Thoughts?

0002: Collect ALTER PUBLICATION SET ALL changes

ALTER PUBLICATION ... SET ALL TABLES and SET ALL SEQUENCES have
a similar issue. These commands fire ddl_command_end,
but pg_event_trigger_ddl_commands() can return no entry.

0002 patch fixes the issue by making AlterPublicationAllFlags() report
whether the publication state changed. If it did, the publication itself is
collected as the ALTER PUBLICATION command.

As a side-effect of this fix, SET ALL TABLES ... EXCEPT (...) may now produce
both a publication-level entry and the existing per-membership entries for
added exclusions. This is intentional, since they represent different parts
of the command.

I think this should be backpatched to v19, where
ALTER PUBLICATION SET ALL TABLES and SET ALL SEQUENCES were introduced.

Regards,

--
Fujii Masao
<v1-0001-Collect-ALTER-PUBLICATION-mapping-drops-for-event.patch><v1-0002-Collect-ALTER-PUBLICATION-SET-ALL-commands-for-ev.patch>

Both v1-0001 and v1-0002 look good to me.

While reviewing 0002, I noticed another suspicion. AlterPublicationAllFlags() directly updates the pg_publication tuple but doesn’t call InvokeObjectPostAlterHook(). This seems inconsistent with AlterPublicationOptions() and AlterPublicationOwner_internal(), which invoke that hook after altering the publication.

As this is a separate issue from this patch, should I start a new thread? This seems an oversight of 493f8c6439c.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Friday, July 24, 2026 10:07 AM Fujii Masao <masao.fujii@gmail.com> wrote:

I found two cases where ALTER PUBLICATION fires ddl_command_end, but
pg_event_trigger_ddl_commands() does not report the corresponding ALTER
PUBLICATION command.

Attached are two patches.

0001 patch: Collect ALTER PUBLICATION mapping drops

When an ALTER PUBLICATION command only removes publication
membership mappings, the publication itself is not collected as a
ddl_command_end object.

For example:
ALTER PUBLICATION p DROP TABLE t1;
ALTER PUBLICATION p SET TABLE t2;

sql_drop reports the removed publication relation, but
pg_event_trigger_ddl_commands() reports no corresponding ALTER
PUBLICATION entry. This seems like a bug to me. Thoughts?

I suspect this wasn't caught earlier because handling the dropped publication
relation was sufficient to deparse it into the original ALTER PUBLICATION
command (by combining multiple dropped items). I also assumed the primary use
case for the event trigger was deparsing. I'm not opposed to also catching this
command in ddl_end - I'm just curious whether it would address any additional
use cases beyond what's already covered.

I think this should be backpatched to all supported branches. But, if the above
behavioral change is considered too risky for backpatching, though, I'm fine
with applying it only to master. Thoughts?

0002: Collect ALTER PUBLICATION SET ALL changes

ALTER PUBLICATION ... SET ALL TABLES and SET ALL SEQUENCES have a
similar issue. These commands fire ddl_command_end, but
pg_event_trigger_ddl_commands() can return no entry.

0002 patch fixes the issue by making AlterPublicationAllFlags() report whether
the publication state changed. If it did, the publication itself is collected as the
ALTER PUBLICATION command.

+1 for addressing this. Without handling this command, there is no alternative
means to deparse it into the original ALTER PUBLICATION SET ALL statement.

As a side-effect of this fix, SET ALL TABLES ... EXCEPT (...) may now produce
both a publication-level entry and the existing per-membership entries for
added exclusions. This is intentional, since they represent different parts of
the command.

Best Regards,
Zhijie Hou

On Fri, Jul 24, 2026 at 4:45 PM Ian Lawrence Barwick <barwick@gmail.com> wrote:

This seems like the kind of issue which doesn't get backpatched because
it changes behaviour and no-one has complained about it (AFAIK),
but that's just my Friday afternoon vibe opinion and more qualified
opinions may be available.

Yeah, I see your point.

Would it be an idea to add SET ALL TABLES ... EXCEPT (...) to the regression
test as well, to clarify the expected behaviour?

Thanks for the suggestion! I've updated patch 0002 accordingly.
Attached are the updated patches.

Patch 0001 is unchanged. Patch 0003 is new and invokes the post-alter
hook for ALTER PUBLICATION SET ALL, as suggested by Chao.

Regards,

--
Fujii Masao

On Fri, Jul 24, 2026 at 5:38 PM Chao Li <li.evan.chao@gmail.com> wrote:

Both v1-0001 and v1-0002 look good to me.

Thanks for the review!

While reviewing 0002, I noticed another suspicion. AlterPublicationAllFlags() directly updates the pg_publication tuple but doesn’t call InvokeObjectPostAlterHook(). This seems inconsistent with AlterPublicationOptions() and AlterPublicationOwner_internal(), which invoke that hook after altering the publication.

As this is a separate issue from this patch, should I start a new thread? This seems an oversight of 493f8c6439c.

Good catch! Although it's a separate issue, I think it's fine to
discuss it here since it's a bit related and a small change.

I put together a patch to invoke the post-alter hook for
ALTER PUBLICATION SET ALL. It's attached as patch 0003
in my reply to Ian upthread.

Regards,

--
Fujii Masao

On Fri, Jul 24, 2026 at 9:03 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

I suspect this wasn't caught earlier because handling the dropped publication
relation was sufficient to deparse it into the original ALTER PUBLICATION
command (by combining multiple dropped items). I also assumed the primary use
case for the event trigger was deparsing. I'm not opposed to also catching this
command in ddl_end - I'm just curious whether it would address any additional
use cases beyond what's already covered.

sql_drop may be sufficient for deparsing those cases. My concern is that
ddl_command_end fires, but pg_event_trigger_ddl_commands() reports
no corresponding ALTER PUBLICATION entry, which seems inconsistent
from a command-level auditing/logging perspective. So at least for me,
collecting the publication itself seems the right approach...

Regards,

--
Fujii Masao

On Jul 24, 2026, at 21:54, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, Jul 24, 2026 at 5:38 PM Chao Li <li.evan.chao@gmail.com> wrote:

Both v1-0001 and v1-0002 look good to me.

Thanks for the review!

While reviewing 0002, I noticed another suspicion. AlterPublicationAllFlags() directly updates the pg_publication tuple but doesn’t call InvokeObjectPostAlterHook(). This seems inconsistent with AlterPublicationOptions() and AlterPublicationOwner_internal(), which invoke that hook after altering the publication.

As this is a separate issue from this patch, should I start a new thread? This seems an oversight of 493f8c6439c.

Good catch! Although it's a separate issue, I think it's fine to
discuss it here since it's a bit related and a small change.

I put together a patch to invoke the post-alter hook for
ALTER PUBLICATION SET ALL. It's attached as patch 0003
in my reply to Ian upthread.

Yeah, that’s better. Thanks for adding 0003. I will review it next Monday.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/