Re: SQL/JSON features for v15

On Sat, Aug 27, 2022 at 5:11 AM Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
> On 26.08.2022 22:25, Andrew Dunstan wrote:
>
> On 2022-08-24 We 20:05, Nikita Glukhov wrote:
>
> v8 - is a highly WIP patch, which I failed to finish today.
> Even some test cases fail now, and they simply show unfinished
> things like casts to bytea (they can be simply removed) and missing
> safe input functions.
>
> Thanks for your work, please keep going.
>
> 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@postgresql.org>
Date:   Sat Mar 16 12:21:19 2019 +0300

    Numeric error suppression in jsonpath

BTW, maybe the following hunk in boolin_opt_error() is unnecessary?

-   len = strlen(str);
+   len -= str - in_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.

>    Maybe it would be better to simply remove DEFAULT ON EMPTY.

So +1 to this for now.

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

That would be nice indeed.

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?

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com