RE: why can't a table be part of the same publication as its schema

On Friday, September 16, 2022 1:42 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Thu, Sep 15, 2022 at 6:27 PM houzj.fnst@fujitsu.com
> <houzj.fnst@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