On Fri, Sep 17, 2021 at 10:09 PM vignesh C <vignes...@gmail.com> wrote: > > Attached v29 patch has the fixes for the same. >
Some minor comments on the v29-0002 patch: (1) In get_object_address_publication_schema(), the error message: + errmsg("publication tables of schema \"%s\" in publication \"%s\" does not exist", isn't grammatically correct. It should probably be: + errmsg("publication tables of schema \"%s\" in publication \"%s\" do not exist", (2) getPublicationSchemaInfo() function header. "string" should be "strings" BEFORE: + * nspname. Both pubname and nspname are palloc'd string which will be freed by AFTER: + * nspname. Both pubname and nspname are palloc'd strings which will be freed by (3) getPublicationSchemaInfo() In the case of "if (!(*nspname))", the following line should probably be added before returning false: *pubname = NULL; (4) GetAllSchemasPublicationRelations() function header Shouldn't: + * Gets the list of all relations published by FOR ALL TABLES IN SCHEMA + * publication(s). actually say: + * Gets the list of all relations published by a FOR ALL TABLES IN SCHEMA + * publication. since it is for a specified publication? (5) I'm wondering, in CheckObjSchemaNotAlreadyInPublication(), instead of checking "checkobjtype" each loop iteration, wouldn't it be better to just use the same for-loop in each IF block? Regards, Greg Nancarrow Fujitsu Australia