On Mon, 25 May 2026 at 13:08, Peter Smith <[email protected]> wrote:
>
> Hi Shlok -
>
> Here are some minor review comments for patch v5-0002.
>
> ======
> doc/src/sgml/catalogs.sgml
>
> 1.
>        <para>
> -       True if the table is excluded from the publication. See
> -       <link 
> linkend="sql-createpublication-params-for-except-table"><literal>EXCEPT</literal></link>.
> +       True if the table or the sequence is excluded from the publication. 
> See
> +       <link 
> linkend="sql-createpublication-params-for-except"><literal>EXCEPT</literal></link>.
>
> /or the sequence/or sequence/
>
> ======
> src/backend/catalog/pg_publication.c
>
> check_publication_add_relation:
>
> 2.
>   * Check if relation can be in given publication and throws appropriate
>   * error if not.
>
> ~
>
> Too terse. There are missing words.
>
> SUGGESTION
> Check if the target relation is allowed to be specified in the given
> publication and throw an error if not.
>
> ~~~
>
> 3.
> if (pri->except)
> {
> relname = RelationGetQualifiedRelationName(targetrel);
> if (pubrelkind == RELKIND_SEQUENCE)
> errormsg = gettext_noop("cannot specify \"%s\" in the publication
> EXCEPT (SEQUENCE) clause");
> else
> errormsg = gettext_noop("cannot specify \"%s\" in the publication
> EXCEPT (TABLE) clause");
> }
> else
> {
> relname = RelationGetRelationName(targetrel);
> errormsg = gettext_noop("cannot add relation \"%s\" to publication");
> }
>
> ~
>
> Wondering why sometimes the error `relname` is fully-qualified and
> sometimes it is not. Isn't it better to always be qualified?
>
This change was made as part of thread [1]. And by reading the thread
I understood that we made 'relname' fully-qualified for the EXCEPT
case.
For the non-EXCEPT case the patch is still in discussion.
I think making `relname` fully-qualified for non-EXCEPT is not related
to this patch and should be discussed in thread [1].

I have addressed the remaining comments in the v6 patches. I have also
merged the patches 0001 and 0002.

[1]: 
https://www.postgresql.org/message-id/CAA4eK1LvV6ex8n1UV_HZ%2Bs77y%2B5wOpbCns-0rF95Gu3EF0SPNA%40mail.gmail.com

Thanks,
Shlok Kyal

Attachment: v6-0001-Support-EXCEPT-for-ALL-SEQUENCES-in-CREATE-PUBLIC.patch
Description: Binary data

Attachment: v6-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLICA.patch
Description: Binary data

Reply via email to