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);
======
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.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
