MIME-Version: 1.0
References: 
 <CANhcyEVSXyQkvmrsOWPdQqnm2J3GMyQQrKhyCJiBQzqs6AvSow@mail.gmail.com>
 <CALDaNm125dv88fUDgBPBM-N-hXbF0NLqKe-ymEpMRNymUYRQAA@mail.gmail.com>
 <CANhcyEUxgzaNJNNO_-12wYwGLihpuKmsMH2g4TFnRPx5AQVZmg@mail.gmail.com>
 <CAJpy0uBB4N8KOrHchdgprVi2Ws1+gTcEr+bC2A_ziAHOcZcTqA@mail.gmail.com>
 <CAJpy0uCAJQvBjD7qNWWGnZP_LDwS8AiUJC7YMict9UcYqH=XeQ@mail.gmail.com>
 <CANhcyEU_Yq9ZJ2n5Sqa7RoHze0TD0RGxLQQgV1F6Jm2AROEh8g@mail.gmail.com>
 <CANhcyEWj-ECj=WC+HD_kv27Dn6FkTngFQCVJVTVZfJnjCTKMBQ@mail.gmail.com>
 <CANhcyEW03XO5tLb7opt1yQGHWJ7Ew=L65EWcdrKH=F0mUpuR3A@mail.gmail.com>
 <CAHut+Ptu0Bkwnr5eetdmFhJC7SsEtKjNe_cTwOg5wF65fjyV8w@mail.gmail.com>
 <CANhcyEU+4Z6NXMEk6OmctFv=_pY8K5AiazkfVmn7zxhwQO6CEQ@mail.gmail.com>
 <CAHut+PvOWjKJfNorGO8whnaAtqZb-4zN8u6aOajCw3hn-tMwyA@mail.gmail.com>
 <CANhcyEV_Sv+xQzsjo6hbowDbGV5J7RhFWuQQxWeUWPNPd0k1=w@mail.gmail.com>
 <CAHut+Pt6O3YD8h61RCtzMrRwgvS=RDRNf4KuXAUn1WYnoFU_uQ@mail.gmail.com>
 <CANhcyEU=Vi2oNRWTSph3x2884J7aTt5BTb4AzwmmY0uQAsEMMg@mail.gmail.com>
In-Reply-To: 
 <CANhcyEU=Vi2oNRWTSph3x2884J7aTt5BTb4AzwmmY0uQAsEMMg@mail.gmail.com>
From: Peter Smith <smithpb2250@gmail.com>
Date: Fri, 12 Jun 2026 15:28:46 +1000
Message-ID: 
 <CAHut+Pv4uKN2LEG0-LUwiwzvOX5H074W8zFrKFTntF6__YgssA@mail.gmail.com>
Subject: Re: Support EXCEPT for ALL SEQUENCES publications
To: Shlok Kyal <shlok.kyal.oss@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>, vignesh C <vignesh21@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHut%2BPv4uKN2LEG0-LUwiwzvOX5H074W8zFrKFTntF6__YgssA%40mail.gmail.com>
Precedence: bulk

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