On Thu, Dec 18, 2025 at 12:37 PM Shlok Kyal <[email protected]> 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.
