On Mon, Jun 22, 2026 at 10:51 PM jian he <[email protected]> wrote:
> Hi, > > In subquery_planner -> preprocess_expression -> > eval_const_expressions, we attempt > to evaluate certain constant expressions in an error-safe manner by > introducing > an ErrorSaveContext in eval_const_expressions_context. However, this may > introduce consistency issues, since not all functions are error-safe. > > Consider the following two queries: > SELECT CAST(65536 AS int2 DEFAULT NULL ON CONVERSION ERROR); > SELECT CAST(65536/0 AS int2 DEFAULT NULL ON CONVERSION ERROR); > ERROR: division by zero > > The first query returns NULL, while the second throws an error. Should we > accept > this inconsistency, or make the first query error out too? > It's not an inconsistency. The expression 65536/0 is evaluated and raises an error before the cast is executed. Similarly, should we reject uncastable CASTs, like `CAST(NULL::date AS > int2)`? > Right now that fails. I think it still should fail because there was no possible conversion to do, so the conversion didn't fail. I could be convinced that in the case of `SELECT CAST($1::date AS int2 DEFAULT 3 ON CONVERSION ERROR)` can recognize that there is no conversion path from "date" to "int2", and it should be optimized to just 3::int2 the same way that `(CASE WHEN FALSE THEN 5 ELSE 3 END)::int2` would be. > More broadly, the patch involved significant effort (refactoring) in > parsing (transformTypeCast), > planning (eval_const_expressions) to ensure expressions do not fail. > These efforts cannot ensure that all operations are error-safe, > therefore there will be consistency issues. > We only have to keep the _conversion_ error-safe. That conversion could be a no-op, could be an CoerceViaIO, or it could be a custom function. If the method is CoerceViaIO and the destination type does not have a safe input function, then the command should fail before execution. Likewise, if there is a custom function from type1->type2 defined, and it isn't error-safe, then the command should similarly fail. The user can then modify the SQL to remove the DEFAULT...ON CONVERSION ERROR clause from the CAST that can't be made safe and re-run. > This raises the question: how should we handle user-defined cast functions > in > CAST(expr AS target_type DEFAULT defexpr ON CONVERSION ERROR)? > > There are three options: > Option 1: Disallow them entirely. > Reject any cast expression backed by a user-defined function at parse time. > I strongly dislike this option. Option 2: Allow them with documented caveats. > Permit user-defined cast functions, but explicitly document the > limitations: > - SQL-language functions are inherently incapable of error-safe execution. > - C-language functions must ensure that all their subroutines are also > error-safe. > - It is the user's responsibility to ensure the cast function is > error-safe; if > it is not, errors will propagate as normal rather than being caught by the > DEFAULT clause. > > Option 2 means that CAST(expr AS target_type DEFAULT defexpr ON > CONVERSION ERROR) does not check if the underlying cast function > actually supports error-safe operations. > I think this would be a significant POLA violation. > Option 3: Disallow SQL-language functions, but allow C-language and > Internal-language functions. > Also document that you must ensure the C function is error-safe if you > want it to catch the error. > This is why I argued for adding a "SAFE" keyword on CREATE CAST and a corresponding safe flag in pg_cast or a value of 's' for castmethod to indicate a function that is declared error-safe back in [1]. There was pushback on the syntax itself, but I think the concept is still the best way forward. I don't think we disallow SQL functions for being SQL functions, but also there's presently no way for a SQL function to be safe. So the effect is the same. [1] https://www.postgresql.org/message-id/CADkLM%3DcFSg3%2B6Sk00dLAF7Q7jnrKBk6%2BN5gRxT5BCxRvaGtR-g%40mail.gmail.com > Currently, we are using Option 1. > Anyway, I rebased the patch and added comments in a few places that I > realized were necessary in hindsight. > > [1]: > https://postgr.es/m/cacjufxhm2e3dqmbrddzvwyg3zclyog6xfifvoz_tgy1tgw7...@mail.gmail.com > > I'll give this new patch a look, but resolving Option 1/2/3 seems most important.
