pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: John Naylor <john.naylor@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: jsonpath
Date: Mon, 29 Apr 2019 11:10:45 -0400
Message-ID: <23737.1556550645@sss.pgh.pa.us> (raw)
In-Reply-To: <CAPpHfdtgFgUuXKjpz0qEH2Q5+S-T2XJJ8_PodBNA0_vRabJiig@mail.gmail.com>
References: <CAPpHfdvkMdV_nQR3gO41GWJS=2Gjdrm6QhhYH2DwXJ+fMa6gBw@mail.gmail.com>
	<23e82578-b648-ab57-a4af-791abccff344@2ndQuadrant.com>
	<dcff626f-d44d-ad96-ecc0-109601e2cfb9@2ndQuadrant.com>
	<4971.1553779874@sss.pgh.pa.us>
	<5656.1553781027@sss.pgh.pa.us>
	<b2e3d035-4f42-4670-97ad-e79727c40960@2ndQuadrant.com>
	<CAPpHfdu-MvmN9pJnTtTCyuzbcF4t2K4hsfMdEtTGjfx7ZbGVYw@mail.gmail.com>
	<CAPpHfdue-pzzEVK+zoY6sbqG4LYC3JvW7-j8J78EqT-TOLdNiw@mail.gmail.com>
	<19611.1554593792@sss.pgh.pa.us>
	<CAPpHfduP2ShBe3r80u0zsOGxRG1XRd4c7PTj-iPfmv4DzL2dyQ@mail.gmail.com>
	<20190409161557.xwc4xwqeaimxcb4d@development>
	<CAPpHfdvtb3MivKZA-8DUBJux=DwkR2Kx2qoHcw+_TKW7Dwv_Ew@mail.gmail.com>
	<1ccb6aa8-0ba5-a14c-d3a3-8c0b7f73244b@2ndQuadrant.com>
	<CACPNZCt5B8jDCCGQiFoSuqmg-za_NCy4QDioBTLaNRih9+-bXg@mail.gmail.com>
	<14890.1555523005@sss.pgh.pa.us>
	<CAPpHfdsKYKDvMXbVU-ojzams0NcMVzE0Fuyg-abxpQGr=0QRcg@mail.gmail.com>
	<CAPpHfds_A=nfJypMNyU3+HxAs3tzT++eGzzQn-QcUYofyjCFJQ@mail.gmail.com>
	<CAPpHfduxZqqnA1CU232+tbB6OrC! dM8eHJUTGQO6G0BZ7tnXxxQ@mail.gmail.com>
	<CAPpHfdu_fX=R1tPCAFeQJRhyapcvhBu19Z2Mk8ibP09CVOu3sw@mail.gmail.com>
	<28019.1556220532@sss.pgh.pa.us>
	<CAPpHfdtgFgUuXKjpz0qEH2Q5+S-T2XJJ8_PodBNA0_vRabJiig@mail.gmail.com>

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





view thread (198+ messages)  latest in thread

Message-ID: <23737.1556550645@sss.pgh.pa.us>
Permalink:  ../23737.1556550645@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/23737.1556550645@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, a.korotkov@postgrespro.ru, john.naylor@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: jsonpath
  In-Reply-To: <23737.1556550645@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox