| 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
| 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 |