Thread

  1. Re: Skipping schema changes in publication

    Shlok Kyal <shlok.kyal.oss@gmail.com> — 2025-11-19T10:04:04Z

    On Fri, 14 Nov 2025 at 12:15, Peter Smith <smithpb2250@gmail.com> wrote:
    >
    > Hi Shlok.
    >
    > Some review comments for patch v27-0001.
    >
    > ======
    > doc/src/sgml/ref/alter_publication.sgml
    >
    > 1.
    > +  <para>
    > +   The <literal>RESET</literal> clause will reset the publication to
    > the default
    > +   state. This includes resetting all publication parameters, setting the
    > +   <literal>ALL TABLES</literal> and <literal>ALL SEQUENCES</literal> flags to
    > +   <literal>false</literal>, and removing all associated tables and
    > schemas from
    > +   the publication.
    >    </para>
    >
    > It would be better to give references to the actual
    > pg_publication.puballtables and .puballsequences flag fields [1]
    > instead of vaguely calling them the "<literal>ALL TABLES</literal> and
    > <literal>ALL SEQUENCES</literal> flags".
    >
    Fixed
    
    > ======
    > src/backend/commands/publicationcmds.c
    >
    > AlterPublicationReset:
    >
    > 2.
    > + if (pubform->puballtables)
    > + CacheInvalidateRelcacheAll();
    >
    > Does that also need to check ->puballsequences?
    >
    I think we call CacheInvalidateRelcacheAll to invalide the relsync
    cache for the case of ALTER Publication. For sequences we do not build
    RelSyncEntry.
    Also I see there are other similar occurrences (such as
    RemovePublicationById, AlterPublicationOptions) where we do not
    invalidate cache if we modify all sequence publications.
    So, I think we do not require this check for puballsequences.
    
    > ======
    > src/test/regress/sql/publication.sql
    >
    > 3.
    > If you want to, you can easily combine many of these test cases and
    > verify them in one go instead of separate ALTER/RESET for every kind
    > of flag.
    >
    > ~~~
    >
    I agree. I have made the changes in the latest patch.
    
    > 4.
    > +-- Verify that 'ALL TABLES' flag is reset
    >
    > Missing test to check the 'ALL SEQUENCES' flag gets reset?
    >
    Added the test.
    
    > ======
    > [1] https://www.postgresql.org/docs/devel/catalog-pg-publication.html
    >
    
    I have also addressed the comments in [1], [2].
    
    [1]: https://www.postgresql.org/message-id/CAHut%2BPtRzCD4-0894cutkU_h8cPNtosN0_oSHn2iAKEfg2ENOQ%40mail.gmail.com
    [2]: https://www.postgresql.org/message-id/CAHut+PuHn-hohA4OdEJz+Zfukfr41TvMTeTH7NwJ=wg1+94uNA@mail.gmail.com
    
    Thanks,
    Shlok Kyal