From cf77edb4b847e196d0fd13a6e2eccecf09ebc8ba Mon Sep 17 00:00:00 2001 From: Athul T R Date: Wed, 20 Dec 2023 22:50:18 +0530 Subject: [PATCH] Fail with proper error on overflow in from_unixtime The exception is due to the value user entered hence should be reported as a TrinoException rather than a GENERIC_INTERNAL_ERROR --- .../operator/scalar/DateTimeFunctions.java | 30 +++++++++++++++---- .../scalar/TestDateTimeFunctions.java | 21 +++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/operator/scalar/DateTimeFunctions.java b/core/trino-main/src/main/java/io/trino/operator/scalar/DateTimeFunctions.java index 34708653f848..37df8394fb58 100644 --- a/core/trino-main/src/main/java/io/trino/operator/scalar/DateTimeFunctions.java +++ b/core/trino-main/src/main/java/io/trino/operator/scalar/DateTimeFunctions.java @@ -127,7 +127,12 @@ public static Slice currentTimeZone(ConnectorSession session) public static long fromUnixTime(ConnectorSession session, @SqlType(StandardTypes.DOUBLE) double unixTime) { // TODO (https://github.com/trinodb/trino/issues/5781) - return packDateTimeWithZone(Math.round(unixTime * 1000), session.getTimeZoneKey()); + try { + return packDateTimeWithZone(Math.round(unixTime * 1000), session.getTimeZoneKey()); + } + catch (IllegalArgumentException e) { + throw new TrinoException(INVALID_FUNCTION_ARGUMENT, e); + } } @ScalarFunction("from_unixtime") @@ -137,11 +142,11 @@ public static long fromUnixTime(@SqlType(StandardTypes.DOUBLE) double unixTime, TimeZoneKey timeZoneKey; try { timeZoneKey = getTimeZoneKeyForOffset(toIntExact(hoursOffset * 60 + minutesOffset)); + return packDateTimeWithZone(Math.round(unixTime * 1000), timeZoneKey); } catch (IllegalArgumentException e) { throw new TrinoException(INVALID_FUNCTION_ARGUMENT, e); } - return packDateTimeWithZone(Math.round(unixTime * 1000), timeZoneKey); } @ScalarFunction("from_unixtime") @@ -149,7 +154,12 @@ public static long fromUnixTime(@SqlType(StandardTypes.DOUBLE) double unixTime, @SqlType("timestamp(3) with time zone") public static long fromUnixTime(@SqlType(StandardTypes.DOUBLE) double unixTime, @SqlType("varchar(x)") Slice zoneId) { - return packDateTimeWithZone(Math.round(unixTime * 1000), zoneId.toStringUtf8()); + try { + return packDateTimeWithZone(Math.round(unixTime * 1000), zoneId.toStringUtf8()); + } + catch (IllegalArgumentException e) { + throw new TrinoException(INVALID_FUNCTION_ARGUMENT, e); + } } @ScalarFunction("from_unixtime_nanos") @@ -172,7 +182,12 @@ public static LongTimestampWithTimeZone fromLong(@LiteralParameter("s") long sca epochSeconds -= 1; picosOfSecond += PICOSECONDS_PER_SECOND; } - return DateTimes.longTimestampWithTimeZone(epochSeconds, picosOfSecond, session.getTimeZoneKey().getZoneId()); + try { + return DateTimes.longTimestampWithTimeZone(epochSeconds, picosOfSecond, session.getTimeZoneKey().getZoneId()); + } + catch (ArithmeticException e) { + throw new TrinoException(INVALID_FUNCTION_ARGUMENT, e); + } } @LiteralParameters({"p", "s"}) @@ -216,7 +231,12 @@ public static long fromISO8601Timestamp(ConnectorSession session, @SqlType("varc DateTimeFormatter formatter = ISODateTimeFormat.dateTimeParser() .withChronology(getChronology(session.getTimeZoneKey())) .withOffsetParsed(); - return packDateTimeWithZone(parseDateTimeHelper(formatter, iso8601DateTime.toStringUtf8())); + try { + return packDateTimeWithZone(parseDateTimeHelper(formatter, iso8601DateTime.toStringUtf8())); + } + catch (IllegalArgumentException e) { + throw new TrinoException(INVALID_FUNCTION_ARGUMENT, e); + } } @ScalarFunction("from_iso8601_timestamp_nanos") diff --git a/core/trino-main/src/test/java/io/trino/operator/scalar/TestDateTimeFunctions.java b/core/trino-main/src/test/java/io/trino/operator/scalar/TestDateTimeFunctions.java index 0e7fd5af9971..9af8140ed98e 100644 --- a/core/trino-main/src/test/java/io/trino/operator/scalar/TestDateTimeFunctions.java +++ b/core/trino-main/src/test/java/io/trino/operator/scalar/TestDateTimeFunctions.java @@ -130,6 +130,10 @@ public void testFromUnixTime() assertThat(assertions.function("from_unixtime", "980172245.888")) .matches("TIMESTAMP '2001-01-22 03:04:05.888 Pacific/Apia'"); + + assertTrinoExceptionThrownBy(assertions.function("from_unixtime", "123456789123456789")::evaluate) + .hasErrorCode(INVALID_FUNCTION_ARGUMENT) + .hasMessage("Millis overflow: 9223372036854775807"); } @Test @@ -197,6 +201,10 @@ public void testFromUnixTimeNanos() assertThat(assertions.function("from_unixtime_nanos", "DECIMAL '-12345678900123456789.500'")) .matches("TIMESTAMP '1578-10-13 17:18:03.876543210 Pacific/Apia'"); + + assertTrinoExceptionThrownBy(assertions.function("from_unixtime_nanos", "DECIMAL '123456789123456789000000000'")::evaluate) + .hasErrorCode(INVALID_FUNCTION_ARGUMENT) + .hasMessage("long overflow"); } @Test @@ -214,6 +222,11 @@ public void testFromUnixTimeWithOffset() assertTrinoExceptionThrownBy(assertions.function("from_unixtime", "0", "-100", "100")::evaluate) .hasErrorCode(INVALID_FUNCTION_ARGUMENT); + + // test millisecond overflow + assertTrinoExceptionThrownBy(assertions.function("from_unixtime", "123456789123456789", "1", "1")::evaluate) + .hasErrorCode(INVALID_FUNCTION_ARGUMENT) + .hasMessage("Millis overflow: 9223372036854775807"); } @Test @@ -236,6 +249,10 @@ public void testFromUnixTimeWithTimeZone() assertThat(assertions.function("from_unixtime", "7200", "'America/Los_Angeles'")) .matches("TIMESTAMP '1969-12-31 18:00:00.000 America/Los_Angeles'"); + + assertTrinoExceptionThrownBy(assertions.function("from_unixtime", "123456789123456789", "'Asia/Kolkata'")::evaluate) + .hasErrorCode(INVALID_FUNCTION_ARGUMENT) + .hasMessage("Millis overflow: 9223372036854775807"); } @Test @@ -262,6 +279,10 @@ public void testFromISO8601() assertThat(assertions.function("from_iso8601_date", "'2001-08-22'")) .matches("DATE '2001-08-22'"); + + assertTrinoExceptionThrownBy(assertions.function("from_iso8601_timestamp", "'115023-03-21T10:45:30.00Z'")::evaluate) + .hasErrorCode(INVALID_FUNCTION_ARGUMENT) + .hasMessage("Millis overflow: 3567614928330000"); } @Test