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

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.

-- 
With Regards,
Amit Kapila.