Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions

>
> The above result shows type casts using functions which cannot be error
> safe.
> Money type related casts still can not be error safe.
>
> Cast from circle to polygon cannot be error safe because the associated
> cast
> function (pg_cast.castfunc) is written in SQL
> (see src/backend/catalog/system_functions.sql LINE 112).
> It appears impossible to make SQL language functions error safe, because
> fmgr_sql ignores fcinfo->context.
>

It is inevitable that some typecast functions are not type safe, either
because they cannot easily be made so, or because they come from an
extension that has not yet made them so.

> eval_const_expressions cannot be error safe, so we need to handle
> source_expr as an UNKNOWN constant in an error safe beforehand.
> For example, we need handle ('1' AS DATE) in an error safe way
> for
> SELECT CAST('1' AS date DEFAULT '2011-01-01' ON ERROR);
>
> Since we must handle the source_expr when it is an UNKNOWN constant in an
> error safe way, we can apply the same handling when source_expr is a
> Const whose type is not UNKNOWN.
> For example:
> SELECT CAST('[(1,2),(3,4)]'::path AS polygon DEFAULT NULL ON CONVERSION
> ERROR);
>

In the case you've presented here, the cast to type "path" doesn't need to
be safe, but the cast path->polygon does. Or rather, if it isn't safe we
should raise an error.

>
> so I introduced:
> evaluate_expr_safe: error safe version of evaluate_expr
> CoerceUnknownConstSafe: tests whether an UNKNOWN Const can be coerced to
> the
> target type.
>

Good. That's the missing bit I knew we needed to add. Sorry I wasn't able
to find time.

Issue 1:
+++ b/doc/src/sgml/syntax.sgml
@@ -2106,6 +2106,10 @@ CAST ( <replaceable>expression</replaceable> AS
<replaceable>type</replaceable>
The <literal>CAST</literal> syntax conforms to SQL; the syntax with
<literal>::</literal> is historical
<productname>PostgreSQL</productname>
usage.
+ It can also be written as:
+<synopsis>
+CAST ( <replaceable>expression</replaceable> AS
<replaceable>type</replaceable> ERROR ON CONVERSION ERROR )
+</synopsis>

The wording implies that this syntax is a new shortcut to a previously
established ON CONVERSION ERROR syntax, when it was the only way to do it
until this patch.

Issue 2:
s/is historical/is the original/
s/It can be also be written as:/The equivalent ON CONVERSION ERROR behavior
is:/

+ /*
+ * Use evaluate_expr_safe to pre-evaluate simple constant cast
+ * expressions early, in a way that tolter errors.

typo on tolter? This happens in 2 places.

Issue 3:
+ errhint("Currently CAST ... DEFAULT ON CONVERSION ERROR does not support
this cast"),

Suggest: errhint("Explicit cast is defined but definition is not declared
as safe")

This hint helps distinguish the fact that it's the cast function getting in
the way of this cast-on-default working. If the cast had been defined as IO
then we wouldn't have had a problem.

It is true that we presently have no means of declaring a cast safe aside
from making it so in pg_proc.dat, but that's coming shortly.

Issue 4:

catversion.h is not updated. Which isn't a bad thing because that brings me
to...

Issue 5:

I think 0019 is a bit big for a committer to digest all in one sitting.
Currently it:

- introduces the type-safe cast node
- introduces the cast on default syntax
- redefines
- adds in test cases for all safe functions defined in patches 1-18.

As tedious as it might be, I think we want to move this patch to the front,
move all pg_cast.dat changes to their respective patches that introduce
that datatype's safe typecast function, as well as the test cases that are
made possible by that new safe typecast. That will make it easier to ensure
that each new cast has test coverage.

Yes, that's going to be a lot of catversion bumps, requiring somebody to
fudge the dates as we presently only allow for 10 catversion bumps in a
day, but the committer will either combine a few of the patches or spread
the work out over a few days.

Overall:

I like where this patchset is going. The introduction of error contexts has
eliminated a lot of the issues I was having carrying the error state
forward into the next eval step.