Re: Redundant errdetail prefix "The error was:" in some logical replication messages

On Tue, Mar 30, 2021 at 11:40 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Peter Smith <smithpb2250@gmail.com> writes:
> > There are a couple of error messages within the logical replication
> > code where the errdetail text includes a prefix of "The error was:"
>
> Hmm, isn't project style more usually to include the error reason
> in the primary message?  That is,
>
>              ereport(LOG,
> -                    (errmsg("could not drop the replication slot \"%s\" on publisher",
> -                            slotname),
> -                     errdetail("The error was: %s", res->err)));
> +                    (errmsg("could not drop the replication slot \"%s\" on publisher: %s",
> +                            slotname, res->err)));
>
> and so on.  If we had reason to think that res->err would be extremely
> long, maybe pushing it to errdetail would be sensible, but I'm not
> seeing that that is likely.
>
> (I think the "the" before "replication slot" could go away, too.)

+1 to have the res->err in the primary message itself and get rid of
errdetail. Currently the error "could not fetch table info for table"
does that.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com