Re: Error-safe user functions - Mailing list pgsql-hackers

Corey Huinker <corey.huinker@gmail.com> writes:
> I'm still working on organizing my patch, but it grew out of a desire to do
> this:
> CAST(value AS TypeName DEFAULT expr)
> This is a thing that exists in other forms in other databases and while it
> may look unrelated, it is essentially the SQL/JSON casts within a nested
> data structure issue, just a lot simpler.

Okay, maybe that's why I was thinking we had a requirement for
failure-free casts.  Sure, you can transform it to the other thing
by always implementing this as a cast-via-IO, but you could run into
semantics issues that way.  (If memory serves, we already have cases
where casting X to Y gives a different result from casting X to text
to Y.)

> My original plan had been two new params to all _in() functions: a boolean
> error_mode and a default expression Datum.
> After consulting with Jeff Davis and Michael Paquier, the notion of
> modifying fcinfo itself two booleans:
>   allow_error (this call is allowed to return if there was an error with
> INPUT) and
>   has_error (this function has the concept of a purely-input-based error,
> and found one)

Hmm ... my main complaint about that is the lack of any way to report
the details of the error.  I realize that a plain boolean failure flag
might be enough for our immediate use-cases, but I don't want to expend
the amount of effort this is going to involve and then later find we
have a use-case where we need the error details.

The sketch that's in my head at the moment is to make use of the existing
"context" field of FunctionCallInfo: if that contains a node of some
to-be-named type, then we are requesting that errors not be thrown
but instead be reported by passing back an ErrorData using a field of
that node.  The issue about not constructing an ErrorData if the outer
caller doesn't need it could perhaps be addressed by adding some boolean
flag fields in that node, but the details of that need not be known to
the functions reporting errors this way; it'd be a side channel from the
outer caller to elog.c.

The main objection I can see to this approach is that we only support
one context value per call, so you could not easily combine this
functionality with existing use-cases for the context field.  A quick
census of InitFunctionCallInfoData calls finds aggregates, window
functions, triggers, and procedures, none of which seem like plausible
candidates for wanting no-error behavior, so I'm not too concerned
about that.  (Maybe the error-reporting node could be made a sub-node
of the context node in any future cases where we do need it?)

> The one gap I see so far in the patch presented is that it returns a null
> value on bad input, which might be ok if the node has the default, but that
> then presents the node with having to understand whether it was a null
> because of bad input vs a null that was expected.

Yeah.  That's something we could probably get away with for the case of
input functions only, but I think explicit out-of-band signaling that
there was an error is a more future-proof solution.