Re: Support EXCEPT for ALL SEQUENCES publications

On Fri, 12 Jun 2026 at 10:59, Peter Smith <smithpb2250@gmail.com> wrote:
>
> 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/
>
Yes, I wanted the text to be consistent with the existing doc. But
after reading your suggestion, I feel your version is more
appropriate. I have updated the patch to use it.

> ======
> 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.
>

I have addressed the remaining comments and have attached the updated
v10 patch in [1].
[1]: https://www.postgresql.org/message-id/CANhcyEVDStqoR3qDSgsv5VEuBMzMX4YpdEfW-7VPX3c5Vf1YJA%40mail.gmail.com

Thanks,
Shlok Kyal