Re: Logical Replication of sequences

From: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, shveta malik <shveta(dot)malik(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 07:06:56
Message-ID: CANhcyEVrragHfJ730+CKvU7=Kgr8Qs-BGk-xpW8e7XTV_T2VEg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 11 Nov 2025 at 16:41, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Tue, 11 Nov 2025 at 11:23, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >
> > Hi Vignesh,
> >
> > A few more comments:
> >
> > > On Nov 7, 2025, at 22:47, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >
> > > The attached v20251107_2 version patch has the changes for the same.
> > >
> > > Regards,
> > > Vignesh
> > > <v20251107_2-0001-Documentation-for-sequence-synchronizati.patch>
> >
> > 1
> > ```
> > - Currently, there can be only one synchronization worker per table.
> > + Currently, there can be only one table synchronization worker per table
> > + and one sequence synchronization worker to synchronize all sequences.
> > ```
> >
> > Feels like this statement is not accurate and leaves an impression that a table has a fixed work to serve it. So I think we can enhance this statement a little bit as:
> >
> > ```
> > Currently, only one table synchronization worker runs per table, and
> > only one sequence synchronization worker runs per subscription at a time.
> > ```
>
> Changed it slightly.
>
> > 2
> > ```
> > +<programlisting>
> > +/* sub # */ CREATE SEQUENCE s1 START WITH 10 INCREMENT BY 1
> > +/* sub # */ CREATE SEQUENCE s2 START WITH 100 INCREMENT BY 10;
> > +</programlisting></para>
> > ```
> >
> > Missed semi-colon for the first SQL statement.
>
> Modified
>
> > 3
> > ```
> > +<programlisting>
> > +/* sub # */ SELECT srrelid::regclass, srsublsn FROM pg_subscription_rel ;
> > ```
> >
> > Unneeded white-space before the semi-colon.
>
> Modified
>
> > 4
> > ```
> > +/* sub # */ SELECT * FROM s2
> > + last_value | log_cnt | is_called
> > +------------+---------+-----------
> > + 610 | 0 | t
> > +(1 row)
> > ```
> >
> > Again, missed semi-colon.
>
> Modified
>
> The attached v20251111 version patch has the changes for the same.

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 Kyal

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2025-12-18 07:22:09 DOC: fixes multiple errors in alter table doc
Previous Message vignesh C 2025-12-18 06:49:21 Re: pg_dump crash due to incomplete ordering of DO_SUBSCRIPTION_REL objects