MIME-Version: 1.0
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>
In-Reply-To: 
 <CAHut+Pv+a-7-NRrZv4v6RfaaUo5b21RXe0tGOu8CfKrxPjE=tw@mail.gmail.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Wed, 8 Apr 2026 10:54:51 +0530
Message-ID: 
 <CAJpy0uAHe88hL5MX3q9tyGyx_gCKKHcWnmETXvXM2CqnE8jrmA@mail.gmail.com>
Subject: Re: Logical Replication - revisit `is_table_publication` function
 implementation
To: Peter Smith <smithpb2250@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAJpy0uAHe88hL5MX3q9tyGyx_gCKKHcWnmETXvXM2CqnE8jrmA%40mail.gmail.com>
Precedence: bulk

On Wed, Apr 8, 2026 at 10:24=E2=80=AFAM Peter Smith <smithpb2250@gmail.com>=
 wrote:
>
> On Wed, Apr 8, 2026 at 1:45=E2=80=AFPM vignesh C <vignesh21@gmail.com> wr=
ote:
> >
> > On Tue, 7 Apr 2026 at 12:32, Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > Hi, after confirming my understanding of pg_publication_rel [1], I
> > > revisited some logical replication internal functions.
> > >
> > > Specifically.
> > > * The `is_table_publication` function is for checking if the
> > > publication has a clause like "FOR TABLE t1".
> > > * The `is_schema_publication` function is for checking if the
> > > publication has a clause like "FOR TABLES IN SCHEMA s1".
> > >
> > > Notice that neither of these ("FOR TABLE", "FOR TABLES IN SCHEMA")
> > > clauses are possible simultaneously with "FOR ALL TABLES".
> > >
> > > And we can readily discover if "FOR ALL TABLES" (aka `puballtables`)
> > > is present from the pubform.
> > >
> > > We can use this to optimise and simplify the implementations of the
> > > `is_schema_publication` and `is_table_publication` functions.
> > >
> > > PSA patch v1.
> > >
> > > AFAICT, the result is:
> > > - less code + simpler logic. e.g. is_table_publication does not check
> > > 'prexcept' anymore
> > > - more efficient. e.g. skips unnecessary scanning when puballtables i=
s true.
> > > - more consistent. e.g., both functions are now almost identical.
> > >
> > > Thoughts?
> >
>
> Hi Vignesh. Thanks for reviewing!
>
> > I'm not sure if this additional check is sufficient in case of
> > is_schema_publication. Checking only puballtables can exclude FOR ALL
> > TABLES, but it still cannot distinguish regular table publications,
> > empty publications, or sequence publications. In all of those cases,
> > we still need to check pg_publication_namespace.
>
> Yes, this condition is only an optimisation for FOR ALL TABLES, as the
> comment says.
>
> IMO, the overhead of 1 additional boolean check for cases where it
> doesn't help is an insignificant trade-off for the savings when it can
> return false.
>
> > And also why just check for puballtables why not to check for puballseq=
uences
>
> 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?

thanks
Shveta