Re: More new SQL/JSON item methods

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*

edbpostgres.com

In response to

Responses

Browse pgsql-hackers by date

  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