Andres Freund <and...@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
