Re: Improve the error message for logical replication of regular column to generated column.

On Fri, 15 Nov 2024 at 15:57, Shubham Khanna
<khannashubham1197(at)gmail(dot)com> wrote:
>
> I have fixed the given comments. The attached Patch contains the
> required changes.

Few comments:
1)
a)You can mention that "If ismissing is true, report the error message
as 'Missing replicated columns.' Otherwise, report the error message
as 'Cannot replicate to generated column."
/*
- * Report error with names of the missing local relation column(s), if any.
+ * Report error with names of the missing and generated local
relation column(s), if any.
*/

b) You can keep the line within 80 chars in this case.

2) Spurious blank line:
+ ereport(ERROR,
+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg_plural("logical
replication target relation \"%s.%s\" is missing replicated column:
%s",
+
"logical replication target relation \"%s.%s\" is missing replicated
columns: %s",
+ attcnt,
+
remoterel->nspname,
+
remoterel->relname,
+
attsbuf.data)));
+
+ else
+ ereport(ERROR,
+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg_plural("cannot
replicate to target relation \"%s.%s\" generated column: %s",
+
"cannot replicate to target relation \"%s.%s\" generated columns: %s",
+ attcnt,
+
remoterel->nspname,
+
remoterel->relname,
+
attsbuf.data)));

3) This comment is not correct as the definition of
generated(publisher) to generated(subscriber) can be same:
+ /*
+ * Add to generatedattrs if names
match but definitions
+ * differ.
+ */
+ if (attr->attgenerated)
+ generatedattrs =
bms_add_member(generatedattrs, i);

4)
a) You can use "regular" instead of "normal":
+# A "normal -> generated" and "generated -> generated" replication fails,
+# reporting an error that the generated column on the subscriber side
+# cannot be replicated.
+#
+# Test Case: normal -> generated and generated -> generated
+# Publisher table has regular column 'c2' and generated column 'c3'.
+# Subscriber table has generated columns 'c2' and 'c3'.

b) similarly here too:
+# --------------------------------------------------
+# A "normal -> missing" replication fails, reporting an error
+# that the subscriber side is missing replicated columns.
+#
+# Testcase: normal -> missing
+# Publisher table has normal columns 'c2' and 'c3'.
+# Subscriber table is missing columns 'c2' and 'c3'.
+# --------------------------------------------------

Regards,
Vignesh