Re: Support EXCEPT for ALL SEQUENCES publications

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Shlok Kyal <shlok(dot)kyal(dot)oss(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-22 04:35:11
Message-ID: CAHut+PvXpsFrNn2NOhepVOS7Zf_OvX-m71Wu+m-o_q9odYFXnA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

~

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.

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

//////////
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.).

======
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message JoongHyuk Shin 2026-06-22 04:37:27 Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks
Previous Message Bertrand Drouvot 2026-06-22 04:26:42 Re: [PATCH] Change wait_time column of pg_stat_lock to double precision