| From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
|---|---|
| To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Remove unused for_all_tables field from AlterPublicationStmt |
| Date: | 2025-11-10 05:20:32 |
| Message-ID: | CAHut+Pt+xrHMaYTnntF4g-vDqBwE1VqEHfbub=QzKVz=FyVveg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, Sep 27, 2025 at 2:28 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Fri, Sep 26, 2025 at 8:02 AM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
> >
> > On 2025-Sep-26, Chao Li wrote:
> >
> > > I agree to remove the field from AlterPublicationStmt, but I think we
> > > should retain "Assert(!stmt)”. Because Assert() is a way to detect
> > > programming bug. During development and debug builds, it prints a
> > > diagnostic message which is helpful for identifying bugs. Without the
> > > Assert(!stmt), it will just silently discard the bug by “if (stmt)” in
> > > case that stmt happens to be NULL.
> >
> > CreatePublication() calls this with an empty stmt, so if you keep the
> > assertion, the program would crash (unless that callsite is dead code,
> > in which case it should probably be modified as well). In any case,
> > such a simple assertion is not very useful: the program would crash
> > anyway as soon as we tried to dereference stmt, which is exactly the
> > same the assertion would do.
> >
> > I'm going to bet that Masahiko has the code right.
>
> Thank you everyone for reviewing the patch.
>
> I agree with Álvaro's point. I've pushed the proposed patch.
>
>
Hi, Sorry, I stumbled upon this thread late...
I feel that using ALTER PUBLICATION commands, a user should be able to
construct any publication equivalent to one that they could have made
using CREATE PUBLICATION in the first place.
Now, it is perfectly fine to make an "empty" publication (e.g. CREATE
PUBLICATION pubname;). But currently, there is no way to alter that
empty publication to become a FOR ALL TABLES publication.
Therefore, it seems to me that the ALTER PUBLICATION syntax is
incomplete. e.g. There also needs to be the ability to do ALTER
PUBLICATION ADD/SET FOR ALL TABLES;
Although the member that was removed might be currently unused, won't
it be needed again whenever those missing ALTER PUBLICATION ADD/SET
FOR ALL TABLES are implemented?
======
Kind Regards,
Peter Smith.
Fujitsu Australia
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2025-11-10 05:33:40 | Re: Extended Statistics set/restore/clear functions. |
| Previous Message | Akshay Joshi | 2025-11-10 05:16:40 | Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement |