On Mon, Apr 29, 2019 at 6:11 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> > [ jsonpath-errors-improve-3.patch ]
>
> This is getting better, but IMO it's still a bit too willing to use
> a boilerplate primary error message plus errdetail. I do not think
> that is project style nor something to be encouraged.
>
> In particular, you've got a whole lot of cases like this:
>
> + errmsg("JSON object is expected"),
> + errdetail("Jsonpath member accessor can only be applied to an object."))));
>
> I think you should just drop the errmsg and use the errdetail as primary
> (with no-initcap and no-trailing-period, of course). I don't see more
> than two or three cases in this whole patch where I'd use an errdetail
> at all; in almost all of them, the proposed errdetail looks perfectly
> suitable to be a primary message.
Ok, now it seems that I understood. errdetail is removed from vast
majority of cases.
> One other generic gripe is that a lot of these messages use the term
> "singleton", which seems a bit too jargon-y to me. As far as I can
> see in a quick look at the backend .po files, we have not up to now
> used that term in *any* user-facing error message. Nor does it appear
> anywhere in our user-facing documentation, except for one place that
> was itself inserted by the jsonpath patch:
>
> Arrays of size 1 are interchangeable with a singleton.
>
> I don't think that's either obvious to a non-mathematician, or
> even technically correct; maybe it'd be better as
>
> An array of size 1 is considered equal to its sole element.
>
> Likewise, I think it'd be better to avoid "singleton" in the error
> messages. In some places you could perhaps use "single" instead.
> In some you just don't need it at all, eg in
> Jsonpath array subscript is not a singleton numeric value.
> you could just drop the word "singleton" and it'd be perfectly
> correct, since a numeric is necessarily a single value.
>
> Also, we do often use the term "scalar" to mean a non-composite
> value; maybe that would work for this context, in places where
> you do really need that meaning.
Makes sense for me. "Singleton" word comes from the standard. But
assuming we almost don't use it in the documentation (and especially
don't define it), it's better to get rid of this word altogether.
Removed from error messages. Separate patch adjusting docs as you
proposed is also attached.
> Sorry to be making you work so hard on this, but I think good
> error messages are an important part of having a quality feature.
> I do see a lot of improvements already compared to where we started.
It's nothing to be sorry about. I need to learn this in order to make
my further commits better. Thank you for your explanations.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company