From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
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 19:38:38 |
Message-ID: | 1436686.1670701118@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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;
^
> 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.
(I wonder if pg_verifybackup's parse_manifest.c could use a second
look at how it's handling errors, given this API. I didn't study it
closely.)
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.
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
0001-fix-json-and-jsonb.patch | text/x-diff | 60.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jeff Davis | 2022-12-10 20:07:12 | Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX |
Previous Message | Joseph Koshakow | 2022-12-10 19:21:12 | Infinite Interval |