On Friday, September 16, 2022 1:42 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Sep 15, 2022 at 6:27 PM houzj.f...@fujitsu.com > <houzj.f...@fujitsu.com> wrote: > > > > Attach the new version patch which added suggested restriction for > > column list and merged Vignesh's patch. > > > > Few comments: > ============ > 1. > static void > -CheckPubRelationColumnList(List *tables, const char *queryString, > +CheckPubRelationColumnList(List *tables, bool publish_schema, > + const char *queryString, > bool pubviaroot) > > It is better to keep bool parameters together at the end. > > 2. > /* > + * Disallow using column list if any schema is in the publication. > + */ > + if (publish_schema) > + ereport(ERROR, > + errcode(ERRCODE_INVALID_PARAMETER_VALUE), > + errmsg("cannot use publication column list for relation \"%s.%s\"", > + get_namespace_name(RelationGetNamespace(pri->relation)), > + RelationGetRelationName(pri->relation)), > + errdetail("Column list cannot be specified if any schema is part of > the publication or specified in the list.")); > > I think it would be better to explain why we disallow this case. > > 3. > + if (!heap_attisnull(coltuple, Anum_pg_publication_rel_prattrs, NULL)) > + ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), > + errmsg("cannot add schema to the publication"), errdetail("Schema > + cannot be added if any table that specifies column > list is already part of the publication")); > > A full stop is missing at the end in the errdetail message. > > 4. I have modified a few comments in the attached. Please check and if you > like > the changes then please include those in the next version.
Thanks for the comments. Attach the new version patch which addressed above comments and ran pgident. I also improved some codes and documents based on some comments given by Vignesh and Peter offlist. Best regards, Hou zj
v4-0001-Allow-publications-with-schema-and-table-of-the-s.patch
Description: v4-0001-Allow-publications-with-schema-and-table-of-the-s.patch