| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Support EXCEPT for ALL SEQUENCES publications |
| Date: | 2026-04-14 12:42:40 |
| Message-ID: | CANhcyEU_Yq9ZJ2n5Sqa7RoHze0TD0RGxLQQgV1F6Jm2AROEh8g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, 14 Apr 2026 at 15:12, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Mon, Apr 13, 2026 at 10:48 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Sun, Apr 12, 2026 at 12:33 AM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)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
Thanks,
Shlok Kyal
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Rename-identifiers-to-use-generic-relation-orient.patch | application/octet-stream | 20.5 KB |
| v3-0003-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLICA.patch | application/octet-stream | 24.5 KB |
| v3-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-CREATE-PUBLIC.patch | application/octet-stream | 44.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Shlok Kyal | 2026-04-14 12:43:45 | Re: Support EXCEPT for ALL SEQUENCES publications |
| Previous Message | Ashutosh Sharma | 2026-04-14 12:31:39 | Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication |