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

From Jeevan Chalke
Subject Re: More new SQL/JSON item methods
Date
Msg-id CAM2+6=WyoinYoiuf3msxqH-hsPBoUz33MmrCpD_4Ceo6Nq1qOg@mail.gmail.com
Whole thread Raw
In response to Re: More new SQL/JSON item methods  (Peter Eisentraut <peter@eisentraut.org>)
Responses Re: More new SQL/JSON item methods
Re: More new SQL/JSON item methods
List pgsql-hackers


On Mon, Jan 15, 2024 at 7:41 PM Peter Eisentraut <peter@eisentraut.org> wrote:
Attached are two small fixup patches for your patch set.

Thanks, Peter.
 

In the first one, I simplified the grammar for the .decimal() method.
It seemed a bit overkill to build a whole list structure when all we
need are 0, 1, or 2 arguments.

Agree.
I added unary '+' and '-' support as well and thus thought of having separate rules altogether rather than folding those in.
 
Per SQL standard, the precision and scale arguments are unsigned
integers, so unary plus and minus signs are not supported.  So my patch
removes that support, but I didn't adjust the regression tests for that.

However, PostgreSQL numeric casting does support a negative scale. Here is an example:

# select '12345'::numeric(4,-2);
 numeric
---------
   12300
(1 row)

And thus thought of supporting those.
Do we want this JSON item method to behave differently here?
 

Also note that in your 0002 patch, the datetime precision is similarly
unsigned, so that's consistent.

By the way, in your 0002 patch, don't see the need for the separate
datetime_method grammar rule.  You can fold that into accessor_op.

Sure.
 

Overall, I think it would be better if you combined all three of these
patches into one.  Right now, you have arranged these as incremental
features, and as a result of that, the additions to the JsonPathItemType
enum and the grammar keywords etc. are ordered in the way you worked on
these features, I guess.  It would be good to maintain a bit of sanity
to put all of this together and order all the enums and everything else
for example in the order they are in the sql_features.txt file (which is
alphabetical, I suppose).  At this point I suspect we'll end up
committing this whole feature set together anyway, so we might as well
organize it that way.

OK.
I will merge them all into one and will try to keep them in the order specified in sql_features.txt.
However, for documentation, it makes more sense to keep them in logical order than the alphabetical one. What are your views on this?


Thanks
--
Jeevan Chalke
Principal, Manager
Product Development




edbpostgres.com

pgsql-hackers by date:

Previous
From: Richard Guo
Date:
Subject: Re: Oversight in reparameterize_path_by_child leading to executor crash
Next
From: Anthonin Bonnefoy
Date:
Subject: Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx