Re: Error-safe user functions

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Joe Conway <mail(at)joeconway(dot)com>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Error-safe user functions
Date: 2022-12-10 23:11:35
Message-ID: 2b1e7d56-6f37-bad9-9702-053ab55ecf60@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Maciek Sakrejda 2022-12-10 23:17:22 GetNewObjectId question
Previous Message Andres Freund 2022-12-10 22:05:01 Re: Speedup generation of command completion tags