Re: remaining sql/json patches

Op 9/19/23 om 13:56 schreef Amit Langote:
> On Tue, Sep 19, 2023 at 7:18 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> 0001: I wonder why you used Node for the ErrorSaveContext pointer
>> instead of the specific struct you want.  I propose the attached, for
>> some extra type-safety.  Or did you have a reason to do it that way?
> 
> No reason other than that most other headers use Node.  I agree that
> making an exception for this patch might be better, so I've
> incorporated your patch into 0001.
> 
> I've also updated the query functions patch (0003) to address the
> crashing bug reported by Erik.  Essentially, I made the coercion step
> of JSON_QUERY to always use json_populate_type() when WITH WRAPPER is
> used.  You might get funny errors with ERROR OR ERROR for many types
> when used in RETURNING, but at least there should no longer be any
> crashes.
> 

Indeed, with v16 those crashes are gone.

Some lesser evil: gcc 13.2.0 gave some warnings, slightly different in 
assert vs non-assert build.

--- assert build:

-- [2023.09.19 14:06:35 json_table2/0] make core: make --quiet -j 4
In file included from ../../../src/include/postgres.h:45,
                  from parse_expr.c:16:
In function ‘transformJsonFuncExpr’,
     inlined from ‘transformExprRecurse’ at parse_expr.c:374:13:
parse_expr.c:4355:22: warning: ‘jsexpr’ may be used uninitialized 
[-Wmaybe-uninitialized]
  4355 |         Assert(jsexpr->formatted_expr);
../../../src/include/c.h:864:23: note: in definition of macro ‘Assert’
   864 |                 if (!(condition)) \
       |                       ^~~~~~~~~
parse_expr.c: In function ‘transformExprRecurse’:
parse_expr.c:4212:21: note: ‘jsexpr’ was declared here
  4212 |         JsonExpr   *jsexpr;
       |                     ^~~~~~


--- non-assert build:

-- [2023.09.19 14:11:03 json_table2/1] make core: make --quiet -j 4
In function ‘transformJsonFuncExpr’,
     inlined from ‘transformExprRecurse’ at parse_expr.c:374:13:
parse_expr.c:4356:28: warning: ‘jsexpr’ may be used uninitialized 
[-Wmaybe-uninitialized]
  4356 |         if (exprType(jsexpr->formatted_expr) != JSONBOID)
       |                      ~~~~~~^~~~~~~~~~~~~~~~
parse_expr.c: In function ‘transformExprRecurse’:
parse_expr.c:4212:21: note: ‘jsexpr’ was declared here
  4212 |         JsonExpr   *jsexpr;
       |                     ^~~~~~


Thank you,

Erik