On Fri, Nov 15, 2024 at 7:07 PM vignesh C <vignes...@gmail.com> wrote:
>
> On Fri, 15 Nov 2024 at 15:57, Shubham Khanna
> <khannashubham1...@gmail.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'.
> +# --------------------------------------------------
>I have fixed the given comments. The attached Patch contains the required changes. Thanks and regards, Shubham Khanna.
v3-0001-Error-message-improvement.patch
Description: Binary data
