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: Robert Haas <robertmhaas(at)gmail(dot)com>, Corey Huinker <corey(dot)huinker(at)gmail(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-03 21:46:41
Message-ID: 2503569.1670104001@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:
> Great. Let's hope we can get this settled early next week and then we
> can get to work on the next tranche of functions, those that will let
> the SQL/JSON work restart.

OK, here's a draft proposal. I should start out by acknowledging that
this steals a great deal from Nikita's original patch as well as yours,
though I editorialized heavily.

0001 is the core infrastructure and documentation for the feature.
(I didn't bother breaking it down further than that.)

0002 fixes boolin and int4in. That is the work that we're going to
have to replicate in an awful lot of places, and I am pleased by how
short-and-sweet it is. Of course, stuff like the datetime functions
might be more complex to adapt.

Then 0003 is a quick-hack version of COPY that is able to exercise
all this. I did not bother with the per-column flags as you had
them, because I'm not sure if they're worth the trouble compared
to a simple boolean; in any case we can add that refinement later.
What I did add was a WARN_ON_ERROR option that exercises the ability
to extract the error message after a soft error. I'm not proposing
that as a shippable feature, it's just something for testing.

I think there are just a couple of loose ends here:

1. Bikeshedding on my name choices is welcome. I know Robert is
dissatisfied with "ereturn", but I'm content with that so I didn't
change it here.

2. Everybody has struggled with just where to put the declaration
of the error context structure. The most natural home for it
probably would be elog.h, but that's out because it cannot depend
on nodes.h, and the struct has to be a Node type to conform to
the fmgr safety guidelines. What I've done here is to drop it
in nodes.h, as we've done with a couple of other hard-to-classify
node types; but I can't say I'm satisfied with that.

Other plausible answers seem to be:

* Drop it in fmgr.h. The only real problem is that historically
we've not wanted fmgr.h to depend on nodes.h either. But I'm not
sure how strong the argument for that really is/was. If we did
do it like that we could clean up a few kluges, both in this patch
and pre-existing (fmNodePtr at least could go away).

* Invent a whole new header just for this struct. But then we're
back to the question of what to call it. Maybe something along the
lines of utils/elog_extras.h ?

regards, tom lane

Attachment Content-Type Size
0001-infrastructure.patch text/x-diff 15.7 KB
0002-fix-bool-and-int4.patch text/x-diff 3.0 KB
0003-quick-COPY-hack.patch text/x-diff 9.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2022-12-04 00:34:44 Re: [PoC] Reducing planning time when tables have many partitions
Previous Message David Rowley 2022-12-03 20:57:44 Re: Questions regarding distinct operation implementation