Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions - Mailing list pgsql-hackers
| From | Corey Huinker |
|---|---|
| Subject | Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions |
| Date | |
| Msg-id | CADkLM=daTLuRcwzc6Egtwvh4XYgtABWuMBVnEznd-dXqmXfzUw@mail.gmail.com Whole thread Raw |
| In response to | Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions (jian he <jian.universality@gmail.com>) |
| List | pgsql-hackers |
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.
+ * 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.
pgsql-hackers by date: