On Fri, 27 Mar 2026 at 07:06, Peter Smith <[email protected]> wrote:
>
> On Fri, Mar 27, 2026 at 2:24 AM vignesh C <[email protected]> wrote:
> >
> ...
> > > By now multiple people (Dilip Kumar, Peter Smith, Sawada Masahiko)
> > > have preferred the alternate syntax, to move TABLE inside () to make
> > > specifying inclusion and exclusion list in a similar way. Unless we
> > > have more feedback, I think we can change it now.
> >
> > Attached patch has the changes for the same.
> >
>
> Hi Vignesh.
>
> Here are some review comments for the new syntax patch 0001.
>
> ======
> doc/src/sgml/ref/create_publication.sgml
>
> 1.
> -    ALL TABLES [ EXCEPT TABLE ( <replaceable
> class="parameter">except_table_object</replaceable> [, ... ] ) ]
> +    ALL TABLES [ EXCEPT ( TABLE  <replaceable
> class="parameter">except_table_object</replaceable> [, ... ] ) ]
>
> I don't think this is correct. To have the same flexibility as the
> TABLE inclusion lists, the TABLE keyword needs to be inside the
> <except_table_object>, and there need to be more ellipses there too.
>
> e.g. Like this:
>
> and publication_all_object is one of:
>     ALL TABLES [ EXCEPT ( except_table_object [, ... ] ) ]
>     ALL SEQUENCES
>
> and except_table_object is:
>     TABLE { [ ONLY ] table_name [ * ] } [, ...]
>
> ~~~
>
> 2.
> The CREATE PUBLICATION page still refers to "EXCEPT TABLE" in multiple places.
>
> Perhaps now it should be called "EXCEPT (TABLE) clause" or just
> "EXCEPT clause" or something else.
>
> ======
> doc/src/sgml/ref/alter_publication.sgml
>
> 3.
> -    ALL TABLES [ EXCEPT TABLE ( <replaceable
> class="parameter">except_table_object</replaceable> [, ... ] ) ]
> +    ALL TABLES [ EXCEPT ( TABLE  <replaceable
> class="parameter">except_table_object</replaceable> [, ... ] ) ]
>
> This ALTER PUBLICATION synopsis should have the same/similar changes
> as in the above review comment above for the CREATE PUBLICATION
> synopsis.
>
> e.g.
>
> and publication_all_object is one of:
>     ALL TABLES [ EXCEPT ( except_table_object [, ... ] ) ]
>     ALL SEQUENCES
>
> and except_table_object is:
>     TABLE { [ ONLY ] table_name [ * ] } [, ...]
>
> ~~~
>
> 4.
> Also, similar to the earlier review comment, this ALTER PUBLICATION
> page still refers to "EXCEPT TABLE" in multiple places.
>
> Perhaps now it should be called "EXCEPT (TABLE) clause" or just "EXCEPT 
> clause".
>
> ======
> src/backend/parser/gram.y
>
> 5.
> - EXCEPT TABLE '(' pub_except_obj_list ')' { $$ = $4; }
> + EXCEPT '(' TABLE pub_except_obj_list ')' { $$ = $4; }
>
> Same review comment as my above synopsis comments.
>
> This patch implementation only supports EXCEPT (TABLE t1,t2,t3);
>
> Specifically, it doesn't have the same flexibility you get from a
> table inclusion list.
>
> e.g. IMO cmd should also work for:
> EXCEPT (TABLE t1, TABLE t2,t3);
> etc.
>
> ======
> src/bin/pg_dump/pg_dump.c
>
> 6.
> Code comment still refers to "EXCEPT TABLE clause", but it doesn't
> look like that anymore.
>
> ======
> src/test/regress/sql/publication.sql
>
> 7.
> Comments still refer to "EXCEPT TABLE" and "EXCEPT TABLE clause", but
> it doesn't look like that anymore.
>
> ~~~
>
> 8.
> There needs to be additional tests to confirm that the syntax has the
> same flexibility as TABLE inclusion lists do
>
> e.g. all these variations below are valid and equivalent:
> ... EXCEPT (TABLE t1, t2, t3, t4);
> ... EXCEPT (TABLE t1, t2, TABLE t3, t4);
> ... EXCEPT (TABLE t1, t2, TABLE t3, t4);
> ... EXCEPT (TABLE t1, TABLE t2, TABLE t3, TABLE t4);
>
I have not tested all the scenarios as per Amit's suggestion in [1]. I
have just modified an existing test.

> ======
> src/test/subscription/t/037_except.pl
>
> 9.
> Comments still refer to "EXCEPT TABLE", but it doesn't look like that anymore.
>
> ======
> (more files)
>
> 10.
> There are still more source files (not currently part of this patch)
> that are still calling this the "EXCEPT TABLE" clause, even though the
> syntax is different now.
>
> e.g. "src/backend/commands/publicationcmds.c" and others.
>
> Need to search the master source to find every affected comment.
>
Hi Peter,

I have addressed the comments. Attached the updated patch.

[1]: 
https://www.postgresql.org/message-id/CAA4eK1Jjm+w3hEGgsDu_r1Pysez=8mmtGu6=xwpe4muh+ey...@mail.gmail.com

Thanks,
Shlok Kyal

Attachment: v2-0001-Change-syntax-of-EXCEPT-TABLE-clause-in-publicati.patch
Description: Binary data

Reply via email to