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

On Tue, Mar 30, 2021 at 11:40 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Peter Smith <smithpb2250(at)gmail(dot)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