From: | Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: More new SQL/JSON item methods |
Date: | 2024-01-17 09:03:36 |
Message-ID: | CAM2+6=WyoinYoiuf3msxqH-hsPBoUz33MmrCpD_4Ceo6Nq1qOg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jan 15, 2024 at 7:41 PM Peter Eisentraut <peter(at)eisentraut(dot)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, ManagerProduct Development*
From | Date | Subject | |
---|---|---|---|
Next Message | Anthonin Bonnefoy | 2024-01-17 09:05:33 | Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx |
Previous Message | Richard Guo | 2024-01-17 09:01:36 | Re: Oversight in reparameterize_path_by_child leading to executor crash |