Thread

  1. Re: Logical Replication of sequences

    vignesh C <vignesh21@gmail.com> — 2025-12-18T12:00:12Z

    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