Hi!
Thank you for your review!
On Mon, Apr 22, 2019 at 1:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> RETURN_ERROR(ereport(ERROR,
> (errcode(ERRCODE_JSON_ARRAY_NOT_FOUND),
> errmsg(ERRMSG_JSON_ARRAY_NOT_FOUND),
> errdetail("Jsonpath wildcard array accessor "
>
> In the first place, I'm not certain that this will result in the error
> message being translatable --- do the gettext tools know how to expand
> macros?
>
> In the second place, the actual strings are just restatements of their
> ERRMSG macro names, which IMO is not conformant to our message style,
> but it's too hard to see that from source code like this. Also this
> style is pretty unworkable/unfriendly if the message needs to contain
> any %-markers, so I suspect that having a coding style like this may be
> discouraging you from providing values in places where it'd be helpful to
> do so. What I actually see happening as a consequence of this approach is
> that you're pushing the useful information off to an errdetail, which is
> not really helpful and it's not per project style either. The idea is to
> make the primary message as helpful as possible without being long, not
> to make it a simple restatement of the SQLSTATE that nobody can understand
> without also looking at the errdetail.
>
> In the third place, this makes it hard for people to grep for occurrences
> of an error string in our source code.
>
> And in the fourth place, we don't do this elsewhere; it does not help
> anybody for jsonpath to invent its own coding conventions that are unlike
> the rest of Postgres.
Just to clarify things. Do you propose to get rid of RETURN_ERROR()
macro by expanding it at every occurrence? Or do you have other ideas
in the mind?
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company