public inbox for [email protected]  

help / color / mirror / Atom feed

From: shveta malik <[email protected]>
To: Peter Smith <[email protected]>
Cc: vignesh C <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: shveta malik <[email protected]>
Subject: Re: Logical Replication - revisit `is_table_publication` function implementation
Date: Thu, 9 Apr 2026 09:39:35 +0530
Message-ID: <CAJpy0uA9rj1xDDot3ydn_DZERH4Mc8D70syE3Xg8LWfS8zanZg@mail.gmail.com> (raw)
In-Reply-To: <CAHut+PsjrPOcs=ePWm+N-q=rdmhDeM-FE05gPgDoN647Jb3RaQ@mail.gmail.com>
References: <CAHut+Pti83yGaV5-DZU=AvJHxFDuoKW8_pjSedRham8SgZxLYA@mail.gmail.com>
	<CALDaNm0nLdBKJVHVvvOnY_5mkVg20=OL18fdjA5+KZ3GhPB=TQ@mail.gmail.com>
	<CAHut+Pv+a-7-NRrZv4v6RfaaUo5b21RXe0tGOu8CfKrxPjE=tw@mail.gmail.com>
	<CAJpy0uAHe88hL5MX3q9tyGyx_gCKKHcWnmETXvXM2CqnE8jrmA@mail.gmail.com>
	<CAHut+PtfHzxHFkHJWYoxOFpgpSH5HNAGmP5sSrwh1d+R0Ab-BQ@mail.gmail.com>
	<CAHut+PsjrPOcs=ePWm+N-q=rdmhDeM-FE05gPgDoN647Jb3RaQ@mail.gmail.com>

On Wed, Apr 8, 2026 at 11:58 AM Peter Smith <[email protected]> wrote:
>
> On Wed, Apr 8, 2026 at 4:04 PM Peter Smith <[email protected]> wrote:
> >
> > On Wed, Apr 8, 2026 at 3:25 PM shveta malik <[email protected]> wrote:
> > >
> > > On Wed, Apr 8, 2026 at 10:24 AM Peter Smith <[email protected]> wrote:
> > > >
> > > > On Wed, Apr 8, 2026 at 1:45 PM vignesh C <[email protected]> wrote:
> > > > >
> ...
> > > > > And also why just check for puballtables why not to check for puballsequences
> > > >
> > > > I think function is_schema_publication() is unrelated to 'puballsequences'.
> > > >
> > > > e.g. all the following will still need to check
> > > > pg_publication_namespace, regardless of the 'puballsequences' value.
> > > >
> > > > ex1. CREATE PUBLICATION ... FOR ALL SEQUENCES;
> > > > ex2. CREATE PUBLICATION ... FOR ALL SEQUENCES, FOR TABLES IN SCHEMA s1;
> > > > ex3. CREATE PUBLICATION ... FOR TABLES IN SCHEMA s1;
> > > >
> > >
> > > IIUC, we don't support mix of ALL SEQUENCES and TABLES IN SCHEMA s1.
> > > So I could not understand your point, why FOR ALL SEQ still need to
> > > check pg_publication_namespace?
> > >
> >
> > Oh! You are right.
> >
> > (Sorry, Vignesh, I did not recognise that combination as unsupported).
> >
> > I'll post a patch update to handle it.
> >
>
> PSA patch v2.
>
> Same as before, but now also doing a quick return false from both
> functions if `puballsequences` is true.
>

Okay. I was trying to determine where this optimization would be beneficial.

In cases, where we attempt to add tables or schemas to an ALL TABLES
or ALL SEQUENCES publication, the operation will error out in
CheckAlterPublication() before is_table_publication() or
is_schema_publication() are even called. And in cases where we are
trying to add table or schema to a non ALL-TABLEs/SEQ pub, and we end
up invoking these functions, we still need to traverse pg_pub_rel.

The only scenario (as I understand it) that benefits from this change
is when we try to add EXCEPT to an ALL TABLES publication. In that
case, both of the concerned functions would not need to access
pg_pub_rel if the publication is already an ALL TABLES publication. So
this optimization helps in a positive (non-erroneous) case.

In cases where we need to throw an error (for example, adding EXCEPT
to a FOR TABLE publication), these checks would not provide any
benefit as we still need to traverse pg_pub_rel to see if it has any
valid tables or it is an emty publication (empty one is fine).

But since the optimization improves a valid, non-erroneous scenario,
IMO, it is good to include it.  Let's see what others have to say on
this.

thanks
Shveta

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Logical Replication - revisit `is_table_publication` function implementation
  In-Reply-To: <CAJpy0uA9rj1xDDot3ydn_DZERH4Mc8D70syE3Xg8LWfS8zanZg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox