Presto: 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!