| From: | Ian Lawrence Barwick <barwick(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 07:45:38 |
| Message-ID: | CAB8KJ=jwEsFKhx+v0xg9K5RQKrAjExWh9nrp7LgNhUkndcNcaQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi
2026年7月24日(金) 11:07 Fujii Masao <masao(dot)fujii(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Laurenz Albe | 2026-07-24 08:02:45 | Re: Build warning with meson and dtrace on Fedora 43 |
| Previous Message | Andrey Rachitskiy | 2026-07-24 07:38:04 | Re: Build warning with meson and dtrace on Fedora 43 |