Prevent silent overflow in packDateTimeWithZone

trinodb · Jun 8, 2020 · 77273a2 · 77273a2
1 parent bd74518
commit 77273a2
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 6 deletions.
diff --git a/...ain/java/io/prestosql/operator/scalar/timestamp/TimestampToTimestampWithTimezoneCast.java b/...ain/java/io/prestosql/operator/scalar/timestamp/TimestampToTimestampWithTimezoneCast.java
@@ -13,6 +13,7 @@
  */
 package io.prestosql.operator.scalar.timestamp;
 
+import io.prestosql.spi.PrestoException;
 import io.prestosql.spi.connector.ConnectorSession;
 import io.prestosql.spi.function.LiteralParameter;
 import io.prestosql.spi.function.LiteralParameters;
@@ -22,6 +23,7 @@
 import io.prestosql.spi.type.StandardTypes;
 import org.joda.time.chrono.ISOChronology;
 
+import static io.prestosql.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
 import static io.prestosql.spi.function.OperatorType.CAST;
 import static io.prestosql.spi.type.DateTimeEncoding.packDateTimeWithZone;
 import static io.prestosql.type.Timestamps.round;
@@ -41,14 +43,22 @@ public static long cast(@LiteralParameter("p") long precision, ConnectorSession
             value = scaleEpochMicrosToMillis(round(value, 3));
         }
 
+        long millisUtc;
         if (session.isLegacyTimestamp()) {
-            return packDateTimeWithZone(value, session.getTimeZoneKey());
+            millisUtc = value;
+        }
+        else {
+            ISOChronology localChronology = getChronology(session.getTimeZoneKey());
+            // This cast does treat TIMESTAMP as wall time in session TZ. This means that in order to get
+            // its UTC representation we need to shift the value by the offset of TZ.
+            millisUtc = localChronology.getZone().convertLocalToUTC(value, false);
+        }
+        try {
+            return packDateTimeWithZone(millisUtc, session.getTimeZoneKey());
+        }
+        catch (IllegalArgumentException e) {
+            throw new PrestoException(INVALID_CAST_ARGUMENT, "Out of range for timestamp with time zone: " + millisUtc, e);
         }
-
-        ISOChronology localChronology = getChronology(session.getTimeZoneKey());
-        // This cast does treat TIMESTAMP as wall time in session TZ. This means that in order to get
-        // its UTC representation we need to shift the value by the offset of TZ.
-        return packDateTimeWithZone(localChronology.getZone().convertLocalToUTC(value, false), session.getTimeZoneKey());
     }
 
     @LiteralParameters("p")

diff --git a/presto-main/src/test/java/io/prestosql/operator/scalar/timestamp/BaseTestTimestamp.java b/presto-main/src/test/java/io/prestosql/operator/scalar/timestamp/BaseTestTimestamp.java
@@ -567,6 +567,12 @@ public void testCastToTimestampWithTimeZone()
 
         // 5-digit year in the past
         assertThat(assertions.expression("CAST(TIMESTAMP '-12001-05-01 12:34:56' AS TIMESTAMP WITH TIME ZONE)", session)).matches("TIMESTAMP '-12001-05-01 12:34:56 America/Los_Angeles'");
+
+        // Overflow
+        assertThatThrownBy(() -> assertions.expression("CAST(TIMESTAMP '123001-05-01 12:34:56' AS TIMESTAMP WITH TIME ZONE)", session))
+                .hasMessage("Out of range for timestamp with time zone: 3819379894496000");
+        assertThatThrownBy(() -> assertions.expression("CAST(TIMESTAMP '-123001-05-01 12:34:56' AS TIMESTAMP WITH TIME ZONE)", session))
+                .hasMessage("Out of range for timestamp with time zone: -3943693366326000");
     }
 
     @Test

diff --git a/presto-main/src/test/java/io/prestosql/type/TestTimestampWithTimeZoneBase.java b/presto-main/src/test/java/io/prestosql/type/TestTimestampWithTimeZoneBase.java
@@ -24,6 +24,7 @@
 
 import java.util.concurrent.TimeUnit;
 
+import static io.prestosql.spi.StandardErrorCode.INVALID_LITERAL;
 import static io.prestosql.spi.function.OperatorType.INDETERMINATE;
 import static io.prestosql.spi.type.BooleanType.BOOLEAN;
 import static io.prestosql.spi.type.DateType.DATE;
@@ -106,6 +107,11 @@ public void testLiteral()
         assertFunction("TIMESTAMP '-12001-01-02 03:04:05.321 Europe/Berlin'",
                 TIMESTAMP_WITH_TIME_ZONE,
                 new SqlTimestampWithTimeZone(new DateTime(-12001, 1, 2, 3, 4, 5, 321, BERLIN_ZONE).getMillis(), BERLIN_TIME_ZONE_KEY));
+
+        // Overflow
+        assertInvalidFunction("TIMESTAMP '123001-01-02 03:04:05.321 Europe/Berlin'", INVALID_LITERAL, "line 1:1: '123001-01-02 03:04:05.321 Europe/Berlin' is not a valid timestamp literal");
+        assertInvalidFunction("TIMESTAMP '+123001-01-02 03:04:05.321 Europe/Berlin'", INVALID_LITERAL, "line 1:1: '+123001-01-02 03:04:05.321 Europe/Berlin' is not a valid timestamp literal");
+        assertInvalidFunction("TIMESTAMP '-123001-01-02 03:04:05.321 Europe/Berlin'", INVALID_LITERAL, "line 1:1: '-123001-01-02 03:04:05.321 Europe/Berlin' is not a valid timestamp literal");
     }
 
     @Test

diff --git a/presto-spi/src/main/java/io/prestosql/spi/type/DateTimeEncoding.java b/presto-spi/src/main/java/io/prestosql/spi/type/DateTimeEncoding.java
@@ -26,6 +26,10 @@ private DateTimeEncoding() {}
 
     private static long pack(long millisUtc, short timeZoneKey)
     {
+        if (millisUtc << MILLIS_SHIFT >> MILLIS_SHIFT != millisUtc) {
+            throw new IllegalArgumentException("Millis overflow: " + millisUtc);
+        }
+
         return (millisUtc << MILLIS_SHIFT) | (timeZoneKey & TIME_ZONE_MASK);
     }