Re: remaining sql/json patches - Mailing list pgsql-hackers

From Amit Langote
Subject Re: remaining sql/json patches
Date
Msg-id CA+HiwqEX703Rb29V0_TxtirrGg2K6xcaOw=-KwVWDGRCf72phA@mail.gmail.com
Whole thread Raw
In response to Re: remaining sql/json patches  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: remaining sql/json patches
List pgsql-hackers
On Thu, Sep 21, 2023 at 5:58 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> I keep looking at 0001, and in the struct definition I think putting the
> escontext at the bottom is not great, because there's a comment a few
> lines above that says "XXX: following fields only needed during
> "compilation"), could be thrown away afterwards".  This comment is not
> strictly true, because innermost_caseval is actually used by
> array_map(); yet it seems that ->escontext should appear before that
> comment.

Hmm.   Actually, we can make it so that *escontext* is only needed
during ExecInitExprRec() and never after that.  I've done that in the
attached updated patch, where you can see that ExprState.escontext is
only ever touched in execExpr.c.   Also, I noticed that I had
forgotten to extract one more expression node type's conversion to use
soft errors from the main patch (0003).  That is CoerceToDomain, which
I've now moved into 0001.

> Also, ->escontext's own comment in ExprState seems to be saying too much
> and not saying enough.  I would reword it as "For expression nodes that
> support soft errors.  NULL if caller wants them thrown instead".  The
> shortest I could make so that it fits in a single is "For nodes that can
> error softly. NULL if caller wants them thrown", or "For
> soft-error-enabled nodes.  NULL if caller wants errors thrown".  Not
> sure if those are good enough, or just make the comment the whole four
> lines ...

How about:

+   /*
+    * For expression nodes that support soft errors.  Set to NULL before
+    * calling ExecInitExprRec() if the caller wants errors thrown.
+    */

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

Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: [PoC] pg_upgrade: allow to upgrade publisher node
Next
From: Daniel Gustafsson
Date:
Subject: Re: Allow specifying a dbname in pg_basebackup connection string