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?
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.
--
With Regards,
Amit Kapila.
