Re: Logical Replication of sequences

On Thu, Dec 18, 2025 at 5:30 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, 18 Dec 2025 at 17:03, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Thu, Dec 18, 2025 at 3:04 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >
> > > On Thu, 18 Dec 2025 at 12:37, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)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.
>

Okay, the patch LGTM.

thanks
Shveta