On Tue, Mar 30, 2021 at 5:10 PM 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.)
Thanks for the review and advice.
PSA version 2 of this patch which adopts your suggestions.
------
Kind Regards,
Peter Smith.
Fujitsu Australia