Re: remaining sql/json patches

On Tue, Sep 19, 2023 at 9:31 PM Erik Rijkers <er@xs4all.nl> wrote:
> 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;
>        |                     ^~~~~~

Thanks, fixed.

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