Re: More new SQL/JSON item methods - Mailing list pgsql-hackers

Thanks, Peter for the comments.

On Fri, Oct 6, 2023 at 5:13 PM Peter Eisentraut <peter@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.

 

> v1-0002-Implement-.date-.time-.time_tz-.timestamp-and-.ti.patch
>
> This commit implements jsonpath .date(), .time(), .time_tz(),
> .timestamp(), .timestamp_tz() methods.  The JSON string representing
> a valid date/time is converted to the specific date or time type
> representation.
>
> The changes use the infrastructure of the .datetime() method and
> perform the datatype conversion as appropriate.  All these methods
> accept no argument and use ISO datetime formats.

These should accept an optional precision argument.  Did you plan to add
that?

Yeah, will add that.

 

> v1-0003-Implement-jsonpath-.boolean-and-.string-methods.patch
>
> This commit implements jsonpath .boolean() and .string() methods.

This contains a compiler warning:

../src/backend/utils/adt/jsonpath_exec.c: In function
'executeItemOptUnwrapTarget':
../src/backend/utils/adt/jsonpath_exec.c:1162:86: error: 'tmp' may be
used uninitialized [-Werror=maybe-uninitialized]

> v1-0004-Implement-jasonpath-.decimal-precision-scale-meth.patch
>
> This commit implements jsonpath .decimal() method with optional
> precision and scale.  If precision and scale are provided, then
> it is converted to the equivalent numerictypmod and applied to the
> numeric number.

This also contains compiler warnings:

Thanks, for reporting these warnings. I don't get those on my machine, thus missed them. Will fix them.

 

../src/backend/utils/adt/jsonpath_exec.c: In function
'executeItemOptUnwrapTarget':
../src/backend/utils/adt/jsonpath_exec.c:1403:53: error: declaration of
'numstr' shadows a previous local [-Werror=shadow=compatible-local]
../src/backend/utils/adt/jsonpath_exec.c:1442:54: error: declaration of
'elem' shadows a previous local [-Werror=shadow=compatible-local]

There is a typo in the commit message: "Implement jasonpath"

Will fix.

 

Any reason this patch is separate from 0002?  Isn't number() and
decimal() pretty similar?

Since DECIMAL has precision and scale arguments, I have implemented that at the end. I tried merging that with 0001, but other patches ended up with the conflicts and thus I didn't merge that and kept it as a separate patch. But yes, logically it belongs to the 0001 group. My bad that I haven't put in that extra effort. Will do that in the next version. Sorry for the same.

 

You could also update src/backend/catalog/sql_features.txt in each patch
(features T865 through T878).

OK.

Thanks 

--

Jeevan Chalke
Senior Staff SDE, Database Architect, and Manager
Product Development



edbpostgres.com