From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Andrew Dunstan <andrew@dunslane.net>,
        Alexander Korotkov <aekorotkov@gmail.com>,
        Robert Haas <robertmhaas@gmail.com>,
        "David G. Johnston" <david.g.johnston@gmail.com>,
        Corey Huinker <corey.huinker@gmail.com>,
        Alvaro Herrera <alvherre@alvh.no-ip.org>,
        Joe Conway <mail@joeconway.com>,
        Nikita Glukhov <n.gluhov@postgrespro.ru>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Error-safe user functions
In-reply-to: <20221211204121.jgncgil4xip7zgd7@awork3.anarazel.de>
References: <512551.1670540229@sss.pgh.pa.us>
 <20221209023351.l362yejerfoq3lfp@awork3.anarazel.de>
 <633426.1670554790@sss.pgh.pa.us>
 <01ff2051-806d-af2f-4f9e-793f7ba9a3f8@dunslane.net>
 <778369.1670598993@sss.pgh.pa.us>
 <5aaf7c66-9df9-21a9-1ff4-02c7848def52@dunslane.net>
 <3bac9841-fe07-713d-fa42-606c225567d6@dunslane.net>
 <1436686.1670701118@sss.pgh.pa.us>
 <2b1e7d56-6f37-bad9-9702-053ab55ecf60@dunslane.net>
 <1805344.1670779451@sss.pgh.pa.us>
 <20221211204121.jgncgil4xip7zgd7@awork3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Sun, 11 Dec 2022 12:41:21 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1880240.1670793818.1@sss.pgh.pa.us>
Date: Sun, 11 Dec 2022 16:23:38 -0500
Message-ID: <1880241.1670793818@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1880241.1670793818%40sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.de> writes:
> On 2022-12-11 12:24:11 -0500, Tom Lane wrote:
>> I spent some time looking at this, and was discouraged to conclude
>> that the notational mess would probably be substantially out of
>> proportion to the value.  The main problem is that we'd have to change
>> the API of pushJsonbValue, which has more than 150 call sites, most
>> of which would need to grow a new test for failure return.  Maybe
>> somebody will feel like tackling that at some point, but not me today.

> Could we address this more minimally by putting the error state into the
> JsonbParseState and add a check for that error state to convertToJsonb() or
> such (by passing in the JsonbParseState)? We'd need to return immediately in
> pushJsonbValue() if there's already an error, but that that's not too bad.

We could shoehorn error state into the JsonbParseState, although the
fact that that stack normally starts out empty is a bit of a problem.
I think you'd have to push a dummy entry if you want soft errors,
store the error state pointer into that, and have pushState() copy
down the parent's error pointer.  Kind of ugly, but do-able.  Whether
it's better than replacing that argument with a pointer-to-struct-
that-includes-the-stack-and-the-error-pointer wasn't real clear to me.

What seemed like a mess was getting the calling code to quit early.
I'm not convinced that just putting an immediate exit into pushJsonbValue
would be enough, because the callers tend to assume a series of calls
will behave as they expect.  Probably some of the call sites could
ignore the issue, but you'd still end with a lot of messy changes
I fear.

			regards, tom lane