Re: Error-safe user functions

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
Views: Raw Message | Whole Thread | Download mbox | Resend email
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

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

regards, tom lane

Attachment Content-Type Size
0001-fix-json-and-jsonb.patch text/x-diff 60.3 KB

In response to


Browse pgsql-hackers by date

  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