Thread

  1. Re: Support EXCEPT for ALL SEQUENCES publications

    Shlok Kyal <shlok.kyal.oss@gmail.com> — 2026-05-07T05:05:46Z

    On Tue, 14 Apr 2026 at 18:12, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
    >
    > On Tue, 14 Apr 2026 at 15:12, shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > On Mon, Apr 13, 2026 at 10:48 AM shveta malik <shveta.malik@gmail.com> wrote:
    > > >
    > > > On Sun, Apr 12, 2026 at 12:33 AM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
    > > > >
    > > > > Hi Vignesh and Shveta,
    > > > >
    > > > > Thanks for reviewing the patches.
    > > > > I have addressed the comments and attached the updated patch.
    > > > >
    > > > > This also addressed the comments shared by Shveta in [1].
    > > > > [1]: https://www.postgresql.org/message-id/CAJpy0uDU0PrH=gvFZjpphOX7t=2jH5wTqYry=C22vnuJJ9Q5=g@mail.gmail.com
    > > > >
    > > >
    > > > Please find few comments on 001 and 002:
    > > >
    > > >
    > > > v-001:
    > > > 1)
    > > > - List    *except_tables; /* tables specified in the EXCEPT clause */
    > > > + List    *except_relations; /* relations specified in the EXCEPT
    > > > + * clause */
    > > > Since we have not changed the comments for anything else in patch001,
    > > > we can keep this comment too same as old and changeit in 002.
    > > >
    > > >
    > > > v-002:
    > > > 2)
    > > > pg_publication_rel's prrelid doc says:
    > > > Reference to table
    > > > We shall change it now to 'Reference to table or sequence'
    > > >
    > > > 3)
    > > > In doc, do we eed to change pg_publication_rel's prqual too? IMO, it
    > > > is not applicable to sequence and thus we can change 'relation' to
    > > > 'table' in explanation..
    > > >
    > > > 4)
    > > >        Marks the publication as one that synchronizes changes for all sequences
    > > > -      in the database, including sequences created in the future.
    > > > +      in the database, including sequences created in the future. Sequences
    > > > +      listed in <literal>EXCEPT</literal> clause are excluded from the
    > > > +      publication.
    > > >
    > > > I think we should place it the end of second paragraph rather than at
    > > > the end of first. How about something liek this:
    > > >
    > > > Marks the publication as one that synchronizes changes for all
    > > > sequences in the database, including sequences created in the future.
    > > >
    > > > Only persistent sequences are included in the publication. Temporary
    > > > sequences and unlogged sequences are excluded from the publication.
    > > > Sequences listed in EXCEPT clause are also excluded from the
    > > > publication.
    > > >
    > > > 5)
    > > > +      In such a case, a table or partition or sequence that is included in one
    > > > +      publication but excluded (explicitly or implicitly) by the
    > > > +      <literal>EXCEPT</literal> clause of another is considered included for
    > > > +      replication.
    > > >
    > > > 'a table or partition or sequence' can be changed to 'a table,
    > > > partition, or sequence'
    > > >
    > > > 6)
    > > > In existign doc, shall we give example of publication creation for
    > > > both tables and sequences, each having its except list? This is
    > > > important to show that EXCEPT to be given with individual ALL OBJ. We
    > > > can cahnge last example of doc file to make this. This one:
    > > > 'Create a publication that publishes all sequences for
    > > > synchronization, and all changes in all tables except users and
    > > > departments:'
    > > >
    > > > 7)
    > > > getPublications:
    > > > + * Get the list of tables/sequences for publications specified in the
    > > > + * EXCEPT clause.
    > > >
    > > > We can have both tables and sequences in single publication. We should change
    > > >
    > > > 'tables/sequences' --> tables and sequences
    > > >
    > > > 8)
    > > > In describePublications(),
    > > >
    > > > We had:
    > > > if (!puballtables)
    > > > else
    > > >   * Get tables in the EXCEPT clause for this publication */
    > > >
    > > > now we have added:
    > > > if (puballsequences)
    > > >  /* Get sequences in the EXCEPT clause for this publication */
    > > >
    > > > Since now we can hit this function for 'all-seq' pub too, shall we
    > > > change if-block's condition to:
    > > >
    > > > if (!puballtables && !puballsequences)
    > > >
    > > > and else-block to
    > > >
    > > > else if (puballtables)
    > > >
    > > > otherwise all-seq case will unnecessary enter these blocks and will
    > > > exceute the logic
    > > >
    > > > Please review other functions too in pg_dump to see if we need such
    > > > conditions altering.
    > > >
    > > >
    > > > 9)
    > > > +# Check the initial data on subscriber
    > > > +$result = $node_subscriber->safe_psql('postgres',
    > > > + "SELECT last_value, is_called FROM seq1");
    > > > +is($result, '200|t', 'sequences in EXCEPT list is excluded');
    > > > +
    > > > +$result = $node_subscriber->safe_psql('postgres',
    > > > + "SELECT last_value, is_called FROM seq2");
    > > > +is($result, '200|t', 'initial test data replicated for seq2');
    > > >
    > > > Since both are replicated now because of conflicting EXCEPT in
    > > > multi-pub, shall we change
    > > > comment in 'is(..)'?
    > > >
    > >
    > >
    > > For v-003, one trivial thing:
    > >
    > > Shall we change the name of AlterPublicationTables() as well? It now
    > > deals in both tables and sequences.
    > >
    > Thanks for reviewing the patch. I agree that we should change the name
    > here. Modified the patch.
    >
    > I have also addressed the remaining comments by you and Vignesh in [1], [2], [3]
    > Attached the updated version.
    >
    > [1]: https://www.postgresql.org/message-id/CALDaNm2pGi3FAkN+x+10nqFKNHOUdMwEgqt_TtjLbvz04F3Ktg@mail.gmail.com
    > [2]: https://www.postgresql.org/message-id/CAJpy0uBB4N8KOrHchdgprVi2Ws1+gTcEr+bC2A_ziAHOcZcTqA@mail.gmail.com
    > [3]: https://www.postgresql.org/message-id/CALDaNm1BRQ1s9na_gwLwN3BYER9be+4QNn4V8sDjiMUvao28Jg@mail.gmail.com
    
    I noticed that CFbot is failling.
    The test case in this patch needs to be updated due to changes
    introduced by the recent commit 2e1d4fd.
    I have made the change and attached the updated patch. I have also
    made some cosmetic changes.
    
    Thanks,
    Shlok Kyal