On 2023-11-06 Mo 08:23, Jeevan Chalke wrote:
On Wed, Nov 1, 2023 at 3:49 PM Andrew Dunstan <and...@dunslane.net> wrote:
On 2023-11-01 We 03:00, Jeevan Chalke wrote:
Hello,
On Tue, Oct 24, 2023 at 6:41 PM Andrew Dunstan
<and...@dunslane.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
<pe...@eisentraut.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@~=# 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
