Re: Support EXCEPT for ALL SEQUENCES publications

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
Cc: shveta malik <shveta(dot)malik(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Support EXCEPT for ALL SEQUENCES publications
Date: 2026-06-12 05:28:46
Message-ID: CAHut+Pv4uKN2LEG0-LUwiwzvOX5H074W8zFrKFTntF6__YgssA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Some comments for v9-0002.

======
APPLY:

0.
The patch does not apply cleanly:

$ git apply ../patches_misc/v9-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLICA.patch
error: patch failed: doc/src/sgml/ref/alter_publication.sgml:277

======
doc/src/sgml/ref/alter_publication.sgml

1.
+ <para>
+ Reset the publication to be a <literal>ALL SEQUENCES</literal> publication
+ with no excluded sequences:

Personally, I don't see how it is good to have "publication" 2x and
"sequences" 2x in the same sentence.

SUGGESTION
Reset the publication to be <literal>ALL SEQUENCES</literal> with no exclusions:

~

I know you're only following the same pattern as "Reset the
publication to be a FOR ALL TABLES publication with no excluded
tables:". It is good to be consistent, but when the original text is
poor, copying it doesn't make it better.

Things like this fall into a grey-zone because, unless they get fixed
"in-passing", nothing ever changes:
a) I think a patch to only change the original wording would be
rejected because it is too trivial.
b) OTOH, when the original is used as a precedent, the poor wording
just spreads further.

~

Anyway, if your chose not to reword this, then there is a typo /a ALL
SEQUENCES publication/an ALL SEQUENCES publication/

======
src/backend/commands/publicationcmds.c

get_delete_rels:

2.
+ /* look up the cache for the old relmap */

/look up/Look up/

~~~

3.
+ if(found)
+ break;

Missing space after 'if'.

~~~

4.
+ oldrel = palloc0_object (PublicationRelInfo);

Unwanted space after function name.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2026-06-12 05:46:29 Re: [PATCH] Fix replica identity mismatch for partitioned tables with publish_via_partition_root
Previous Message Peter Smith 2026-06-12 05:25:27 Re: Support EXCEPT for ALL SEQUENCES publications