On Tue, 14 Apr 2026 at 15:12, shveta malik <[email protected]> wrote: > > On Mon, Apr 13, 2026 at 10:48 AM shveta malik <[email protected]> wrote: > > > > On Sun, Apr 12, 2026 at 12:33 AM Shlok Kyal <[email protected]> > > 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_ttjlbvz04f3...@mail.gmail.com [2]: https://www.postgresql.org/message-id/cajpy0ubb4n8korhchdgprvi2ws1+gtcer+bc2a_ziahoczc...@mail.gmail.com [3]: https://www.postgresql.org/message-id/caldanm1brq1s9na_gwlwn3byer9be+4qnn4v8sdjimuvao2...@mail.gmail.com Thanks, Shlok Kyal
v3-0001-Rename-identifiers-to-use-generic-relation-orient.patch
Description: Binary data
v3-0003-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLICA.patch
Description: Binary data
v3-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-CREATE-PUBLIC.patch
Description: Binary data
