Re: jsonpath versus NaN

On Thu, Jun 18, 2020 at 8:04 PM Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> On Thu, Jun 18, 2020 at 7:45 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)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