On Thu, Sep 30, 2021 at 3:39 PM vignesh C <vignes...@gmail.com> wrote: > > The suggested change works, I have modified it in the attached patch. >
I have reviewed the latest version and made a number of changes to the 0001 patch. The changes are in v1-0001-Changes-by-Amit. It includes (a) Changed preprocess_pubobj_list() to make the code easy to understand, (b) the handling of few variables was missing in equal function, (c) the ordering of functions, and a few parameters were not matching .c and .h files, (d) added/edited multiple comments and other cosmetic changes. Apart from that, I have few other comments: 1. It seems you have started using unique list variants in GetPubPartitionOptionRelations because one of its caller GetSchemaPublicationRelations need it. I think the unique variants are costlier, so isn't it better to use it where it is required? I think it would be good to use in GetPubPartitionOptionRelations, if most of its caller requires the same. 2. In GetSchemaPublicationRelations(), I think we need to perform a second scan using RELKIND_PARTITIONED_TABLE only if we publish_via_root (aka pub_partopt is PUBLICATION_PART_ROOT). This is what we are doing in GetAllTablesPublicationRelations. Is there a reason to be different here? 3. @@ -538,7 +788,7 @@ RemovePublicationById(Oid pubid) if (!HeapTupleIsValid(tup)) elog(ERROR, "cache lookup failed for publication %u", pubid); - pubform = (Form_pg_publication)GETSTRUCT(tup); + pubform = (Form_pg_publication) GETSTRUCT(tup); We don't need the above change for this patch. I think this may be due pgindent but we can do this separately rather than as part of this patch. -- With Regards, Amit Kapila.
v35-0001-Added-schema-level-support-for-publication.patch
Description: Binary data
v1-0001-Changes-by-Amit.patch
Description: Binary data
