On Tue, 20 Dec 2022 at 04:27, Corey Huinker <corey.huin...@gmail.com> wrote:
>
>
> Attached is my work in progress to implement the changes to the CAST()
> function as proposed by Vik Fearing.
>
> This work builds upon the Error-safe User Functions work currently ongoing.
>
> The proposed changes are as follows:
>
> CAST(expr AS typename)
> continues to behave as before.
>
> CAST(expr AS typename ERROR ON ERROR)
> has the identical behavior as the unadorned CAST() above.
>
> CAST(expr AS typename NULL ON ERROR)
> will use error-safe functions to do the cast of expr, and will return
> NULL if the cast fails.
>
> CAST(expr AS typename DEFAULT expr2 ON ERROR)
> will use error-safe functions to do the cast of expr, and will return
> expr2 if the cast fails.
>
> There is an additional FORMAT parameter that I have not yet implemented, my
> understanding is that it is largely intended for DATE/TIME field conversions,
> but others are certainly possible.
> CAST(expr AS typename FORMAT fmt DEFAULT expr2 ON ERROR)
>
> What is currently working:
> - Any scalar expression that can be evaluated at parse time. These tests from
> cast.sql all currently work:
>
> VALUES (CAST('error' AS integer));
> VALUES (CAST('error' AS integer ERROR ON ERROR));
> VALUES (CAST('error' AS integer NULL ON ERROR));
> VALUES (CAST('error' AS integer DEFAULT 42 ON ERROR));
>
> SELECT CAST('{123,abc,456}' AS integer[] DEFAULT '{-789}' ON ERROR) as
> array_test1;
>
> - Scalar values evaluated at runtime.
>
> CREATE TEMPORARY TABLE t(t text);
> INSERT INTO t VALUES ('a'), ('1'), ('b'), (2);
> SELECT CAST(t.t AS integer DEFAULT -1 ON ERROR) AS foo FROM t;
> foo
> -----
> -1
> 1
> -1
> 2
> (4 rows)
>
>
> Along the way, I made a few design decisions, each of which is up for debate:
>
> First, I created OidInputFunctionCallSafe, which is to OidInputFunctionCall
> what InputFunctionCallSafe is to InputFunctionCall. Given that the only place
> I ended up using it was stringTypeDatumSafe(), it may be possible to just
> move that code inside stringTypeDatumSafe.
>
> Next, I had a need for FuncExpr, CoerceViaIO, and ArrayCoerce to all report
> if their expr argument failed, and if not, just past the evaluation of expr2.
> Rather than duplicate this logic in several places, I chose instead to modify
> CoalesceExpr to allow for an error-test mode in addition to its default
> null-test mode, and then to provide this altered node with two expressions,
> the first being the error-safe typecast of expr and the second being the
> non-error-safe typecast of expr2.
>
> I still don't have array-to-array casts working, as the changed I would
> likely need to make to ArrayCoerce get somewhat invasive, so this seemed like
> a good time to post my work so far and solicit some feedback beyond what I've
> already been getting from Jeff Davis and Michael Paquier.
>
> I've sidestepped domains as well for the time being as well as avoiding JIT
> issues entirely.
>
> No documentation is currently prepared. All but one of the regression test
> queries work, the one that is currently failing is:
>
> SELECT CAST('{234,def,567}'::text[] AS integer[] DEFAULT '{-1011}' ON ERROR)
> as array_test2;
>
> Other quirks:
> - an unaliased CAST ON DEFAULT will return the column name of "coalesce",
> which internally is true, but obviously would be quite confusing to a user.
>
> As a side observation, I noticed that the optimizer already tries to resolve
> expressions based on constants and to collapse expression trees where
> possible, which makes me wonder if the work done to do the same in
> transformTypeCast/ and coerce_to_target_type and coerce_type isn't also
> wasted.
CFBot shows some compilation errors as in [1], please post an updated
version for the same:
[02:53:44.829] time make -s -j${BUILD_JOBS} world-bin
[02:55:41.164] llvmjit_expr.c: In function ‘llvm_compile_expr’:
[02:55:41.164] llvmjit_expr.c:928:6: error: ‘v_resnull’ undeclared
(first use in this function); did you mean ‘v_resnullp’?
[02:55:41.164] 928 | v_resnull = LLVMBuildLoad(b, v_reserrorp, "");
[02:55:41.164] | ^~~~~~~~~
[02:55:41.164] | v_resnullp
[02:55:41.164] llvmjit_expr.c:928:6: note: each undeclared identifier
is reported only once for each function it appears in
[02:55:41.164] llvmjit_expr.c:928:35: error: ‘v_reserrorp’ undeclared
(first use in this function); did you mean ‘v_reserror’?
[02:55:41.164] 928 | v_resnull = LLVMBuildLoad(b, v_reserrorp, "");
[02:55:41.164] | ^~~~~~~~~~~
[02:55:41.164] | v_reserror
[02:55:41.165] make[2]: *** [<builtin>: llvmjit_expr.o] Error 1
[02:55:41.165] make[2]: *** Waiting for unfinished jobs....
[02:55:45.495] make[1]: *** [Makefile:42: all-backend/jit/llvm-recurse] Error 2
[02:55:45.495] make: *** [GNUmakefile:21: world-bin-src-recurse] Error 2
[1] - https://cirrus-ci.com/task/6687753371385856?logs=gcc_warning#L448
Regards,
Vignesh
