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

On Tue, Nov 26, 2024 at 1:42 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>.
>
> Few comments:
> 1) Now that attribute string generation is moved to get_attrs_str and
> there are only a couple of error statements in this function, how
> about removing the function:
> +/*
> + * If !bms_is_empty(missingatts), report the error message as 'Missing
> + * replicated columns.' Otherwise, report the error message as
> 'Cannot replicate
> + * to generated columns.'
> + */
> +static void
> +logicalrep_report_missing_and_gen_attrs(LogicalRepRelation *remoterel,
> +
> Bitmapset *missingatts,
> +
> Bitmapset *genatts)
> +{
> + if (!bms_is_empty(missingatts))
> 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",
> - missingattcnt,
> - remoterel->nspname,
> - remoterel->relname,
> -
> missingattsbuf.data)));
> - }
> +
> 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",
> +
> bms_num_members(missingatts),
> + remoterel->nspname,
> + remoterel->relname,
> +
> get_attrs_str(remoterel, missingatts)));
> +
> + if (!bms_is_empty(genatts))
> + 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",
> +
> bms_num_members(genatts),
> + remoterel->nspname,
> + remoterel->relname,
> +
> get_attrs_str(remoterel, genatts)));
> }
>

+1. This idea to just inline those errors instead of calling the
function sounds OK to me too.
Please consider also moving my suggested function comment if you
refactor this way.

======
Kind Regards,
Peter Smith.
Fujitsu Australia