Re: Support EXCEPT for ALL SEQUENCES publications - Mailing list pgsql-hackers
| From | Shlok Kyal |
|---|---|
| Subject | Re: Support EXCEPT for ALL SEQUENCES publications |
| Date | |
| Msg-id | CANhcyEU_Yq9ZJ2n5Sqa7RoHze0TD0RGxLQQgV1F6Jm2AROEh8g@mail.gmail.com Whole thread |
| In response to | Re: Support EXCEPT for ALL SEQUENCES publications (shveta malik <shveta.malik@gmail.com>) |
| List | pgsql-hackers |
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 Thanks, Shlok Kyal
Attachment
pgsql-hackers by date: