Re: Logical Replication of sequences

On Thu, 18 Dec 2025 at 17:03, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, Dec 18, 2025 at 3:04 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Thu, 18 Dec 2025 at 12:37, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> > >
> > > Hi Team,
> > >
> > > While working on another thread, I noticed a bug introduced by commit
> > > as part of this thread.
> > > In function pg_get_publication_tables, We have code:
> > > ```
> > >                      if (pub_elem->alltables)
> > > pub_elem_tables = GetAllPublicationRelations(RELKIND_RELATION,
> > >  pub_elem->pubviaroot);
> > > else
> > > {
> > > List     *relids,
> > >    *schemarelids;
> > >
> > > relids = GetPublicationRelations(pub_elem->oid,
> > >  pub_elem->pubviaroot ?
> > >  PUBLICATION_PART_ROOT :
> > >  PUBLICATION_PART_LEAF);
> > > schemarelids = GetAllSchemaPublicationRelations(pub_elem->oid,
> > > pub_elem->pubviaroot ?
> > > PUBLICATION_PART_ROOT :
> > > PUBLICATION_PART_LEAF);
> > > pub_elem_tables = list_concat_unique_oid(relids, schemarelids);
> > > }
> > > ```
> > >
> > > So, when we create an 'ALL SEQUENCE publication' and we execute
> > > 'SELECT * from pg_publication_tables'
> > > We will enter the else condition in the above code, which does not
> > > seem correct to me.
> > > It will call functions which are not required to be called. It will
> > > also call the function 'GetPublicationRelations' which contradicts the
> > > comment above this function.
> > >
> > > Similar issue is present for functions "InvalidatePubRelSyncCache" and
> > > "AlterPublicationOptions".
> >
> > Thanks Shlok for reporting these issues,  In the areas highlighted, we
> > can skip processing for sequences-only publications. The attached
> > patch implements these changes.
> >
>
> We have '!pub->alltables' check at 2 more places:
> -- pgoutput_row_filter_init()

It's not necessary here, because only publications that actually
replicate the given relation are passed to pgoutput_row_filter_init.
Sequence publications are excluded from this list, so they will never
appear there.

> -- pg_get_publication_tables()

We don't need to explicitly check !pub->allsequences here. The guard
if (funcctx->call_cntr < list_length(table_infos))
already ensures that the remaining code is executed only when
table_infos contains elements. If the list has entries, it necessarily
represents a table publication, and it will enumerate those tables. If
the list is empty, the condition fails and no rows are returned, so
the extra check is not required.

Regards,
Vignesh