Thread
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Make cast functions to type money error safe
- b36b95640487 19 (unreleased) landed
-
Make cast function from circle to polygon error safe
- 26f9012beecf 19 (unreleased) landed
-
Make geometry cast functions error safe
- 45cdaf3665be 19 (unreleased) landed
-
Make cast functions from jsonb error safe
- 10e4d8aaf46f 19 (unreleased) landed
-
Make many cast functions error safe
- e2f289e5b9b8 19 (unreleased) landed
-
Add SQL/JSON query functions
- 6185c9737cf4 17.0 cited
-
Add soft error handling to some expression nodes
- aaaf9449ec6b 17.0 cited
-
CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2022-12-19T22:56:37Z
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. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-02T15:57:27Z
Corey Huinker <corey.huinker@gmail.com> 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
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
vignesh C <vignesh21@gmail.com> — 2023-01-03T12:10:39Z
On Tue, 20 Dec 2022 at 04:27, Corey Huinker <corey.huinker@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 -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Andrew Dunstan <andrew@dunslane.net> — 2023-01-03T17:08:56Z
On 2023-01-02 Mo 10:57, Tom Lane wrote: > Corey Huinker <corey.huinker@gmail.com> 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
Corey Huinker <corey.huinker@gmail.com> — 2023-01-03T18:02:36Z
On Mon, Jan 2, 2023 at 10:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Corey Huinker <corey.huinker@gmail.com> 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-f40d3e18bea8@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
Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-03T18:14:05Z
Corey Huinker <corey.huinker@gmail.com> writes: > On Mon, Jan 2, 2023 at 10:57 AM Tom Lane <tgl@sss.pgh.pa.us> 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-f40d3e18bea8@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
Vik Fearing <vik@postgresfriends.org> — 2023-01-03T18:32:58Z
On 1/3/23 19:14, Tom Lane wrote: > Corey Huinker <corey.huinker@gmail.com> writes: >> On Mon, Jan 2, 2023 at 10:57 AM Tom Lane <tgl@sss.pgh.pa.us> 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-f40d3e18bea8@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
Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-03T19:15:46Z
Vik Fearing <vik@postgresfriends.org> 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
Gregory Stark (as CFM) <stark.cfm@gmail.com> — 2023-03-28T18:52:50Z
On Tue, 3 Jan 2023 at 14:16, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Vik Fearing <vik@postgresfriends.org> 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
Isaac Morland <isaac.morland@gmail.com> — 2023-03-28T19:25:05Z
On Mon, 19 Dec 2022 at 17:57, Corey Huinker <corey.huinker@gmail.com> 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
Corey Huinker <corey.huinker@gmail.com> — 2023-03-28T20:06:20Z
On Tue, Mar 28, 2023 at 2:53 PM Gregory Stark (as CFM) <stark.cfm@gmail.com> wrote: > On Tue, 3 Jan 2023 at 14:16, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > Vik Fearing <vik@postgresfriends.org> 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
Corey Huinker <corey.huinker@gmail.com> — 2023-03-28T20:23:26Z
On Tue, Mar 28, 2023 at 3:25 PM Isaac Morland <isaac.morland@gmail.com> wrote: > On Mon, 19 Dec 2022 at 17:57, Corey Huinker <corey.huinker@gmail.com> > 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
jian he <jian.universality@gmail.com> — 2025-07-22T01:59:19Z
hi. more preparation work has been committed. 1. SQL/JSON patch [1] added keyword ERROR 2. CoerceViaIo, CoerceToDomain can be evaluated error safe. see commit [2]. 3. ExprState added ErrorSaveContext point, so before calling ExecInitExprRec set valid ErrorSaveContext for ExprState->escontext we should evaluate expression error softly. see commit [2] also. I only found oracle implement, [3]. Based on my reading of [4], it seems CAST(EXPRESSION AS TYPE DEFAULT def_expr ON ERROR) is not included in SQL:2023. anyway, just share my POC based on the previous patch in this thread. it will work for domain over composite, composite over domain. example: CREATE DOMAIN d_char3_not_null as char(3) NOT NULL; CREATE TYPE comp_domain_with_typmod AS (a d_char3_not_null, b int); SELECT CAST('(,42)' AS comp_domain_with_typmod DEFAULT NULL ON ERROR); --return NULL [1]: https://git.postgresql.org/cgit/postgresql.git/diff/src/backend/parser/gram.y?id=6185c9737cf48c9540782d88f12bd2912d6ca1cc [2]: https://git.postgresql.org/cgit/postgresql.git/commit/?id=aaaf9449ec6be62cb0d30ed3588dc384f56274bf [3] https://docs.oracle.com/en/database/oracle/oracle-database/23/sqlrf/CAST.html [4] https://peter.eisentraut.org/blog/2023/04/04/sql-2023-is-finished-here-is-whats-new -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Vik Fearing <vik@postgresfriends.org> — 2025-07-22T06:45:15Z
On 22/07/2025 03:59, jian he wrote: > Based on my reading of [4], it seems CAST(EXPRESSION AS TYPE DEFAULT > def_expr ON ERROR) > is not included in SQL:2023. > > [4]https://peter.eisentraut.org/blog/2023/04/04/sql-2023-is-finished-here-is-whats-new It was accepted into the standard after 2023 was released. I am the author of this change in the standard, so feel free to ask me anything you're unsure about. -- Vik Fearing
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-07-22T10:19:18Z
On Tue, Jul 22, 2025 at 2:45 PM Vik Fearing <vik@postgresfriends.org> wrote: > > It was accepted into the standard after 2023 was released. I am the > author of this change in the standard, so feel free to ask me anything > you're unsure about. > is the generally syntax as mentioned in this thread: CAST(source_expression AS target_type DEFAULT default_expression ON ERROR) if so, what's the restriction of default_expression?
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Vik Fearing <vik@postgresfriends.org> — 2025-07-22T12:26:25Z
On 22/07/2025 12:19, jian he wrote: > On Tue, Jul 22, 2025 at 2:45 PM Vik Fearing <vik@postgresfriends.org> wrote: >> It was accepted into the standard after 2023 was released. I am the >> author of this change in the standard, so feel free to ask me anything >> you're unsure about. >> > is the generally syntax as mentioned in this thread: > CAST(source_expression AS target_type DEFAULT default_expression ON ERROR) > > if so, what's the restriction of default_expression? The actual syntax is: <cast specification> ::= CAST <left paren> <cast operand> AS <cast target> [ FORMAT <cast template> ] [ <cast error behavior> ON CONVERSION ERROR ] <right paren> "CONVERSION" is probably a noise word, but it is there because A) Oracle wanted it there, and B) it makes sense because if the <cast error behavior> fails, that is still a failure of the entire CAST. The <cast error behavior> is: <cast error behavior> ::= ERROR | NULL | DEFAULT <value expression> but I am planning on removing the NULL variant in favor of having the <value expression> be a <contextually typed value specification>. So it would be either ERROR ON CONVERSION ERROR (postgres's current behavior), or DEFAULT NULL ON CONVERSION ERROR. An example of B) above would be: CAST('five' AS INTEGER DEFAULT 'six' ON CONVERSION ERROR). 'six' is no more an integer than 'five' is, so that would error out because the conversion error does not happen on the operand but on the default clause. CAST('five' AS INTEGER DEFAULT 6 ON CONVERSION ERROR) would work. -- Vik Fearing -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Vik Fearing <vik@postgresfriends.org> — 2025-07-22T12:45:42Z
On 22/07/2025 14:26, Vik Fearing wrote: > The <cast error behavior> is: > > <cast error behavior> ::= > ERROR > | NULL > | DEFAULT <value expression> > > but I am planning on removing the NULL variant in favor of having the > <value expression> be a <contextually typed value specification>. So > it would be either ERROR ON CONVERSION ERROR (postgres's current > behavior), or DEFAULT NULL ON CONVERSION ERROR. Sorry, I meant <implicitly typed value specification>. The point being that CAST(ARRAY['1', '2', 'three'] AS INTEGER ARRAY DEFAULT NULL ON CONVERSION ERROR) will give you (CAST NULL AS INTEGER ARRAY) and *not* ARRAY[1, 2, NULL]. -- Vik Fearing
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-07-22T15:55:33Z
On Tue, Jul 22, 2025 at 2:45 AM Vik Fearing <vik@postgresfriends.org> wrote: > > On 22/07/2025 03:59, jian he wrote: > > Based on my reading of [4], it seems CAST(EXPRESSION AS TYPE DEFAULT > > def_expr ON ERROR) > > is not included in SQL:2023. > > > > [4] > https://peter.eisentraut.org/blog/2023/04/04/sql-2023-is-finished-here-is-whats-new > > > It was accepted into the standard after 2023 was released. I am the > author of this change in the standard, so feel free to ask me anything > you're unsure about. > > That's excellent news. I was already planning on retrying this for v19, but I'll try sooner now.
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-07-24T01:22:19Z
On Tue, Jul 22, 2025 at 8:26 PM Vik Fearing <vik@postgresfriends.org> wrote: > > The actual syntax is: > > > <cast specification> ::= > CAST <left paren> > <cast operand> AS <cast target> > [ FORMAT <cast template> ] > [ <cast error behavior> ON CONVERSION ERROR ] > <right paren> > > > "CONVERSION" is probably a noise word, but it is there because A) Oracle > wanted it there, and B) it makes sense because if the <cast error > behavior> fails, that is still a failure of the entire CAST. > > > The <cast error behavior> is: > > <cast error behavior> ::= > ERROR > | NULL > | DEFAULT <value expression> > > > but I am planning on removing the NULL variant in favor of having the > <value expression> be a <contextually typed value specification>. So it > would be either ERROR ON CONVERSION ERROR (postgres's current behavior), > or DEFAULT NULL ON CONVERSION ERROR. > > > An example of B) above would be: CAST('five' AS INTEGER DEFAULT 'six' ON > CONVERSION ERROR). 'six' is no more an integer than 'five' is, so that > would error out because the conversion error does not happen on the > operand but on the default clause. CAST('five' AS INTEGER DEFAULT 6 ON > CONVERSION ERROR) would work. > hi. > <cast error behavior> ::= > ERROR > | NULL > | DEFAULT <value expression> for <value expression> I disallow it from returning a set, or using aggregate or window functions. For example, the following three cases will fail: +SELECT CAST('a' as int DEFAULT sum(1) ON CONVERSION ERROR); --error +SELECT CAST('a' as int DEFAULT sum(1) over() ON CONVERSION ERROR); --error +SELECT CAST('a' as int DEFAULT ret_setint() ON CONVERSION ERROR) --error (ret_setint function is warped as (select 1 union all select 2)) for array coerce, which you already mentioned, i think the following is what we expected. +SELECT CAST('{234,def,567}'::text[] AS integer[] DEFAULT '{-1011}' ON CONVERSION ERROR); + int4 +--------- + {-1011} +(1 row) I didn't implement the [ FORMAT <cast template> ] part for now. please check the attached regress test and tests expected result. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-07-24T13:44:22Z
On Tue, Jul 22, 2025 at 8:26 PM Vik Fearing <vik@postgresfriends.org> wrote: > > > On 22/07/2025 12:19, jian he wrote: > > On Tue, Jul 22, 2025 at 2:45 PM Vik Fearing <vik@postgresfriends.org> wrote: > >> It was accepted into the standard after 2023 was released. I am the > >> author of this change in the standard, so feel free to ask me anything > >> you're unsure about. > >> > > is the generally syntax as mentioned in this thread: > > CAST(source_expression AS target_type DEFAULT default_expression ON ERROR) > > > > if so, what's the restriction of default_expression? > > > The actual syntax is: > > > <cast specification> ::= > CAST <left paren> > <cast operand> AS <cast target> > [ FORMAT <cast template> ] > [ <cast error behavior> ON CONVERSION ERROR ] > <right paren> > > "CONVERSION" is probably a noise word, but it is there because A) Oracle > wanted it there, and B) it makes sense because if the <cast error > behavior> fails, that is still a failure of the entire CAST. > > > The <cast error behavior> is: > > > <cast error behavior> ::= > ERROR > | NULL > | DEFAULT <value expression> > > hi. just want to confirm my understanding of ``[ FORMAT <cast template> ]``. SELECT CAST('2022-13-32' AS DATE FORMAT 'YYYY-MM-DD' DEFAULT NULL ON CONVERSION ERROR); will return NULL. because ``SELECT to_date('2022-13-32', 'YYYY-MM-DD');`` will error out, so the above query will fall back to the DEFAULT expression evaluation. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Vik Fearing <vik@postgresfriends.org> — 2025-07-24T14:10:16Z
On 24/07/2025 03:22, jian he wrote: > +SELECT CAST('a' as int DEFAULT sum(1) ON CONVERSION ERROR); --error > +SELECT CAST('a' as int DEFAULT sum(1) over() ON CONVERSION ERROR); --error This seems like an arbitrary restriction. Can you explain why this is necessary? Those same expressions are allowed as the <cast operand>. > +SELECT CAST('a' as int DEFAULT ret_setint() ON CONVERSION ERROR) --error > (ret_setint function is warped as (select 1 union all select 2)) This makes sense to me. > for array coerce, which you already mentioned, i think the following > is what we expected. > +SELECT CAST('{234,def,567}'::text[] AS integer[] DEFAULT '{-1011}' ON > CONVERSION ERROR); > + int4 > +--------- > + {-1011} > +(1 row) Yes, that looks correct to me. > I didn't implement the [ FORMAT <cast template> ] part for now. That is fine, since it's separate feature > please check the attached regress test and tests expected result. Except for the weird restriction on the default value, this all looks good to me (with the usual caveat that I am not an expert in C). Are you planning to also implement the <castable predicate>? -- Vik Fearing -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Vik Fearing <vik@postgresfriends.org> — 2025-07-24T14:19:34Z
On 24/07/2025 15:44, jian he wrote: > just want to confirm my understanding of ``[ FORMAT <cast template> ]``. > > SELECT CAST('2022-13-32' AS DATE FORMAT 'YYYY-MM-DD' DEFAULT NULL ON > CONVERSION ERROR); > will return NULL. > because ``SELECT to_date('2022-13-32', 'YYYY-MM-DD');`` > will error out, so the above query will fall back to the DEFAULT > expression evaluation. That is correct. Any error produced during typecasting will fall back to the DEFAULT value. If not supplied, the behavior is ERROR ON ERROR as it currently is. Any error produced while converting the DEFAULT value to the requested type is raised as an error. -- Vik Fearing -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-07-30T19:14:58Z
> > > I didn't implement the [ FORMAT <cast template> ] part for now. > please check the attached regress test and tests expected result. > Question about this: +/* + * Push steps to evaluate a SafeTypeCastExpr and its various subsidiary expressions. + * We already handle CoerceViaIO, CoerceToDomain, and ArrayCoerceExpr error + * softly. However, FuncExpr (e.g., int84) cannot be made error-safe. + * In such cases, we wrap the source expression and target type information into + * a CoerceViaIO node instead. + */ I'm not sure we _can_ just fall back to the CoerceViaIO if there is a defined cast from TypeA -> TypeB. I seem to recall there was some reason we couldn't do that, possibly to do with how it handled rounding, but I have no clear memory of it. Aside from that, I like what you've done with making SafeTypeCastExpr be its own node type and not saddling regular typecasts with the overhead.
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-08-01T05:55:44Z
On Thu, Jul 31, 2025 at 3:15 AM Corey Huinker <corey.huinker@gmail.com> wrote: > > > Question about this: > > +/* > + * Push steps to evaluate a SafeTypeCastExpr and its various subsidiary expressions. > + * We already handle CoerceViaIO, CoerceToDomain, and ArrayCoerceExpr error > + * softly. However, FuncExpr (e.g., int84) cannot be made error-safe. > + * In such cases, we wrap the source expression and target type information into > + * a CoerceViaIO node instead. > + */ > > I'm not sure we _can_ just fall back to the CoerceViaIO if there is a defined cast from TypeA -> TypeB. I seem to recall there was some reason we couldn't do that, possibly to do with how it handled rounding, but I have no clear memory of it. > indeed. select ('11.1'::numeric::int); return 11, but '11.1' string can not coerce to int 11. So in this case, we can not use CoerceViaIO. so we need to handle numeric source types with fractional points with special care. currently, this applies only to numeric, float4, and float8. (hope this is all the corner case we need to catch...) select castsource::regtype, casttarget::regtype, castfunc::regproc, castcontext from pg_cast pc where castsource::regtype = ANY('{numeric, float4, float8}'::regtype[]) and castmethod = 'f'; only return 17 rows. one row is cast numreic to money, function numeric_cash. numeric_cash seems more trickly to be error safe, because it will call numeric_mul. so I made these 16 function errors safe. see v3-0001-make-some-numeric-cast-function-error-safe.patch -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-08-04T01:46:45Z
hi. fix the regress tests failure in https://api.cirrus-ci.com/v1/artifact/task/5894868779663360/testrun/build/testrun/regress/regress/regression.diffs
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-08-04T05:09:21Z
> > so we need to handle numeric source types with fractional points with > special care. > currently, this applies only to numeric, float4, and float8. > (hope this is all the corner case we need to catch...) > I'm fairly certain that the committers won't like us special-casing the internal cast functions, as we would have to maintain these special cases as new core types are added, and it still bypasses the defined cast function for user-defined types, which could have similar issues similar to the rounding issue. I think the way forward here is either to: 1. add a second function definition to CAST. The potential syntax forr a second function gets clumsy, but might look something like this: CREATE CAST (source_type AS target_type) WITH FUNCTION function_name [ (argument_type [, ...]) ] [ AS ASSIGNMENT | AS IMPLICIT ] [ WITH SAFE FUNCTION function_name [ (argument_type [, ...]) ] [ AS ASSIGNMENT | AS IMPLICIT ] ] That doesn't seem right to me, it seems easier to: 2. Modify the CAST definition to indicate whether the existing cast function has the regular function signature or a -Safe one. In cases where a CAST has a defined function but the safe flag is turned off, we would have to fail the query with an error like "Defined CAST function from srctype to desttype is not error-safe". This would involve changing the syntax of CREATE CAST by adding an option SAFE, or ERROR SAFE, or similar: CREATE CAST (source_type AS target_type) WITH [SAFE] FUNCTION function_name [ (argument_type [, ...]) ] [ AS ASSIGNMENT | AS IMPLICIT ] We would add a new value to pg_cast.castmethod, 's' for "safe". We could refactor all the numeric types to use the modified functions, so no special-case code there anymore, and it gives extension writers an incentive to (eventually) make their own cast functions error-safe. While method 2 seems a lot cleaner, there may be a performance regression in the now error-safe typecast functions. If so, that might tip the balance to having two functions defined. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-08-04T14:29:22Z
On Mon, Aug 4, 2025 at 1:09 PM Corey Huinker <corey.huinker@gmail.com> wrote: >> >> so we need to handle numeric source types with fractional points with >> special care. >> currently, this applies only to numeric, float4, and float8. >> (hope this is all the corner case we need to catch...) > > > I'm fairly certain that the committers won't like us special-casing the internal cast functions, as we would have to maintain these special cases as new core types are added, and it still bypasses the defined cast function for user-defined types, which could have similar issues similar to the rounding issue. > It's not special-casing the internal cast functions. It's how the cast being evaluated. There are two ways: CoerceViaIO, FuncExpr. generally if there is a pg_cast entry, postgres will use FuncExpr. but to safely cast evaluation (DEFAULT ON CONVERSION ERROR) we can not use FuncExpr in some cases. Because the FuncExpr associate function is not error safe. So in v4, we try to use CoerceViaIO to evaluate the case, but it turns out CoerceViaIO results are not the same as FuncExpr. one of the example is: select ('11.1'::numeric::int); In the end, it seems we need to make all these functions in the below query error safe. select castsource::regtype, casttarget::regtype, castfunc, castcontext,castmethod, pp.prosrc, pp.proname from pg_cast pc join pg_proc pp on pp.oid = pc.castfunc and pc.castfunc > 0 order by castsource::regtype; It's a lot of work, but seems doable, after playing around with it. I don't think we need to change the pg_cast catalog entry, we just need to make these function (pg_cast.castmethod) errors safe. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-08-05T04:10:33Z
> > > In the end, it seems we need to make all these functions in the below > query error safe. > select castsource::regtype, casttarget::regtype, castfunc, > castcontext,castmethod, pp.prosrc, pp.proname from pg_cast pc join pg_proc > pp on > pp.oid = pc.castfunc and pc.castfunc > 0 > order by castsource::regtype; > It's a lot of work, but seems doable, after playing around with it. > It is do-able. But that's just the cast functions that are part of core postgres. > > > I don't think we need to change the pg_cast catalog entry, > we just need to make these function (pg_cast.castmethod) errors safe. That would break any user-defined cast functions that were not also error safe, which is to say all of them. We need a way for user-defined cast functions to indicate whether or not they are error safe, and handle both situations accordingly (i.e. fail a CAST ON DEFAULT when the user-defined cast is not error-safe).
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-08-11T06:18:44Z
On Tue, Aug 5, 2025 at 12:10 PM Corey Huinker <corey.huinker@gmail.com> wrote: >> >> In the end, it seems we need to make all these functions in the below >> query error safe. >> select castsource::regtype, casttarget::regtype, castfunc, >> castcontext,castmethod, pp.prosrc, pp.proname from pg_cast pc join pg_proc pp on >> pp.oid = pc.castfunc and pc.castfunc > 0 >> order by castsource::regtype; >> It's a lot of work, but seems doable, after playing around with it. > > It is do-able. But that's just the cast functions that are part of core postgres. > hi. it's doable for most of the data types. but I found geometric type related cast function refactoring to error safe is quite challenging, so I skip that part refactoring. >> >> I don't think we need to change the pg_cast catalog entry, >> we just need to make these function (pg_cast.castmethod) errors safe. > > That would break any user-defined cast functions that were not also error safe, which is to say all of them. > > We need a way for user-defined cast functions to indicate whether or not they are error safe, and handle both situations accordingly (i.e. fail a CAST ON DEFAULT when the user-defined cast is not error-safe). > I understand what you mean now. CREATE CAST can use built-in functions too, we have no way to check whether these CREATE CAST associated functions are error safe or not. for example: CREATE CAST (jsonpath AS bytea) WITH FUNCTION jsonpath_send (jsonpath ) AS ASSIGNMENT; select '$'::jsonpath::bytea; To avoid this situation, we have to add a new column to pg_cast to indicate whether a cast function is error-safe. It's unlikely a pg_cast entry has two functions, one is error safe, one is not, adding pg_cast.casterrorsafefunc would not be that appropriate. so I choose pg_cast.casterrorsafe would be fine. pg_cast.casterrorsafe true means castfunc function is error safe, we can use it as safe cast evaluation (CAST... DEFAULT defexpr ON CONVERSION ERROR) please check the attached V6 script: v6-0001 to v6-0016 is about making existing pg_cast.castfunc function error safe. (commit message have associated query to explain the refactoring, as mentioned above, geometric and money associated type not refactored yet) v6-0017-make-ArrayCoerceExpr-error-safe.patch v6-0018-CAST-expr-AS-newtype-DEFAULT-ON-ERROR.patch is about (CAST... DEFAULT defexpr ON CONVERSION ERROR).
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-10-10T12:23:07Z
hi. First of all, rebase v6. select distinct castsource::regtype, casttarget::regtype, pp.prosrc from pg_cast pc join pg_proc pp on pp.oid = pc.castfunc join pg_type pt on pt.oid = castsource join pg_type pt1 on pt1.oid = casttarget and pc.castfunc > 0 and pt.typarray <> 0 and pt.typnamespace = 'pg_catalog'::regnamespace and pt1.typnamespace = 'pg_catalog'::regnamespace order by castsource::regtype, casttarget::regtype; The above query will list all cast functions in PG_CATALOG schema that need to be refactored to be error-safe. looking around, I found that the below source data type, the cast function is already error safe: boolean name "char" ("char" to integer was handled in integer cast part) regprocedure regoper regoperator regclass regtype regconfig regdictionary regproc oid regnamespace regrole regcollation regdatabase int4range numrange tsrange tstzrange daterange int8range xid8 time without time zone time with time zone After applying the attached v7 patch patchset, the below are cast functions that I didn't refactor to error safe yet. select pc.castsource::regtype, pc.casttarget::regtype, castfunc::regproc, pp.prosrc from pg_cast pc join pg_proc pp on pp.oid = pc.castfunc join pg_type pt on pt.oid = castsource join pg_type pt1 on pt1.oid = casttarget and pc.castfunc > 0 and pt.typnamespace = 'pg_catalog'::regnamespace and pt1.typnamespace = 'pg_catalog'::regnamespace and not pc.casterrorsafe; castsource | casttarget | castfunc | prosrc ------------+------------+----------------------+-------------- bigint | money | pg_catalog.money | int8_cash integer | money | pg_catalog.money | int4_cash numeric | money | pg_catalog.money | numeric_cash money | numeric | pg_catalog."numeric" | cash_numeric (4 rows) The reason I don't refactor these cast functions related to money data type is because 1. I am not sure if the PGLC_localeconv() function is error safe or not. 2. refactoring such ``DirectFunctionCall1(numeric_int8, amount));`` requires more effort. please check the attached v7 patch set: 01-error-safe-for-casting-bytea-to-other-types 02-error-safe-for-casting-bit-varbit-to-other-types 03-error-safe-for-casting-character-to-other-types 04-error-safe-for-casting-text-to-other-types 05-error-safe-for-casting-character-varying-to-other-types 06-error-safe-for-casting-inet-to-other-types 07-error-safe-for-casting-macaddr8-to-other-types 08-error-safe-for-casting-integer-to-other-types 09-error-safe-for-casting-bigint-to-other-types 10-error-safe-for-casting-numeric-to-other-types 11-error-safe-for-casting-float4-to-other-types 12-error-safe-for-casting-float8-to-other-types 13-error-safe-for-casting-date-to-other-types 14-error-safe-for-casting-interval-to-other-types 15-error-safe-for-casting-timestamptz-to-other-types 16-error-safe-for-casting-timestamp-to-other-types 17-error-safe-for-casting-jsonb-to-other-types 18-error-safe-for-casting-geometry-data-types 19-invent_some_error_safe_function.patch 20-CAST-expr-AS-newtype-DEFAULT-ON-ERROR.patch Each patch includes a commit message explaining which function is being refactored to be error-safe. I also made some changes for "20-CAST-expr-AS-newtype-DEFAULT-ON-ERROR.patch" If the source expression is Const, it can be unknown type or other type. * for UNKNOWN type, example: SELECT CAST ('\x112233445566778899'::bytea AS int8 DEFAULT NULL ON CONVERSION ERROR); We use CoerceUnknownConstSafe to verify earlier that such Const can be coerced to target data type or not. * for other data type, example: SELECT CAST ('\x112233445566778899'::bytea AS int8 DEFAULT NULL ON CONVERSION ERROR); we use evaluate_expr_safe to evaluate it error safe way earlier in transformTypeSafeCast. similar to transformPartitionBoundValue call evaluate_expr The geometry cast functions ``(poly_circle(PG_FUNCTION_ARGS)`` have problems making it error safe, working on it. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-10-14T02:00:26Z
On Fri, Oct 10, 2025 at 8:23 PM jian he <jian.universality@gmail.com> wrote: > After applying the attached v7 patch patchset, > the below are cast functions that I didn't refactor to error safe yet. > > select pc.castsource::regtype, > pc.casttarget::regtype, castfunc::regproc, pp.prosrc > from pg_cast pc join pg_proc pp on pp.oid = pc.castfunc > join pg_type pt on pt.oid = castsource > join pg_type pt1 on pt1.oid = casttarget > and pc.castfunc > 0 and pt.typnamespace = 'pg_catalog'::regnamespace > and pt1.typnamespace = 'pg_catalog'::regnamespace and not pc.casterrorsafe; > > castsource | casttarget | castfunc | prosrc > ------------+------------+----------------------+-------------- > bigint | money | pg_catalog.money | int8_cash > integer | money | pg_catalog.money | int4_cash > numeric | money | pg_catalog.money | numeric_cash > money | numeric | pg_catalog."numeric" | cash_numeric > (4 rows) > > The reason I don't refactor these cast functions related to money data > type is because > 1. I am not sure if the PGLC_localeconv() function is error safe or not. > 2. refactoring such ``DirectFunctionCall1(numeric_int8, amount));`` > requires more effort. > > please check the attached v7 patch set: > 01-error-safe-for-casting-bytea-to-other-types > 02-error-safe-for-casting-bit-varbit-to-other-types > 03-error-safe-for-casting-character-to-other-types > 04-error-safe-for-casting-text-to-other-types > 05-error-safe-for-casting-character-varying-to-other-types > 06-error-safe-for-casting-inet-to-other-types > 07-error-safe-for-casting-macaddr8-to-other-types > 08-error-safe-for-casting-integer-to-other-types > 09-error-safe-for-casting-bigint-to-other-types > 10-error-safe-for-casting-numeric-to-other-types > 11-error-safe-for-casting-float4-to-other-types > 12-error-safe-for-casting-float8-to-other-types > 13-error-safe-for-casting-date-to-other-types > 14-error-safe-for-casting-interval-to-other-types > 15-error-safe-for-casting-timestamptz-to-other-types > 16-error-safe-for-casting-timestamp-to-other-types > 17-error-safe-for-casting-jsonb-to-other-types > 18-error-safe-for-casting-geometry-data-types > 19-invent_some_error_safe_function.patch > 20-CAST-expr-AS-newtype-DEFAULT-ON-ERROR.patch > > Each patch includes a commit message explaining which function is being > refactored to be error-safe. > Summary of what this patch set is doing: To implement the syntax: CAST(source_expr AS target_type DEFAULT def_expr ON CONVERSION ERROR) we need to ensure that the transformed cast expression is error-safe. A transformed cast expression can be either a CoerceViaIO or a FuncExpr. Since CoerceViaIO is already error-safe but FuncExpr cannot be rewritten as CoerceViaIO one of the example: SELECT ('11.1'::numeric::int) So we need to refactor how pg_cast.castfunc works to make it error-safe. We also need to query pg_cast to determine whether a given pg_cast.castfunc is error-safe. For this, a new field casterrorsafe is added to pg_cast. Patches 01–18: Refactor all pg_cast.castfunc entries (except those with the money data type) to be error-safe. Patches 19–20: Implement support for CAST(... AS ... DEFAULT def_expr ON CONVERSION ERROR) Please check the attached v8. It includes minor changes compared to v7: mainly a small mistake fix in src/backend/jit/llvm/llvmjit_expr.c. Additionally, one occurrence of NO_XML_SUPPORT(); was replaced with errsave(escontext, …). -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-11-04T06:02:14Z
On Tue, Oct 14, 2025 at 10:00 AM jian he <jian.universality@gmail.com> wrote: > Please check the attached v8. > hi. please see the attached V9 patchset. v9-0001 to v9-0018: refactor pg_cast.castfunc entries, make it error safe. v9-0019: CAST(... AS ... DEFAULT def_expr ON CONVERSION ERROR) select pc.castsource::regtype,pc.casttarget::regtype, castfunc::regproc, pp.prosrc from pg_cast pc join pg_proc pp on pp.oid = pc.castfunc join pg_type pt on pt.oid = castsource join pg_type pt1 on pt1.oid = casttarget and pc.castfunc > 0 and pt.typnamespace = 'pg_catalog'::regnamespace and pt1.typnamespace = 'pg_catalog'::regnamespace and not pc.casterrorsafe; castsource | casttarget | castfunc | prosrc ------------+------------+----------------------+-------------- circle | polygon | pg_catalog.polygon | bigint | money | pg_catalog.money | int8_cash integer | money | pg_catalog.money | int4_cash numeric | money | pg_catalog.money | numeric_cash money | numeric | pg_catalog."numeric" | cash_numeric (5 rows) 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. 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); If source_expr is a Const and the cast expression is a FuncExpr, we can be certain that all arguments (FuncExpr->args) are also Const; see the function chain coerce_to_target_type → coerce_type → build_coercion_expression. We don’t need to worry about deparsing the expression because struct SafeTypeCastExpr includes source_expr, cast_expr, and default_expr. Even if cast_expr is NULL, we can still use source_expr to reconstruct the original CAST expression. 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. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-11-05T22:00:01Z
> > 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. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-11-11T03:06:11Z
On Thu, Nov 6, 2025 at 6:00 AM Corey Huinker <corey.huinker@gmail.com> wrote: > > 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. > Currently, patches v9-0001 through v9-0018 focus solely on refactoring pg_cast.castfunc. This refactoring is already valuable on its own because it establishes the infrastructure needed for error safe type casts. 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). 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. 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. -------------------------------------------------- The CAST DEFAULT patch is large, I tried to split some newly created function into a seperate patch. see v10-0019-invent-some-error-safe-functions.patch. SELECT CAST('five' AS INTEGER DEFAULT 6 ON CONVERSION ERROR); is not hard to make it error safe. ('five' is a simple Cons node) However, I found out, to make SELECT CAST('five'::INTEGER AS INTEGER DEFAULT 6 ON CONVERSION ERROR); error safe is hard. ('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. 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. -- jian https://www.enterprisedb.com/ -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-11-11T04:36:49Z
> > 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. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
amul sul <sulamul@gmail.com> — 2025-11-17T13:43:15Z
On Tue, Nov 11, 2025 at 8:37 AM jian he <jian.universality@gmail.com> wrote: > > On Thu, Nov 6, 2025 at 6:00 AM Corey Huinker <corey.huinker@gmail.com> wrote: > > [...] > > Currently, patches v9-0001 through v9-0018 focus solely on refactoring > pg_cast.castfunc. I had a quick look but haven't finished the full review due to lack of time today. Here are a few initial comments: v10-0003: - NO_XML_SUPPORT(); + errsave(escontext, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("unsupported XML feature"), + errdetail("This functionality requires the server to be built with libxml support.")); return NULL; Can use ereturn() instead of errsave() followed by return NULL. -- 10-0004: +/* error safe version of textToQualifiedNameList */ +List * +textToQualifiedNameListSafe(text *textval, Node *escontext) If I am not mistaken, it looks like an exact copy of textToQualifiedNameList(). I think you can simply keep only textToQualifiedNameListSafe() and call that from textToQualifiedNameList() with a NULL value for escontext. This way, all errsave() or ereturn() calls will behave like ereport(). The same logic applies to RangeVarGetRelidExtendedSafe() and makeRangeVarFromNameListSafe. These can be called from RangeVarGetRelidExtended() and makeRangeVarFromNameList(), respectively. -- + { + errsave(escontext, + errcode(ERRCODE_INVALID_NAME), + errmsg("invalid name syntax")); + return NIL; + } I prefer ereturn() instead of errsave + return. -- v10-0017 for (i = 0; i < lengthof(messages); i++) if (messages[i].type == type) - ereport(ERROR, + { + errsave(escontext, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg(messages[i].msg, sqltype))); + return; + } Here, I think, you can use ereturn() to return void. -- v10-0018 + if (unlikely(result == 0.0) && val1 != 0.0 && !isinf(val2)) + { + errsave(escontext, + errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("value out of range: underflow")); + + result = 0.0; + return result; + } Here, you can use ereturn() to return 0.0. Similar changes are needed at other places in the same patch. Regards, Amul -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-11-21T07:32:45Z
On Mon, Nov 10, 2025 at 11:36 PM Corey Huinker <corey.huinker@gmail.com> wrote: > 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. > With a second look at the patches, I think the organization is fine as-is. >> 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. > I can work on this part if you don't have time. > I'll get to reviewing this patchset soon. > Not as soon as I would have liked, but the patchset still applies without error, and survives all of my attempts to break it, including user defined functions that generate errors deterministically as well as non-deterministically, hoping to see it incorrectly use the default rather than reporting the error.
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-11-21T08:41:05Z
On Mon, Nov 17, 2025 at 9:43 PM Amul Sul <sulamul@gmail.com> wrote: > > 10-0004: > > +/* error safe version of textToQualifiedNameList */ > +List * > +textToQualifiedNameListSafe(text *textval, Node *escontext) > > If I am not mistaken, it looks like an exact copy of > textToQualifiedNameList(). I think you can simply keep only > textToQualifiedNameListSafe() and call that from > textToQualifiedNameList() with a NULL value for escontext. This way, > all errsave() or ereturn() calls will behave like ereport(). > > The same logic applies to RangeVarGetRelidExtendedSafe() and > makeRangeVarFromNameListSafe. These can be called from > RangeVarGetRelidExtended() and makeRangeVarFromNameList(), > respectively. > -- > hi. List * textToQualifiedNameList(text *textval) { List *namelist; rawname = text_to_cstring(textval); if (!SplitIdentifierString(rawname, '.', &namelist)) ereport(ERROR, (errcode(ERRCODE_INVALID_NAME), errmsg("invalid name syntax"))); } I don’t see any way to pass the escontext (ErrorSaveContext) without changing the textToQualifiedNameList function signature. There are around 30 occurrences of textToQualifiedNameList. changing the function textToQualifiedNameList signature is invasive, so I tend to avoid it. I think it's easier to just duplicate textToQualifiedNameList than changing the function textToQualifiedNameList signature. Am I missing something? -- jian https://www.enterprisedb.com -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-11-21T22:18:59Z
> > > The same logic applies to RangeVarGetRelidExtendedSafe() and > > makeRangeVarFromNameListSafe. These can be called from > > RangeVarGetRelidExtended() and makeRangeVarFromNameList(), > > respectively. > > -- > > > > I don’t see any way to pass the escontext (ErrorSaveContext) without > changing > the textToQualifiedNameList function signature. > ... > Am I missing something? > I think we need to keep these separate. The execution paths that don't care about capturing errors shouldn't be slowed down by minority of paths that do. That may change in the future, but if it does, we'll be getting rid of a lot of internal functions with this type of difference.
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
amul sul <sulamul@gmail.com> — 2025-11-24T03:38:08Z
On Fri, Nov 21, 2025 at 2:11 PM jian he <jian.universality@gmail.com> wrote: > > On Mon, Nov 17, 2025 at 9:43 PM Amul Sul <sulamul@gmail.com> wrote: > > > > 10-0004: > > > > +/* error safe version of textToQualifiedNameList */ > > +List * > > +textToQualifiedNameListSafe(text *textval, Node *escontext) > > > > If I am not mistaken, it looks like an exact copy of > > textToQualifiedNameList(). I think you can simply keep only > > textToQualifiedNameListSafe() and call that from > > textToQualifiedNameList() with a NULL value for escontext. This way, > > all errsave() or ereturn() calls will behave like ereport(). > > > > The same logic applies to RangeVarGetRelidExtendedSafe() and > > makeRangeVarFromNameListSafe. These can be called from > > RangeVarGetRelidExtended() and makeRangeVarFromNameList(), > > respectively. > > -- > > > > hi. > > List * > textToQualifiedNameList(text *textval) > { > List *namelist; > rawname = text_to_cstring(textval); > if (!SplitIdentifierString(rawname, '.', &namelist)) > ereport(ERROR, > (errcode(ERRCODE_INVALID_NAME), > errmsg("invalid name syntax"))); > } > > I don’t see any way to pass the escontext (ErrorSaveContext) without changing > the textToQualifiedNameList function signature. > > There are around 30 occurrences of textToQualifiedNameList. > changing the function textToQualifiedNameList signature is invasive, > so I tend to avoid it. > I think it's easier to just duplicate textToQualifiedNameList > than changing the function textToQualifiedNameList signature. > > Am I missing something? > The change I was suggesting will be as below: --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -2684,6 +2684,13 @@ name_text(PG_FUNCTION_ARGS) */ List * textToQualifiedNameList(text *textval) +{ + textToQualifiedNameListSafe(textval, NULL); +} + +/* error safe version of textToQualifiedNameList */ +List * +textToQualifiedNameListSafe(text *textval, Node *escontext) { char *rawname; List *result = NIL; We must try to avoid duplication whenever possible, as any bug fixes or enhancements would need to be copied to multiple places, which is often overlooked. Regards, Amul -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-11-25T02:59:42Z
On Mon, Nov 24, 2025 at 11:38 AM Amul Sul <sulamul@gmail.com> wrote: > > The change I was suggesting will be as below: > > --- a/src/backend/utils/adt/varlena.c > +++ b/src/backend/utils/adt/varlena.c > @@ -2684,6 +2684,13 @@ name_text(PG_FUNCTION_ARGS) > */ > List * > textToQualifiedNameList(text *textval) > +{ > + textToQualifiedNameListSafe(textval, NULL); > +} > + > +/* error safe version of textToQualifiedNameList */ > +List * > +textToQualifiedNameListSafe(text *textval, Node *escontext) > { > char *rawname; > List *result = NIL; > > > We must try to avoid duplication whenever possible, as any bug fixes > or enhancements would need to be copied to multiple places, which is > often overlooked. > hi. great idea! I incorporated all of your ideas into v11. I replaced all errsave to ereturn. I aslo simplified T_SafeTypeCastExpr expression initialization, evaluation logic within execExpr.c, execExprInterp.c. but one thing I didn't touch: float8_div. +static inline float8 +float8_div_safe(const float8 val1, const float8 val2, struct Node *escontext) but we can change float8_div to: static inline float8 float8_div(const float8 val1, const float8 val2) { return float8_div_safe(val1, val2, NULL); } I am worried that entering another function would cause a minor performance degradation. And since these simple functions are so simple, keeping them separated should not be a big problem. also I placed float8_div, float8_div_safe together. -- jian https://www.enterprisedb.com/ -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-11-25T12:27:52Z
On Fri, Nov 21, 2025 at 3:32 PM Corey Huinker <corey.huinker@gmail.com> wrote: > >>> >>> 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. > > > I can work on this part if you don't have time. Do you mean change pg_cast.casterrorsafe from boolean to char? Currently pg_cast.casterrorsafe works just fine. if the cast function is not applicable, the castfunc would be InvalidOid. also the cast function is either error safe or not, I don't see a usage case for the third value. attached v12-0004 fixes the xmlparse error, see https://cirrus-ci.com/task/6181359384788992?logs=build#L1217. all other patches remain the same as v11. -- jian https://www.enterprisedb.com/
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-12-01T05:41:37Z
> > >> 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. > > > > > > I can work on this part if you don't have time. > > Do you mean change pg_cast.casterrorsafe from boolean to char? > No, I meant implementing the syntax for being able to declare a custom CAST function as safe (or not). Basically adding the [SAFE] to CREATE CAST (*source_type* AS *target_type*) WITH [SAFE] FUNCTION *function_name* [ (*argument_type* [, ...]) ] I'm not tied to this syntax choice, but this one seemed the most obvious and least invasive. But this brings up an interesting point: if a cast is declared as WITHOUT FUNCTION aka COERCION_METHOD_BINARY, then the cast can never fail, and we should probably check for that because a cast that cannot fail can ignore the DEFAULT clause altogether and fall back to being an ordinary CAST(). > Currently pg_cast.casterrorsafe works just fine. > if the cast function is not applicable, the castfunc would be InvalidOid. > also the cast function is either error safe or not, I don't see a > usage case for the third value. > Agreed, it's fine. A committer may want a char with 's'/'u' values to keep all the options char types, but even if they do that's a very minor change. -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
amul sul <sulamul@gmail.com> — 2025-12-01T12:08:24Z
On Tue, Nov 25, 2025 at 8:30 AM jian he <jian.universality@gmail.com> wrote: > > On Mon, Nov 24, 2025 at 11:38 AM Amul Sul <sulamul@gmail.com> wrote: > > > > [...] > +static inline float8 > +float8_div_safe(const float8 val1, const float8 val2, struct Node *escontext) > > but we can change float8_div to: > > static inline float8 > float8_div(const float8 val1, const float8 val2) > { > return float8_div_safe(val1, val2, NULL); > } > I am worried that entering another function would cause a minor performance > degradation. And since these simple functions are so simple, keeping them > separated should not be a big problem. also I placed float8_div, > float8_div_safe together. Since you declared float8_div_safe() as static inline, I believe it wouldn't have any performance degradation since most compilers optimize it. Also, I suggest you pass the ErrorSafeContext to float_overflow_error(), float_underflow_error(), and float_zero_divide_error() so that you can avoid duplicating error messages. Regards, Amul -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-12-02T07:52:57Z
On Mon, Dec 1, 2025 at 8:09 PM Amul Sul <sulamul@gmail.com> wrote: > > Since you declared float8_div_safe() as static inline, I believe it > wouldn't have any performance degradation since most compilers > optimize it. Also, I suggest you pass the ErrorSafeContext to > float_overflow_error(), float_underflow_error(), and > float_zero_divide_error() so that you can avoid duplicating error > messages. > hi. First I want to use ereturn, then I found out float_overflow_error, float_underflow_error, float_zero_divide_error used both in float4, float8. ereturn would not be appropriate for both types. so I choose errsave. for these 3 functions, now it looks like: pg_noinline void float_overflow_error(struct Node *escontext) { errsave(escontext, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("value out of range: overflow"))); } pg_noinline void float_underflow_error(struct Node *escontext) { errsave(escontext, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("value out of range: underflow"))); } pg_noinline void float_zero_divide_error(struct Node *escontext) { errsave(escontext, (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); } -- jian https://www.enterprisedb.com/ -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
amul sul <sulamul@gmail.com> — 2025-12-04T12:46:50Z
On Tue, Dec 2, 2025 at 1:23 PM jian he <jian.universality@gmail.com> wrote: > > On Mon, Dec 1, 2025 at 8:09 PM Amul Sul <sulamul@gmail.com> wrote: > > > > Since you declared float8_div_safe() as static inline, I believe it > > wouldn't have any performance degradation since most compilers > > optimize it. Also, I suggest you pass the ErrorSafeContext to > > float_overflow_error(), float_underflow_error(), and > > float_zero_divide_error() so that you can avoid duplicating error > > messages. > > > hi. > > First I want to use ereturn, then I found out > float_overflow_error, float_underflow_error, float_zero_divide_error > used both in float4, float8. > ereturn would not be appropriate for both types. > so I choose errsave. > for these 3 functions, now it looks like: Make sense, thanks for updating the patch. Regarding v13-0019 and v13-0020 patches, I have a bunch of comments for the code, but I'd like to understand the implemented design first. I have some basic questions regarding the commit message and code comment, as follows: " We cannot simply prohibit user-defined functions in pg_cast for safe cast evaluation because CREATE CAST can also utilize built-in functions. So, to completely disallow custom casts created via CREATE CAST used in safe cast evaluation, a new field in pg_cast would unfortunately be necessary. " I might not have understood the implementation completely -- can't we use fmgr_last_builtin_oid to detect built-in functions? -- + /* + * We have to to use CoerceUnknownConstSafe rather than + * coerce_to_target_type. because coerce_to_target_type is not error + * safe. + */ How difficult would it be to modify coerce_to_target_type() to make it error safe? Could we utilize the existing type casting infrastructure for this, perhaps by simply considering the evolution and use of the additional default expression? If we could, the resulting implementation would be very clean, IMHO. Kindly excuse and point me if that is already discussed. -- Here are few comments for v13-0018: static inline void point_add_point(Point *result, Point *pt1, Point *pt2); +static inline bool point_add_point_safe(Point *result, Point *pt1, Point *pt2, + Node *escontext); +static inline float8 point_dt_safe(Point *pt1, Point *pt2, Node *escontext); static inline float8 point_sl(Point *pt1, Point *pt2); I think we should avoid introducing the "_safe" version of static routines in the .c file. Instead, we can add the Node *escontext argument to the existing routines, similar to how single_decode() was previously modified to accept it. -- -static void poly_to_circle(CIRCLE *result, POLYGON *poly); +static bool poly_to_circle_safe(CIRCLE *result, POLYGON *poly, Node *escontext); Following the previous suggestion, please keep the existing function as it is and just add one more argument to it. -- } + static inline float4 float4_mi(const float4 val1, const float4 val2) { A spurious change. -- Regards, Amul -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-12-08T14:58:00Z
On Thu, Dec 4, 2025 at 8:47 PM Amul Sul <sulamul@gmail.com> wrote: > > Regarding v13-0019 and v13-0020 patches, I have a bunch of comments > for the code, but I'd like to understand the implemented design first. > I have some basic questions regarding the commit message and code > comment, as follows: > > " > We cannot simply prohibit user-defined functions in pg_cast for safe cast > evaluation because CREATE CAST can also utilize built-in functions. So, to > completely disallow custom casts created via CREATE CAST used in safe cast > evaluation, a new field in pg_cast would unfortunately be necessary. > " > > I might not have understood the implementation completely -- can't we > use fmgr_last_builtin_oid to detect built-in functions? > -- hi, Amul. I have removed the pg_cast.casterrorsafe field. But in the future, to make the below (Examples) CASTs associated with built-in function error safe, I think we need pg_cast.casterrorsafe. Otherwise, how can we know if the pg_cast.castfunc (built-in function) is error safe or not. CREATE CAST (interval AS uuid ) WITH FUNCTION uuidv7(interval); CREATE CAST (jsonb AS text) WITH FUNCTION jsonb_array_element_text(jsonb, int); CREATE CAST (json AS text) WITH FUNCTION json_array_element_text(json, int); I’ve added a dedicated function: CoercionErrorSafeCheck, which checks whether the cast can be evaluated safely. If not, we throw an error. SELECT CAST('A' AS DATE DEFAULT NULL ON CONVERSION ERROR) Here, 'A' is an Unknown Const. As mentioned earlier, Unknown Const Node must be coerced error safe, otherwise, it will raise an error instead of returning NULL. The coercion of Unknown Const happens inside coerce_to_target_type, which means part of that function must run in an error safe. coerce_to_target_type is used widely, adding another parameter to it seems no good, so I introduced coerce_to_target_type_extended, coerce_type_extend. CoerceUnknownConstSafe is being removed. stringTypeDatumSafe, OidInputFunctionCallSafe functions are needed for coercing Unknown Const to the target type. Refactoring existing function seems not ideal, so i invented these two functions. -- jian https://www.enterprisedb.com -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-12-09T03:39:11Z
On Mon, Dec 1, 2025 at 1:41 PM Corey Huinker <corey.huinker@gmail.com> wrote: >> >> >> 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. >> > >> > >> > I can work on this part if you don't have time. >> >> Do you mean change pg_cast.casterrorsafe from boolean to char? > > > No, I meant implementing the syntax for being able to declare a custom CAST function as safe (or not). Basically adding the [SAFE] to > > CREATE CAST (source_type AS target_type) > WITH [SAFE] FUNCTION function_name [ (argument_type [, ...]) ] > > I'm not tied to this syntax choice, but this one seemed the most obvious and least invasive. > hi. In v14, I have removed pg_cast.casterrorsafe, but for user-defined CREATE CAST, castfunc can be built-in function or user-defined function, we do need a way to distinguish if the cast function is error safe or not. I’ll incorporate pg_cast.casterrorsafe along with the implementation of the user-defined CREATE CAST syntax into one patch. > But this brings up an interesting point: if a cast is declared as WITHOUT FUNCTION aka COERCION_METHOD_BINARY, then the cast can never fail, and we should probably check for that because a cast that cannot fail can ignore the DEFAULT clause altogether and fall back to being an ordinary CAST(). > >> integer and oid are binary coercible, but the following should fail. SELECT CAST(11 as oid DEFAULT 'a' ON CONVERSION ERROR); if you mean that skip + ExecInitExprRec((Expr *) stcstate->stcexpr->default_expr, + state, resv, resnull); For binary-coercible types, this approach seems fine. We’ve done something similar in ExecInitExprRec for T_ArrayCoerceExpr. ``` if (elemstate->steps_len == 1 && elemstate->steps[0].opcode == EEOP_CASE_TESTVAL) { /* Trivial, so we need no per-element work at runtime */ elemstate = NULL; } ``` in V14, I didn't do this part, I'll keep this in mind. -- jian https://www.enterprisedb.com/ -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-12-10T08:57:09Z
On Tue, Dec 9, 2025 at 11:39 AM jian he <jian.universality@gmail.com> wrote: > > On Mon, Dec 1, 2025 at 1:41 PM Corey Huinker <corey.huinker@gmail.com> wrote: > >> > > No, I meant implementing the syntax for being able to declare a custom CAST function as safe (or not). Basically adding the [SAFE] to > > > > CREATE CAST (source_type AS target_type) > > WITH [SAFE] FUNCTION function_name [ (argument_type [, ...]) ] > > > > I'm not tied to this syntax choice, but this one seemed the most obvious and least invasive. > > hi. please see the attached v15. the primary implementation of CAST DEFAULT is contained in V15-0021. changes compared to v14. 1. separate patch (v15-0017) for float error. -pg_noreturn extern void float_overflow_error(void); -pg_noreturn extern void float_underflow_error(void); -pg_noreturn extern void float_zero_divide_error(void); +extern void float_overflow_error(struct Node *escontext); +extern void float_underflow_error(struct Node *escontext); +extern void float_zero_divide_error(struct Node *escontext); 2. separate patch (v15-0018) for newly added float8 functions: float8_pl_safe float8_mi_safe float8_mul_safe float8_div_safe refactoring existing functions is too invasive, I choose not to. 3. refactor point_dt (v15-0019). This is necessary for making geometry data type error-safe, separate from the main patch (v15-0020). I hope to make it easier to review. -static inline float8 point_dt(Point *pt1, Point *pt2); +static inline float8 point_dt(Point *pt1, Point *pt2, Node *escontext); 4. skip compile DEFAULT expression (ExecInitExprRec) for binary coercion cast, as mentioned before. See ExecInitSafeTypeCastExpr. 5. Support user-defined type cast error-safe, see v15-0022. user-defined error-safe cast syntax: CREATE CAST (source_type AS target_type) WITH [SAFE] FUNCTION function_name [ (argument_type [, ...]) ] [ AS ASSIGNMENT | AS IMPLICIT ] this only adds a new keyword SAFE. This works for C and internal language functions only now. To make it really usable, I have made citext, hstore module castfunc error safe. A new column: pg_cast.casterrorsafe was added, this is needed for CREATE CAST WITH SAFE FUNCTION. +select CAST(ARRAY['a','g','b','h',null,'i'] AS hstore + DEFAULT NULL ON CONVERSION ERROR); + array +------- + +(1 row) + 6. slightly polished the doc. -- jian https://www.enterprisedb.com/ -
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Kirill Reshke <reshkekirill@gmail.com> — 2025-12-10T13:32:06Z
On Wed, 10 Dec 2025 at 13:58, jian he <jian.universality@gmail.com> wrote: > > On Tue, Dec 9, 2025 at 11:39 AM jian he <jian.universality@gmail.com> wrote: > > > > On Mon, Dec 1, 2025 at 1:41 PM Corey Huinker <corey.huinker@gmail.com> wrote: > > >> > > > No, I meant implementing the syntax for being able to declare a custom CAST function as safe (or not). Basically adding the [SAFE] to > > > > > > CREATE CAST (source_type AS target_type) > > > WITH [SAFE] FUNCTION function_name [ (argument_type [, ...]) ] > > > > > > I'm not tied to this syntax choice, but this one seemed the most obvious and least invasive. > > > > > hi. > please see the attached v15. > the primary implementation of CAST DEFAULT is contained in V15-0021. > > changes compared to v14. > 1. separate patch (v15-0017) for float error. > -pg_noreturn extern void float_overflow_error(void); > -pg_noreturn extern void float_underflow_error(void); > -pg_noreturn extern void float_zero_divide_error(void); > +extern void float_overflow_error(struct Node *escontext); > +extern void float_underflow_error(struct Node *escontext); > +extern void float_zero_divide_error(struct Node *escontext); > > 2. separate patch (v15-0018) for newly added float8 functions: > float8_pl_safe > float8_mi_safe > float8_mul_safe > float8_div_safe > refactoring existing functions is too invasive, I choose not to. > > 3. refactor point_dt (v15-0019). This is necessary for making geometry data type > error-safe, separate from the main patch (v15-0020). I hope to make it easier to > review. > -static inline float8 point_dt(Point *pt1, Point *pt2); > +static inline float8 point_dt(Point *pt1, Point *pt2, Node *escontext); > > 4. skip compile DEFAULT expression (ExecInitExprRec) for binary coercion cast, > as mentioned before. See ExecInitSafeTypeCastExpr. > > 5. Support user-defined type cast error-safe, see v15-0022. > user-defined error-safe cast syntax: > CREATE CAST (source_type AS target_type) > WITH [SAFE] FUNCTION function_name [ (argument_type [, ...]) ] > [ AS ASSIGNMENT | AS IMPLICIT ] > > this only adds a new keyword SAFE. > This works for C and internal language functions only now. > To make it really usable, I have made citext, hstore module castfunc error safe. > A new column: pg_cast.casterrorsafe was added, this is needed for > CREATE CAST WITH SAFE FUNCTION. > > +select CAST(ARRAY['a','g','b','h',null,'i'] AS hstore > + DEFAULT NULL ON CONVERSION ERROR); > + array > +------- > + > +(1 row) > + > > 6. slightly polished the doc. > > > -- > jian > https://www.enterprisedb.com/ Hi! Overall, I think this patch is doing a good thing. Also, are we holding it until the next SQL standard release, because sql/23 leaks this feature? Below are my 2c. 1) First of all, I would prefer the `Bumps catversion` comment in the commit msg of v15-0022. 2) In v15-0006, if dont understand when memory allocated by `result = (macaddr *) palloc0(sizeof(macaddr));` will be freed. Does it persist until the query ends? I tried to get OOM with a query that errors out macaddr8 casts repeatedly, but failed. 3) > * When error_safe set to true, we will evaluate the constant expression in a > * error safe way. If the evaluation fails, return NULL instead of throwing > * error. Somebody has to say it - s/error_safe set/error_safe is set/, also s/throwing error/throwing an error/ -- Best regards, Kirill Reshke
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
Corey Huinker <corey.huinker@gmail.com> — 2025-12-10T21:54:37Z
> > Overall, I think this patch is doing a good thing. Also, are we > holding it until the next SQL standard release, because sql/23 leaks > this feature? > The impression that I get is that the SQL Standard has become more descriptive and less prescriptive. It takes things that exist and builds the standard around those, so by implementing this draft feature, we help ensure that it makes it into the standard. This might be counter-intuitive, but that's what dictionaries do: they describe how the language is used (descriptive), rather than dictate how it should be used (prescriptive).
-
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
jian he <jian.universality@gmail.com> — 2025-12-15T04:37:43Z
hi. On Wed, Dec 10, 2025 at 9:32 PM Kirill Reshke <reshkekirill@gmail.com> wrote: > > Hi! > > Overall, I think this patch is doing a good thing. Also, are we > holding it until the next SQL standard release, because sql/23 leaks > this feature? > > Below are my 2c. > > 1) > First of all, I would prefer the `Bumps catversion` comment in the > commit msg of v15-0022. > ok. > 2) > In v15-0006, if dont understand when memory allocated by > `result = (macaddr *) palloc0(sizeof(macaddr));` will be freed. Does > it persist until the query ends? I tried to get OOM with a query that > errors out macaddr8 casts repeatedly, but failed. > if ((addr->d != 0xFF) || (addr->e != 0xFE)) - ereport(ERROR, + ereturn(fcinfo->context, (Datum) 0, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("macaddr8 data out of range to convert to macaddr"), errhint("Only addresses that have FF and FE as values in the " the change is minor, changing ereport to ereturn. The whole refactoring does not related to OOM errror, OOM errors will behave exactly as they did previously. > 3) > > * When error_safe set to true, we will evaluate the constant expression in a > > * error safe way. If the evaluation fails, return NULL instead of throwing > > * error. > > Somebody has to say it - s/error_safe set/error_safe is set/, also > s/throwing error/throwing an error/ > sure. mainly rebase due to recent palloc_object, palloc_array conflict. -- jian https://www.enterprisedb.com/