Re: Support EXCEPT for ALL SEQUENCES publications

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-06-23 09:33:20
Message-ID: CANhcyEWfzDRCiny6A_swAD7WUgdb37-TA4xTAUQ3JJXK9UPBZQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 22 Jun 2026 at 10:05, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Shlok.
>
> Some review comments for v11-0001 and v11-0002.
>
> //////////
> v11-0001
>
> ======
> doc/src/sgml/ref/create_publication.sgml
>
> (EXCEPT)
>
> 1.
> The recent commit 77b6dd9 added some more information to say:
>
> ---
> Once a table is excluded, the exclusion applies to that table
> regardless of its name or schema. Renaming the table or moving it to
> another schema using <command>ALTER TABLE ... SET SCHEMA</command> does
> not remove the exclusion.
> ---
>
> 1a
> AFAIK this same rule (the exclusion follows the object regardless of
> the schema) is going to apply also for excluding sequences. So, the
> docs should be updated to say something similar about behaviour for
> sequences.
>
Modified
> ~
>
> 1b.
> Maybe you need to add another test case to exclude some sequence, then
> alter the sequence's schema, then verify that the moved sequence is
> still excluded.
>
Added the test

> ~~~
> On Thu, Jun 18, 2026 at 11:29 PM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
> >
> > On Wed, 17 Jun 2026 at 12:37, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> ...
> > > ======
> > > src/bin/psql/describe.c
> > >
> > > describeOneTableDetails:
> > >
> > > 5.
> > > printfPQExpBuffer(&buf, "/* %s */\n",
> > > _("Get publications containing this sequence"));
> > > - appendPQExpBuffer(&buf, "SELECT pubname FROM pg_catalog.pg_publication p"
> > > + appendPQExpBuffer(&buf, "SELECT p.pubname FROM pg_catalog.pg_publication p"
> > > "\nWHERE p.puballsequences"
> > > "\n AND pg_catalog.pg_relation_is_publishable('%s')"
> > > + "\n AND NOT EXISTS (\n"
> > > + " SELECT 1\n"
> > > + " FROM pg_catalog.pg_publication_rel pr\n"
> > > + " WHERE pr.prpubid = p.oid AND\n"
> > > + " pr.prrelid = '%s')"
> > > "\nORDER BY 1",
> > > - oid);
> > > + oid, oid);
> > >
> > > 5a.
> > > IIUC, the "NOT EXISTS" part is for skipping the excluded sequences.
> > > And, you don't say "AND pr.prexcept" because it is redundant since
> > > the only way for a SEQUENCE to be individually in pg_publication_rel
> > > is if it is an excluded sequence.
> > >
> > > Is my understanding correct? I think it is too subtle -- it might be
> > > better to say "AND pr.prexcept" even if it is not strictly needed.
> > >
> > Yes, your understanding is correct.
> > For ALL SEQUENCES publication, the pg_publication_rel will have the
> > entry only if the sequence is specified in the EXCEPT list.
> > I didn't add "AND pr.prexcept" because the corresponding code for ALL
> > TABLES publications also does not use "AND pr.prexcept".
> ...
> > I think it will be consistent with EXCEPT (TABLE .. ) case. So, I
> > think we should not add "AND pr.prexcept".
>
> That’s OK, but I felt if you are going to omit the 'prexcept', then
> perhaps there should be a comment to explain (in all places) why it is
> ok to do that.
>
Added the comments

> //////////
> v11-0002.
>
> ======
> doc/src/sgml/ref/alter_publication.sgml
>
> 1.
> ...or marks the publication as FOR ALL SEQUENCES or FOR ALL TABLES,
> optionally using EXCEPT to exclude specific tables or sequences.
>
> ~
>
> That current documentation sentence seems muddled because the
> publication types are not in the same as the order of the excluded
> types.
>
> IMO, alternatives below are better:
> a) ...FOR ALL SEQUENCES or FOR ALL TABLES, optionally using EXCEPT to
> exclude specific sequences or tables.
> b) ...FOR ALL TABLES or FOR ALL SEQUENCES, optionally using EXCEPT to
> exclude specific tables or sequences.
>
> (I preferred b, since everywhere else seems to be ordered
> tables/sequences not sequences/tables.).
>
I also think (b) is better.

I have addressed the comments and attached the updated v12 patch

Thanks,
Shlok Kyal

Attachment Content-Type Size
v12-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLIC.patch application/octet-stream 27.8 KB
v12-0001-Support-EXCEPT-for-ALL-SEQUENCES-in-CREATE-PUBLI.patch application/octet-stream 61.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Rafia Sabih 2026-06-23 09:38:41 Re: Bypassing cursors in postgres_fdw to enable parallel plans
Previous Message Chao Li 2026-06-23 09:29:04 Re: Fix variadic argument types for pg_get_xxx_ddl() functions