Re: Error-safe user functions

On 2022-12-10 Sa 14:38, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> OK, json is a fairly easy case, see attached. But jsonb is a different
>> kettle of fish. Both the semantic routines called by the parser and the
>> subsequent call to JsonbValueToJsonb() can raise errors. These are
>> pretty much all about breaking various limits (for strings, objects,
>> arrays). There's also a call to numeric_in, but I assume that a string
>> that's already parsed as a valid json numeric literal won't upset
>> numeric_in.
> Um, nope ...
>
> regression=# select '1e1000000'::jsonb;
> ERROR: value overflows numeric format
> LINE 1: select '1e1000000'::jsonb;
> ^

Oops, yeah.

>> Many of these occur several calls down the stack, so
>> adjusting everything to deal with them would be fairly invasive. Perhaps
>> we could instead document that this class of input error won't be
>> trapped, at least for jsonb.
> Seeing that SQL/JSON is one of the major drivers of this whole project,
> it seemed a little sad to me that jsonb couldn't manage to implement
> what is required. So I spent a bit of time poking at it. Attached
> is an extended version of your patch that also covers jsonb.
>
> The main thing I soon realized is that the JsonSemAction API is based
> on the assumption that semantic actions will report errors by throwing
> them. This is a bit schizophrenic considering the parser itself carefully
> hands back error codes instead of throwing anything (excluding palloc
> failures of course). What I propose in the attached is that we change
> that API so that action functions return JsonParseErrorType, and add
> an enum value denoting "I already logged a suitable error, so you don't
> have to". It was a little tedious to modify all the existing functions
> that way, but not hard. Only the ones used by jsonb_in need to do
> anything except "return JSON_SUCCESS", at least for now.

Many thanks for doing this, it looks good.

> I have not done anything here about errors within JsonbValueToJsonb.
> There would need to be another round of API-extension in that area
> if we want to be able to trap its errors. As you say, those are mostly
> about exceeding implementation size limits, so I suppose one could argue
> that they are not so different from palloc failure. It's still annoying.
> If people are good with the changes attached, I might take a look at
> that.
>
>

Awesome.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com