Re: Logical Replication of sequences

From: vignesh C <vignesh21(at)gmail(dot)com>
To: shveta malik <shveta(dot)malik(at)gmail(dot)com>
Cc: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Shinya Kato <shinya11(dot)kato(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>
Subject: Re: Logical Replication of sequences
Date: 2025-12-18 12:00:12
Message-ID: CALDaNm3f8v7LAcj+DsitC1d6MjuEnyL-N5KQL3kUVzA=+WcOdg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Regards,
Vignesh

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2025-12-18 12:05:45 Re: pg_dump crash due to incomplete ordering of DO_SUBSCRIPTION_REL objects
Previous Message Alexander Korotkov 2025-12-18 11:57:46 Re: Newly created replication slot may be invalidated by checkpoint