| 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/
| 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 |