| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Peter Smith <smithpb2250(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-05-26 06:00:12 |
| Message-ID: | CANhcyEU+4Z6NXMEk6OmctFv=_pY8K5AiazkfVmn7zxhwQO6CEQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, 25 May 2026 at 13:08, Peter Smith <smithpb2250(at)gmail(dot)com> 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.
Thanks,
Shlok Kyal
| Attachment | Content-Type | Size |
|---|---|---|
| v6-0001-Support-EXCEPT-for-ALL-SEQUENCES-in-CREATE-PUBLIC.patch | application/octet-stream | 58.7 KB |
| v6-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLICA.patch | application/octet-stream | 24.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-05-26 06:02:50 | Re: PostgreSQL and OpenSSL 4.0.0 |
| Previous Message | Peter Smith | 2026-05-26 05:56:45 | Re: Support EXCEPT for TABLES IN SCHEMA publications |