Re: Added schema level support for publication.

On Fri, Sep 17, 2021 at 10:09 PM vignesh C <vignesh21@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