Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On Tue, Mar 28, 2023 at 3:25 PM Isaac Morland wrote: > On Mon, 19 Dec 2022 at 17:57, Corey Huinker > wrote: > >> >> Attached is my work in progress to implement the changes to the CAST() >> function as proposed by Vik Fearing. >> >> 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. >> > > Is there any difference between NULL and DEFAULT NULL? > What I think you're asking is: is there a difference between these two statements: SELECT CAST(my_string AS integer NULL ON ERROR) FROM my_table; SELECT CAST(my_string AS integer DEFAULT NULL ON ERROR) FROM my_table; And as I understand it, the answer would be no, there is no practical difference. The first case is just a convenient shorthand, whereas the second case tees you up for a potentially complex expression. Before you ask, I believe the ON ERROR syntax could be made optional. As I implemented it, both cases create a default expression which then typecast to integer, and in both cases that expression would be a const-null, so the optimizer steps would very quickly collapse those steps into a plain old constant.
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On Tue, Mar 28, 2023 at 2:53 PM Gregory Stark (as CFM) wrote: > On Tue, 3 Jan 2023 at 14:16, Tom Lane wrote: > > > > Vik Fearing writes: > > > > > I don't think we should add that syntax until I do get it through the > > > committee, just in case they change something. > > > > Agreed. So this is something we won't be able to put into v16; > > it'll have to wait till there's something solid from the committee. > > I guess I'll mark this Rejected in the CF then. Who knows when the SQL > committee will look at this... > > -- > Gregory Stark > As Commitfest Manager > Yes, for now. I'm in touch with the pg-people on the committee and will resume work when there's something to act upon.
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On Mon, 19 Dec 2022 at 17:57, Corey Huinker wrote: > > Attached is my work in progress to implement the changes to the CAST() > function as proposed by Vik Fearing. > > 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. > Is there any difference between NULL and DEFAULT NULL?
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On Tue, 3 Jan 2023 at 14:16, Tom Lane wrote: > > Vik Fearing writes: > > > I don't think we should add that syntax until I do get it through the > > committee, just in case they change something. > > Agreed. So this is something we won't be able to put into v16; > it'll have to wait till there's something solid from the committee. I guess I'll mark this Rejected in the CF then. Who knows when the SQL committee will look at this... -- Gregory Stark As Commitfest Manager
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Vik Fearing writes: > I have not posted my paper to the committee yet, but I plan to do so > before the working group's meeting early February. Just like with > posting patches here, I cannot guarantee that it will get accepted but I > will be arguing for it. > I don't think we should add that syntax until I do get it through the > committee, just in case they change something. Agreed. So this is something we won't be able to put into v16; it'll have to wait till there's something solid from the committee. regards, tom lane
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On 1/3/23 19:14, Tom Lane wrote: Corey Huinker writes: On Mon, Jan 2, 2023 at 10:57 AM Tom Lane wrote: While I approve of trying to get some functionality in this area, I'm not sure that extending CAST is a great idea, because I'm afraid that the SQL committee will do something that conflicts with it. I'm going off the spec that Vik presented in https://www.postgresql.org/message-id/f8600a3b-f697-2577-8fea-f40d3e18b...@postgresfriends.org which is his effort to get it through the SQL committee. I'm pretty certain that sending something to pgsql-hackers will have exactly zero impact on the SQL committee. Is there anything actually submitted to the committee, and if so what's its status? I have not posted my paper to the committee yet, but I plan to do so before the working group's meeting early February. Just like with posting patches here, I cannot guarantee that it will get accepted but I will be arguing for it. I don't think we should add that syntax until I do get it through the committee, just in case they change something. -- Vik Fearing
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker writes: > On Mon, Jan 2, 2023 at 10:57 AM Tom Lane wrote: >> While I approve of trying to get some functionality in this area, >> I'm not sure that extending CAST is a great idea, because I'm afraid >> that the SQL committee will do something that conflicts with it. > I'm going off the spec that Vik presented in > https://www.postgresql.org/message-id/f8600a3b-f697-2577-8fea-f40d3e18b...@postgresfriends.org > which is his effort to get it through the SQL committee. I'm pretty certain that sending something to pgsql-hackers will have exactly zero impact on the SQL committee. Is there anything actually submitted to the committee, and if so what's its status? regards, tom lane
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On Mon, Jan 2, 2023 at 10:57 AM Tom Lane wrote: > Corey Huinker writes: > > 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. > > While I approve of trying to get some functionality in this area, > I'm not sure that extending CAST is a great idea, because I'm afraid > that the SQL committee will do something that conflicts with it. > If we know that they are about to standardize exactly this syntax, > where is that information available? If we don't know that, > I'd prefer to invent some kind of function or other instead of > extending the grammar. > > regards, tom lane > I'm going off the spec that Vik presented in https://www.postgresql.org/message-id/f8600a3b-f697-2577-8fea-f40d3e18b...@postgresfriends.org which is his effort to get it through the SQL committee. I was alreading thinking about how to get the SQLServer TRY_CAST() function into postgres, so this seemed like the logical next step. While the syntax may change, the underlying infrastructure would remain basically the same: we would need the ability to detect that a typecast had failed, and replace it with the default value, and handle that at parse time, or executor time, and handle array casts where the array has the default but the underlying elements can't. It would be simple to move the grammar changes to their own patch if that removes a barrier for people.
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On 2023-01-02 Mo 10:57, Tom Lane wrote: > Corey Huinker writes: >> 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. > While I approve of trying to get some functionality in this area, > I'm not sure that extending CAST is a great idea, because I'm afraid > that the SQL committee will do something that conflicts with it. > If we know that they are about to standardize exactly this syntax, > where is that information available? If we don't know that, > I'd prefer to invent some kind of function or other instead of > extending the grammar. +1 cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On Tue, 20 Dec 2022 at 04:27, Corey Huinker 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]: *** [: 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
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker writes: > 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. While I approve of trying to get some functionality in this area, I'm not sure that extending CAST is a great idea, because I'm afraid that the SQL committee will do something that conflicts with it. If we know that they are about to standardize exactly this syntax, where is that information available? If we don't know that, I'd prefer to invent some kind of function or other instead of extending the grammar. regards, tom lane
CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
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.
From 897c9c68a29ad0fa57f28734df0c77553e026d80 Mon Sep 17 00:00:00 2001
From: coreyhuinker
Date: Sun, 18 Dec 2022 16:20:01 -0500
Subject: [PATCH 1/3] add OidInputFunctionCall
---
src/backend/utils/fmgr/fmgr.c | 13 +
src/include/fmgr.h| 4
2 files changed, 17 insertions(+)
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 0d37f69298..e9a19ce653 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -1701,6 +1701,19 @@ OidInputFunctionCall(Oid functionId, char *str, Oid typioparam, int32 typmod)
return InputFunctionCall(, str, typioparam, typmod);
}
+bool
+OidInputFunctionCallSafe(Oid functionId, char *str, Oid typioparam,
+ int32 typmod, fmNodePtr escontext,
+ Datum *result)
+{
+ FmgrInfo flinfo;
+
+ fmgr_info(functionId, );
+
+ return InputFunctionCallSafe(, str, typioparam, typmod,
+ escontext, result);
+}
+
char *
OidOutputFunctionCall(Oid functionId, Datum val)
{
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index b7832d0aa2..b835ef72b5 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -706,6 +706,10 @@ extern bool InputFunctionCallSafe(FmgrInfo *flinfo, char *str,
Datum *result);
extern Datum OidInputFunctionCall(Oid functionId, char *str,
Oid typioparam, int32 typmod);
+extern bool
+OidInputFunctionCallSafe(Oid functionId, char *str, Oid typioparam,
+ int32 typmod, fmNodePtr escontext,
+ Datum *result);
extern char *OutputFunctionCall(FmgrInfo
