Re: Logical Replication of sequences - Mailing list pgsql-hackers

From vignesh C
Subject Re: Logical Replication of sequences
Date
Msg-id CALDaNm3f8v7LAcj+DsitC1d6MjuEnyL-N5KQL3kUVzA=+WcOdg@mail.gmail.com
Whole thread Raw
In response to Re: Logical Replication of sequences  (shveta malik <shveta.malik@gmail.com>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: Newly created replication slot may be invalidated by checkpoint
Next
From: vignesh C
Date:
Subject: Re: pg_dump crash due to incomplete ordering of DO_SUBSCRIPTION_REL objects