Re: Support EXCEPT for ALL SEQUENCES publications - Mailing list pgsql-hackers

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(..)'?

thanks
Shveta