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
trinodb · Dec 20, 2023 · cf77edb · cf77edb
1 parent 6d08816
commit cf77edb
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 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
@@ -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,19 +142,24 @@ 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")
     @LiteralParameters("x")
     @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
@@ -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