Re: SQL/JSON features for v15

Hi,

On 29.08.2022 15:56, Amit Langote wrote:
> On Sat, Aug 27, 2022 at 5:11 AM Nikita Glukhov<n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>> On 26.08.2022 22:25, Andrew Dunstan wrote:
>>
>> On 2022-08-24 We 20:05, Nikita Glukhov wrote:
>>
>> I have completed in v9 all the things I previously planned:
>>
>> - Added missing safe I/O and type conversion functions for
>> datetime, float4, varchar, bpchar. This introduces a lot
>> of boilerplate code for returning errors and also maybe
>> adds some overhead.
> Didn't know that we have done similar things in the past for jsonpath, as in:
>
> commit 16d489b0fe058e527619f5e9d92fd7ca3c6c2994
> Author: Alexander Korotkov<akorotkov(at)postgresql(dot)org>
> Date: Sat Mar 16 12:21:19 2019 +0300
>
> Numeric error suppression in jsonpath

This was necessary for handling errors in arithmetic operations.

> BTW, maybe the following hunk in boolin_opt_error() is unnecessary?
>
> - len = strlen(str);
> + len -= str - in_str;
>
This is really not necessary, but helps to avoid extra strlen() call.
I have replaced it with more intuitive

+ {

str++;
+ len--;
+ }

- len = strlen(str);

>> - Added JSON_QUERY coercion to UTF8 bytea using pg_convert_to().
>>
>> - Added immutability checks that were missed with elimination
>> of coercion expressions.
>> Coercions text::datetime, datetime1::datetime2 and even
>> datetime::text for some datetime types are mutable.
>> datetime::text can be made immutable by passing ISO date
>> style into output functions (like in jsonpath).
>>
>> - Disabled non-Const expressions in DEFAULT ON EMPTY in non
>> ERROR ON ERROR case. Non-constant expressions are tried to
>> evaluate into Const directly inside transformExpr().
> I am not sure if it's OK to eval_const_expressions() on a Query
> sub-expression during parse-analysis. IIUC, it is only correct to
> apply it to after the rewriting phase.

I also was not sure. Maybe it can be moved to rewriting phase or
even to execution phase.

>> Maybe it would be better to simply remove DEFAULT ON EMPTY.
> So +1 to this for now.

See last patch #9.

>> It is possible to easily split this patch into several subpatches,
>> I will do it if needed.
> That would be nice indeed.

I have extracted patches #1-6 with numerous safe input and type conversion
functions.

> I'm wondering if you're going to change the PASSING values
> initialization to add the steps into the parent JsonExpr's ExprState,
> like the previous patch was doing?

I forget to incorporate your changes for subsidary ExprStates elimination.
See patch #8.

--
Nikita Glukhov
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company