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

>
> As mentioned before, to make
> CAST(source_expr AS target_type DEFAULT expr ON CONVERSION ERROR);
> work,
> we cannot just simply replace casting FuncExpr nodes with CoerceViaIO,
> since
> type modifiers behave differently in these two Nodes.
> (e.g., casting numeric 1.11 to integer is not equivalent to casting the
> literal
> "1.11" to integer).
>

I wasn't suggesting that. I was suggesting that we move tests that use a
given type's custom cast function into the same patch that makes that cast
safe. This would mean that the initial syntax+nodes+executor patch starts
out only with test cases known to not have custom functions.



>
> Also, are we settled on this new pg_cast column name
> (pg_cast.casterrorsafe)?
> Overall, I think it's better not to touch pg_cast.dat in each of these
> refactoring patches.
>

I'm fine with it. I can see having 'f' and 's' both mean cast functions,
but 's' means safe, but the extra boolean works too and we'll be fine with
either method.


>
> Without first refactoring pg_cast.castfunc (01 to 18), making CAST ...
> DEFAULT as the first patch (0001)
> won't work, since pg_cast.castfunc itself is not error safe yet, and we
> have no way to test the CAST DEFAULT syntax.
>
> So we have to *first* refactor pg_cast.castfunc, make it error safe
> then implement CAST DEFAULT.
>

Makes sense.


>
> However, I found out, to make
> SELECT CAST('five'::INTEGER AS INTEGER DEFAULT 6 ON CONVERSION ERROR);
> error safe is hard.
>

That's no problem at all. The CAST () function can't do anything about an
input that's already an error before the CAST node executes. Nor should it.
It should only concern itself with errors related to the actual casting of
a value to the specified type.


>
> ('five'::INTEGER) is a TypeCast node, normally it will error out in
> transformTypeCast->
> coerce_to_target_type->coerce_type ```(if (inputTypeId == UNKNOWNOID
> && IsA(node, Const)))```
> If we do not error out, then we need a Node to represent the failed cast,
> mainly
> for deparse purposes.
>

We _must_ error out in that case, before the CAST executes.


>
> that means for CAST(source_expr .... DEFAULT defexpr ON CONVERSION ERROR);
> The only corner case we handle is when source_expr is a simple Const node.
> In all other cases, source_expr is processed through transformExpr,
> which does all
> the normal parse analysis for a node.
>

I'll get to reviewing this patchset soon.