Thread

  1. Re: Logical Replication of sequences

    Shlok Kyal <shlok.kyal.oss@gmail.com> — 2025-12-24T06:45:19Z

    On Mon, 22 Dec 2025 at 11:08, Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Thu, Dec 18, 2025 at 12:37 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
    > >
    > > 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.
    > >
    >
    > I see that we will needlessly call GetPublicationRelations or others
    > for all_schema publication but is there any problem/bug due to that?
    No, I did not encounter a problem/bug.
    
    > AFAICS, the function will still return correct results. Yes, there is
    > an argument to better performance for large numbers of all_sequence
    > publications and that too in DDL like Create/Alter Subscription. I am
    > not sure that it is really worth adding more checks at multiple places
    > in the code though we can improve comments atop
    > GetPublicationRelations. I feel if we encounter such cases in the
    > field then it makes sense to add these additional optimizations at
    > various places.
    >
    I agree with you. And attached a patch to modify the comment above
    GetPublicationRelations.
    
    Thanks,
    Shlok Kyal