On Thu, Sep 28, 2023 at 8:04 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2023-Sep-27, Amit Langote wrote:
> > Maybe the following is better:
> >
> > + /*
> > + * For expression nodes that support soft errors. Should be set to NULL
> > + * before calling ExecInitExprRec() if the caller wants errors thrown.
> > + */
> >
> > ...as in the attached.
>
> That's good.
>
> > Alvaro, do you think your concern regarding escontext not being in the
> > right spot in the ExprState struct is addressed? It doesn't seem very
> > critical to me to place it in the struct's 1st cacheline, because
> > escontext is not accessed in performance critical paths such as during
> > expression evaluation, especially with the latest version. (It would
> > get accessed during evaluation with previous versions.)
> >
> > If so, I'd like to move ahead with committing it.
>
> Yeah, looks OK to me in v21.
Thanks. I will push the attached 0001 shortly.
Also, I've updated 0002's commit message to mention why it only
changes the functions local to jsonfuncs.c to add the Node *escontext
parameter, but not any external ones that may be invoked, such as,
makeMdArrayResult(). The assumption behind that is that jsonfuncs.c
functions validate any data that they pass to such external functions,
so no *suppressible* errors should occur.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com