[CALCITE-6751] Reduction of CAST from string to interval is incorrect #4116
Conversation
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
| @@ -304,8 +304,8 @@ Expression translateCast( | |||
| ConstantExpression format) { | |||
| Expression convert = getConvertExpression(sourceType, targetType, operand, format); | |||
| Expression convert2 = checkExpressionPadTruncate(convert, sourceType, targetType); | |||
| Expression convert3 = expressionHandlingSafe(convert2, safe, targetType); | |||
| return scaleValue(sourceType, targetType, convert3); | |||
| Expression convert3 = scaleValue(sourceType, targetType, convert2); | |||
There was a problem hiding this comment.
this is the right order to do the conversion: first scaling, then checking for safety.
Otherwise the safe cast implementation returns null, which cannot be an input for the scaling computation.
| @@ -569,6 +569,13 @@ static void assertQuery( | |||
| try { | |||
| if (updateChecker == null) { | |||
| resultSet = statement.executeQuery(sql); | |||
| if (resultChecker == null && exceptionChecker != null) { | |||
There was a problem hiding this comment.
This is a separate bug, which was uncovered by the @NonDeterministic annotation on toInt. The exception was happening at compilation time, instead of runtime. This change is necessary to uncover exceptions that happen at runtime.
|
| @@ -4271,6 +4271,10 @@ public static int toInt(java.sql.Time v) { | |||
| return v == null ? castNonNull(null) : toInt(v); | |||
| } | |||
|
|
|||
| // Method tagged as non-deterministic because it can throw. | |||
| // The DeterministicCodeOptimizer may otherwise try to lift it out of try-catch blocks. | |||
| !ok | ||
|
|
||
| # Unfortunately, due to CALCITE-6752 the following test crashes | ||
| # SELECT TIME '10:00:00' + CAST('1.1' AS INTERVAL SECOND); |
There was a problem hiding this comment.
Can the annotations here add the correct results？Even if it can't run here.
| @@ -4271,6 +4271,10 @@ public static int toInt(java.sql.Time v) { | |||
| return v == null ? castNonNull(null) : toInt(v); | |||
| } | |||
|
|
|||
| // Method tagged as non-deterministic because it can throw. | |||
| // The DeterministicCodeOptimizer may otherwise try to lift it out of try-catch blocks. | |||
| // See https://issues.apache.org/jira/browse/CALCITE-6753 | |||
There was a problem hiding this comment.
CALCITE-6753 can be resolved in an independent PR.
While fixing this bug I uncovered two more bugs, https://issues.apache.org/jira/browse/CALCITE-6752 and https://issues.apache.org/jira/browse/CALCITE-6753. Some of the code changes are workarounds for these other bugs.