Re: remaining sql/json patches

On Tue, Sep 19, 2023 at 9:31 PM Erik Rijkers <er(at)xs4all(dot)nl> wrote:
> Op 9/19/23 om 13:56 schreef Amit Langote:
> > On Tue, Sep 19, 2023 at 7:18 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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;
> | ^~~~~~

Thanks, fixed.

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