Re: Remove unused for_all_tables field from AlterPublicationStmt

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.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com