Re: More new SQL/JSON item methods

On 2023-11-06 Mo 08:23, Jeevan Chalke wrote:
>
>
> On Wed, Nov 1, 2023 at 3:49 PM Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
> On 2023-11-01 We 03:00, Jeevan Chalke wrote:
>> Hello,
>>
>> On Tue, Oct 24, 2023 at 6:41 PM Andrew Dunstan
>> <andrew(at)dunslane(dot)net> wrote:
>>
>>
>> On 2023-10-19 Th 02:06, Jeevan Chalke wrote:
>>> Thanks, Peter for the comments.
>>>
>>> On Fri, Oct 6, 2023 at 5:13 PM Peter Eisentraut
>>> <peter(at)eisentraut(dot)org> wrote:
>>>
>>> On 29.08.23 09:05, Jeevan Chalke wrote:
>>> >
>>> v1-0001-Implement-jsonpath-.bigint-.integer-and-.number-m.patch
>>> >
>>> > This commit implements jsonpath .bigint(), .integer(),
>>> and .number()
>>> > methods.  The JSON string or a numeric value is
>>> converted to the
>>> > bigint, int4, and numeric type representation.
>>>
>>> A comment that applies to all of these: These add
>>> various keywords,
>>> switch cases, documentation entries in some order.  Are
>>> we happy with
>>> that?  Should we try to reorder all of that for better
>>> maintainability
>>> or readability?
>>>
>>>
>>> Yeah, that's the better suggestion. While implementing these
>>> methods, I was confused about where to put them exactly and
>>> tried keeping them in some logical place.
>>> I think once these methods get in, we can have a follow-up
>>> patch reorganizing all of these.
>>
>>
>> I think it would be better to organize things how we want
>> them before adding in more stuff.
>>
>>
>> I have tried reordering all the jsonpath Operators and Methods
>> consistently. With this patch, they all appear in the same order
>> when together in the group.
>>
>> In some switch cases, they are still divided, like in
>> flattenJsonPathParseItem(), where 2-arg, 1-arg, and no-arg cases
>> are clubbed together. But I have tried to keep them in order in
>> those subgroups.
>>
>> I will rebase my patches for this task on this patch, but before
>> doing so, I  would like to get your views on this reordering.
>>
>>
>
> This appears to be reasonable. Maybe we need to add a note in one
> or two places about maintaining the consistency?
>
> +1
> Added a note in jsonpath.h where enums are defined.
>
> I have rebased all three patches over this reordering patch making 4
> patches in the set.
>
> Let me know your views on the same.
>
> Thanks
>
>

Hi Jeevan,

I think these are in reasonably good shape, but there are a few things
that concern me:

andrew(at)~=# select jsonb_path_query_array('[1.2]', '$[*].bigint()');
ERROR:  numeric argument of jsonpath item method .bigint() is out of
range for type bigint

I'm ok with this being an error, but I think the error message is wrong.
It should be the "invalid input" message.

andrew(at)~=# select jsonb_path_query_array('[1.0]', '$[*].bigint()');
ERROR:  numeric argument of jsonpath item method .bigint() is out of
range for type bigint

Should we trim trailing dot+zeros from numeric values before trying to
convert to bigint/int? If not, this too should be an "invalid input" case.

andrew(at)~=# select jsonb_path_query_array('[1.0]', '$[*].boolean()');
ERROR:  numeric argument of jsonpath item method .boolean() is out of
range for type boolean

It seems odd that any non-zero integer is true but not any non-zero
numeric. Is that in the spec? If not I'd avoid trying to convert it to
an integer first, and just check for infinity/nan before looking to see
if it's zero.

The code for integer() and bigint() seems a bit duplicative, but I'm not
sure there's a clean way of avoiding that.

The items for datetime types and string look OK.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com