public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrew Dunstan <[email protected]>
To: Jeevan Chalke <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: More new SQL/JSON item methods
Date: Sun, 3 Dec 2023 11:14:15 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAM2+6=UrDfcsz0xn+oUmomOnPYazuP8dAM_Wrx5YMsWxXjWyKA@mail.gmail.com>
References: <CAM2+6=XjTyqrrqHAOj80r0wVQxJSxc0iyib9bPC55uFO9VKatg@mail.gmail.com>
<[email protected]>
<CAM2+6=XTW1COd+56ekjP=CUceZyrgyrcBge_z0ZRkCgVwcGHQg@mail.gmail.com>
<[email protected]>
<CAM2+6=UCSp74devvHYpMx5NvpyWbCO3gYvgpEYHtNYqF2j6Z9Q@mail.gmail.com>
<[email protected]>
<CAM2+6=UrDfcsz0xn+oUmomOnPYazuP8dAM_Wrx5YMsWxXjWyKA@mail.gmail.com>
On 2023-11-06 Mo 08:23, Jeevan Chalke wrote:
>
>
> On Wed, Nov 1, 2023 at 3:49 PM Andrew Dunstan <[email protected]> wrote:
>
>
> On 2023-11-01 We 03:00, Jeevan Chalke wrote:
>> Hello,
>>
>> On Tue, Oct 24, 2023 at 6:41 PM Andrew Dunstan
>> <[email protected]> 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
>>> <[email protected]> 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@~=# 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@~=# 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@~=# 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
view thread (11+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: More new SQL/JSON item methods
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox