From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew@dunslane.net>
cc: Corey Huinker <corey.huinker@gmail.com>,
        Nikita Glukhov <n.gluhov@postgrespro.ru>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Error-safe user functions
In-reply-to: <469338a3-da38-160f-23b9-813c261bc1d6@dunslane.net>
References: <3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru>
 <3190059.1665164276@sss.pgh.pa.us>
 <57b7cc2e-1215-3f43-84e4-6d9f8ad575df@dunslane.net>
 <CADkLM=djieyppZpKgveeDdt3QYLt+XQGHzpoa+Sd_Lwa5dDTWA@mail.gmail.com>
 <3049337.1669008405@sss.pgh.pa.us>
 <469338a3-da38-160f-23b9-813c261bc1d6@dunslane.net>
Comments: In-reply-to Andrew Dunstan <andrew@dunslane.net>
	message dated "Thu, 01 Dec 2022 12:46:48 -0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1677333.1669918475.1@sss.pgh.pa.us>
Date: Thu, 01 Dec 2022 13:14:35 -0500
Message-ID: <1677334.1669918475@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1677334.1669918475%40sss.pgh.pa.us>
Precedence: bulk

Andrew Dunstan <andrew@dunslane.net> writes:
> OK, here's a set of minimal patches based on Nikita's earlier work and
> also some work by my colleague Amul Sul. It tries to follow Tom's
> original outline at [1], and do as little else as possible.

This is not really close at all to what I had in mind.

The main objection is that we shouldn't be passing back a "char *"
error string (though I observe that your 0003 and 0004 patches aren't
even bothering to do that much).  I think we want to pass back a
fully populated ErrorData struct so that we can report everything
the actual error would have done (notably, the SQLSTATE).  That means
that elog.h/.c has to be intimately involved in this.  I liked Nikita's
overall idea of introducing an "ereturn" macro, with the idea that
where we have, say,

    ereport(ERROR,
            (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
             errmsg("value \"%s\" is out of range for type %s",
                    s, "integer")));

we would write

    ereturn(context, ERROR,
            (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
             errmsg("value \"%s\" is out of range for type %s",
                    s, "integer")));
    return NULL;              // or whatever is appropriate

and the involvement with the contents of the context node would
all be confined to some new code in elog.c.  That would help
prevent the #include-footprint-bloat that is otherwise going to
ensue.

(Maybe we could assume that ereturn's elevel must be ERROR, and
save a little notation.  I'm not very wedded to "ereturn" as the
new macro name either, though it's not awful.)

Also, as I said before, the absolute first priority has to be
documentation explaining what function authors are supposed to
do differently than before.

I'd be willing to have a go at this myself, except that Corey
said he was working on it, so I don't want to step on his toes.

			regards, tom lane