Horton

Re: Added schema level support for publication.

Greg Nancarrow <gregn4422@gmail.com>

View thread View on postgr.es
From: Greg Nancarrow <gregn4422@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Smith <smithpb2250@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, Ajin Cherian <itsajin@gmail.com>, Rahila Syed <rahilasyed90@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2021-09-21T03:33:43Z
Lists: pgsql-hackers 
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

Commits

  1. Include schema/table publications even with exclude options in dump.

  2. Rename some enums to use TABLE instead of REL.

  3. Add tap tests for the schema publications.

  4. Allow publishing the tables of schema.

  5. In pg_dump, use simplehash.h to look up dumpable objects by OID.