DateTimeException misclassified as INTERNAL_ERROR

[JamesRTaylor]

Seeing the following exception which ends up being classified as an INTERNAL_ERROR instead of a USER_ERROR:

java.time.DateTimeException: Invalid value for HourOfDay (valid values 0 - 23): 24
	at java.base/java.time.temporal.ValueRange.checkValidValue(ValueRange.java:311)
	at java.base/java.time.temporal.ChronoField.checkValidValue(ChronoField.java:717)
	at java.base/java.time.LocalTime.of(LocalTime.java:339)
	at java.base/java.time.LocalDateTime.of(LocalDateTime.java:362)
	at java.base/java.time.ZonedDateTime.of(ZonedDateTime.java:339)
	at io.prestosql.operator.scalar.timestamp.VarcharToTimestampCast.castToShortTimestamp(VarcharToTimestampCast.java:132)
	at io.prestosql.operator.scalar.timestamp.VarcharToTimestampCast.castToLegacyShortTimestamp(VarcharToTimestampCast.java:96)
	at io.prestosql.operator.scalar.timestamp.VarcharToTimestampCast.castToShort(VarcharToTimestampCast.java:57)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:710)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:735)
	at io.prestosql.sql.InterpretedFunctionInvoker.invoke(InterpretedFunctionInvoker.java:100)
	at io.prestosql.sql.planner.ExpressionInterpreter$Visitor.visitCast(ExpressionInterpreter.java:1133)
	at io.prestosql.sql.tree.Cast.accept(Cast.java:91)
	at io.prestosql.sql.tree.AstVisitor.process(AstVisitor.java:27)
	at io.prestosql.sql.planner.ExpressionInterpreter$Visitor.visitComparisonExpression(ExpressionInterpreter.java:747)
	at io.prestosql.sql.tree.ComparisonExpression.accept(ComparisonExpression.java:71)
	at io.prestosql.sql.tree.AstVisitor.process(AstVisitor.java:27)
	at io.prestosql.sql.planner.ExpressionInterpreter$Visitor.visitLogicalBinaryExpression(ExpressionInterpreter.java:861)
	at io.prestosql.sql.tree.LogicalBinaryExpression.accept(LogicalBinaryExpression.java:88)
	at io.prestosql.sql.tree.AstVisitor.process(AstVisitor.java:27)
	at io.prestosql.sql.planner.ExpressionInterpreter.optimize(ExpressionInterpreter.java:287)
	at io.prestosql.sql.planner.iterative.rule.SimplifyExpressions.rewrite(SimplifyExpressions.java:54)
	at io.prestosql.sql.planner.iterative.rule.SimplifyExpressions.lambda$createRewrite$0(SimplifyExpressions.java:79)
	at io.prestosql.sql.planner.iterative.rule.ExpressionRewriteRuleSet$FilterExpressionRewrite.apply(ExpressionRewriteRuleSet.java:226)
	at io.prestosql.sql.planner.iterative.rule.ExpressionRewriteRuleSet$FilterExpressionRewrite.apply(ExpressionRewriteRuleSet.java:207)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.transform(IterativeOptimizer.java:168)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreNode(IterativeOptimizer.java:143)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:108)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:193)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:110)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:193)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:110)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:193)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:110)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:193)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:110)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:193)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:110)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:193)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:110)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:193)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:110)
	at io.prestosql.sql.planner.iterative.IterativeOptimizer.optimize(IterativeOptimizer.java:99)
	at io.prestosql.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:200)
	at io.prestosql.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:189)
	at io.prestosql.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:184)
	at io.prestosql.execution.SqlQueryExecution.doPlanQuery(SqlQueryExecution.java:433)
	at io.prestosql.execution.SqlQueryExecution.planQuery(SqlQueryExecution.java:421)
	at io.prestosql.execution.SqlQueryExecution.start(SqlQueryExecution.java:373)
	at io.prestosql.execution.SqlQueryManager.createQuery(SqlQueryManager.java:232)
	at io.prestosql.dispatcher.LocalDispatchQuery.lambda$startExecution$7(LocalDispatchQuery.java:132)
	at io.prestosql.$gen.Presto_338_12____20201102_161224_2.run(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)

[findepi]

The VarcharToTimestampCast should catch DateTimeException and rethrow as PrestoException with proper error code.

[martint]

It might be better to just validate the range of values for each field directly. There's an opportunity for producing better error messages.

[johnwhumphreys]

I'll try to do this one; assigned to myself. Found that it can be reproduced with:

select cast('2020-01-01 24:01:01' as timestamp)

java.time.DateTimeException: Invalid value for HourOfDay (valid values 0 - 23): 24
at java.base/java.time.temporal.ValueRange.checkValidValue(ValueRange.java:311)
at java.base/java.time.temporal.ChronoField.checkValidValue(ChronoField.java:717)

And it does report INTERNAL_ERROR/GENERIC_INTERNAL_ERROR (65536) on the UI.

[johnwhumphreys]

@martint

It might be better to just validate the range of values for each field directly. There's an opportunity for producing better error messages.

The DateTimeException has very good messages already, so I think we can just leverage them to give good messages out of the original error. Then we can avoid adding much extra logic. E.g.

        catch (DateTimeException e) {
            //Leverage highly specific error message from the source exception.
            throw new PrestoException(INVALID_CAST_ARGUMENT,
                    String.format("Value cannot be cast to timestamp; %s. ", e.getMessage()), e);
        }

Produces a user-error like:

SQL Error [9]: Query failed (#20201115_215031_00010_bqy8y): Value cannot be cast to timestamp; Invalid value for HourOfDay (valid values 0 - 23): 24.

or:

SQL Error [9]: Query failed (#20201115_215055_00011_bqy8y): Value cannot be cast to timestamp; Invalid value for MonthOfYear (valid values 1 - 12): 13.

For these two bad cast values respectively.

select cast('2020-13-01 23:61:01' as timestamp);
select cast('2020-13-01 23:59:01' as timestamp);

Does this look okay to you approach-wise?

[johnwhumphreys]

@martint - Made a PR with the above fix and some text cases for you to review. Happy to change it if you have any issues. Thanks!

[rnzucker]

This is still listed as Open, but it looks like it has been fixed. Is that correct?

[hashhar]

@rnzucker Thanks for bumping this up - I see the PR is still open but I cannot reproduce the behaviour anymore with select cast('2020-01-01 24:01:01' as timestamp).

Closing. Feel free to re-open if needed.

EDIT: I apparently misread the issue, it's still classified as an INTERNAL_ERROR. The PR needs to be rebased and should be good to go then.

[rnzucker]

Okay. I was just looking for issues for my son to start on for his first OSS project (in Java). Hence my question.

[sthandassery]

I see this has been handled already with newer version. @hashhar can you close this issue?

[hashhar]

Yes, I can confirm too. Thanks for verifying @sthandassery.