Re: Collect ALTER PUBLICATION commands for event triggers

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Collect ALTER PUBLICATION commands for event triggers
Date: 2026-07-24 08:38:12
Message-ID: B2B24887-00C6-4A4D-97E0-89529D476D47@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Jul 24, 2026, at 10:06, Fujii Masao <masao(dot)fujii(at)gmail(dot)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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2026-07-24 08:53:16 Re: Fix publisher-side sequence permission reporting
Previous Message Nazir Bilal Yavuz 2026-07-24 08:07:26 Re: read stream: Backward I/O combining