Re: remaining sql/json patches

Op 8/31/23 om 14:57 schreef Amit Langote:
> Hello,
>
> On Wed, Aug 16, 2023 at 1:27 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> I will post a new version after finishing working on a few other
>> improvements I am working on.
>
> Sorry about the delay. Here's a new version.
>
Hi,

While compiling the new set

[v12-0001-Support-soft-error-handling-during-CoerceViaIO-e.patch]
[v12-0002-SQL-JSON-query-functions.patch]
[v12-0003-JSON_TABLE.patch]
[v12-0004-Claim-SQL-standard-compliance-for-SQL-JSON-featu.patch]

gcc 13.2.0 is sputtering somewhat:

--------------
In function ‘transformJsonFuncExpr’,
inlined from ‘transformExprRecurse’ at parse_expr.c:374:13:
parse_expr.c:4362:13: warning: ‘contextItemExpr’ may be used
uninitialized [-Wmaybe-uninitialized]
4362 | if (exprType(contextItemExpr) != JSONBOID)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
parse_expr.c: In function ‘transformExprRecurse’:
parse_expr.c:4214:21: note: ‘contextItemExpr’ was declared here
4214 | Node *contextItemExpr;
| ^~~~~~~~~~~~~~~
nodeFuncs.c: In function ‘exprSetCollation’:
nodeFuncs.c:1238:25: warning: this statement may fall through
[-Wimplicit-fallthrough=]
1238 | {
| ^
nodeFuncs.c:1247:17: note: here
1247 | case T_JsonCoercion:
| ^~~~
--------------

Those looks pretty unimportant, but I thought I'd let you know.

Tests (check, check-world and my own) still run fine.

Thanks,

Erik Rijkers

> I found out that llvmjit_expr.c additions have been broken all along,
> I mean since I rewrote the JsonExpr evaluation code to use soft error
> handling back in January or so. For example, I had made CoerceiViaIO
> evaluation code (EEOP_IOCOERCE ExprEvalStep) invoked by JsonCoercion
> node's evaluation to pass an ErrorSaveContext to the type input
> functions so that any errors result in returning NULL instead of
> throwing the error. Though the llvmjit_expr.c code was not modified
> to do the same, so the SQL/JSON query functions would return wrong
> results when JITed. I have made many revisions to the JsonExpr
> expression evaluation itself, not all of which were reflected in the
> llvmjit_expr.c counterparts. I've fixed all that in the attached.
>
> I've broken the parts to teach the CoerceViaIO evaluation code to
> handle errors softly into a separate patch attached as 0001.
>
> Other notable changes in the SQL/JSON query functions patch (now 0002):
>
> * Significantly rewrote the parser changes to make it a bit more
> readable than before. My main goal was to separate the code for each
> JSON_EXISTS_OP, JSON_QUERY_OP, and JSON_VALUE_OP such that the
> op-type-specific behaviors are more readily apparent by reading the
> code.
>
> * Got rid of JsonItemCoercions struct/node, which contained a
> JsonCoercion field to store the coercion expressions for each JSON
> item type that needs to be coerced to the RETURNING type, in favor of
> using List of JsonCoercion nodes. That resulted in simpler code in
> many places, most notably in the executor / llvmjit_expr.c.
>