Re: Error-safe user functions

On Fri, Dec 2, 2022 at 9:34 AM Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:

>
> On 2022-12-02 Fr 09:12, Tom Lane wrote:
> > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> >> I think the design is evolving in your head as you think about this
> >> more, which is totally understandable and actually very good. However,
> >> this is also why I think that you should produce the patch you
> >> actually want instead of letting other people repeatedly submit
> >> patches and then complain that they weren't what you had in mind.
> > OK, Corey hasn't said anything, so I will have a look at this over
> > the weekend.
> >
> >
>
>
> 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.
>
>
> cheers
>
>
> andrew
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>
>
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.

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)

The nice part about this is that unaware functions can ignore these values,
and custom data types that did not check these values would continue to
work as before. It wouldn't respect the CAST default, but that's up to the
extension writer to fix, and that's a pretty acceptable failure mode.

Where this gets tricky is arrays and complex types: the default expression
applies only to the object explicitly casted, so if somebody tried CAST
('{"123","abc"}'::text[] AS integer[] DEFAULT '{0}') the inner casts need
to know that they _can_ allow input errors, but have no default to offer,
they need merely report their failure upstream...and that's where the
issues align with the SQL/JSON issue.

In pursuing this, I see that my method was simultaneously too little and
too much. Too much in the sense that it alters the structure for all fmgr
functions, though in a very minor and back-compatible way, and too little
in the sense that we could actually return the ereport info in a structure
and leave it to the node to decide whether to raise it or not. Though I
should add that there many situations where we don't care about the
specifics of the error, we just want to know that one existed and move on,
so time spent forming that return structure would be time wasted.

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.