Re: jsonpath - Mailing list pgsql-hackers

From Tom Lane
Subject Re: jsonpath
Date
Msg-id 23737.1556550645@sss.pgh.pa.us
Whole thread Raw
In response to Re: jsonpath  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Responses Re: jsonpath
List pgsql-hackers
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.

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.

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.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: pg_ssl
Next
From: Andres Freund
Date:
Subject: Re: "long" type is not appropriate for counting tuples