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  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.
>