From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
---|---|
To: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, jian he <jian(dot)universality(at)gmail(dot)com>, Erik Rijkers <er(at)xs4all(dot)nl>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: remaining sql/json patches |
Date: | 2023-09-21 12:41:32 |
Message-ID: | CA+HiwqEX703Rb29V0_TxtirrGg2K6xcaOw=-KwVWDGRCf72phA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 21, 2023 at 5:58 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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 | Content-Type | Size |
---|---|---|
v19-0001-Add-soft-error-handling-to-some-expression-nodes.patch | application/octet-stream | 16.3 KB |
v19-0004-JSON_TABLE.patch | application/octet-stream | 170.9 KB |
v19-0005-Claim-SQL-standard-compliance-for-SQL-JSON-featu.patch | application/octet-stream | 2.3 KB |
v19-0002-Add-soft-error-handling-to-populate_record_field.patch | application/octet-stream | 22.8 KB |
v19-0003-SQL-JSON-query-functions.patch | application/octet-stream | 203.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2023-09-21 12:53:11 | Re: Allow specifying a dbname in pg_basebackup connection string |
Previous Message | Amit Kapila | 2023-09-21 12:15:00 | Re: [PoC] pg_upgrade: allow to upgrade publisher node |