| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, YeXiu <1518981153(at)qq(dot)com>, Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Skipping schema changes in publication |
| Date: | 2026-03-27 07:50:14 |
| Message-ID: | CANhcyEVLp5kbaVR4=nh1jR4YWqv7YpVx_SnYoshbnOrnY79_fg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, 27 Mar 2026 at 07:06, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Fri, Mar 27, 2026 at 2:24 AM vignesh C <vignesh21(at)gmail(dot)com> 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.
Thanks,
Shlok Kyal
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Change-syntax-of-EXCEPT-TABLE-clause-in-publicati.patch | application/octet-stream | 46.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jakub Wartak | 2026-03-27 08:00:03 | Re: pg_plan_advice |
| Previous Message | Amit Kapila | 2026-03-27 07:49:58 | Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion? |