Re: jsonpath versus NaN
Alexander Korotkov <aekorotkov@gmail.com>
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-07-06T12:19:21Z
Lists: pgsql-hackers
Attachments
- 0001-Improve-error-reporting-for-jsonpath-.double-method.patch (application/octet-stream) patch 0001
- 0002-Forbid-NaNs-in-jsonpath.patch (application/octet-stream) patch 0002
On Thu, Jun 18, 2020 at 8:04 PM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote: > On Thu, Jun 18, 2020 at 7:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Alexander Korotkov <a.korotkov@postgrespro.ru> writes: > > > Thank you for your answer. I'm trying to understand your point. > > > Standard claims that .double() method should behave the same way as > > > CAST to double. However, standard references the standard behavior of > > > CAST here, not behavior of your implementation of CAST. So, if we > > > extend the functionality of standard CAST in our implementation, that > > > doesn't automatically mean we should extend the .double() jsonpath > > > method in the same way. Is it correct? > > > > Right. We could, if we chose, extend jsonpath to allow Inf/NaN, but > > I don't believe there's an argument that the spec requires us to. > > > > Also the larger point is that it doesn't make sense to extend jsonpath > > that way when we haven't extended json(b) that way. This code wart > > wouldn't exist were it not for that inconsistency. Also, I find it hard > > to see why anyone would have a use for NaN in a jsonpath test when they > > can't write NaN in the input json data, nor have it be correctly reflected > > into output json data either. > > Ok, I got the point. I have nothing against removing support of NaN > in jsonpath as far as it doesn't violates the standard. I'm going to > write the patch for this. The patchset is attached, sorry for the delay. The first patch improves error messages, which appears to be unclear for me. If one applies .double() method to a numeric value, we restrict that this numeric value should fit to double precision type. If it doesn't fit, the current error message just says the following. ERROR: jsonpath item method .double() can only be applied to a numeric value But that's confusing, because .double() method is naturally applied to a numeric value. Patch makes this message explicitly report that numeric value is out of range for double type. This patch also adds test exercising this error. When string can't be converted to double precision, I think it's better to explicitly say that we expected valid string representation of double precision type. Second patch forbids to convert NaN using .double() method. As I get, NaN can't be result of any jsonpath computations assuming there is no NaN input. So, I just put an assert to convertJsonbScalar() ensuring there is no NaN in JsonbValue. ------ Regards, Alexander Korotkov
Commits
-
Forbid numeric NaN in jsonpath
- f4ae676e3178 12.4 landed
- 89a0b1a7ca0a 13.0 landed
- df646509f371 14.0 landed
-
Improve error reporting for jsonpath .double() method
- 3ec5f6b53dfb 12.4 landed
- b9a04a9bc665 13.0 landed
- 065718116746 14.0 landed
-
Partial implementation of SQL/JSON path language
- 72b6460336e8 12.0 cited