Render timestamps as ISO8601 without a timezone

Timestamps do not have an associated timezone, so they shouldn't be render with one attached.
trinodb · Jun 6, 2020 · d469a1c · d469a1c
1 parent 20d960f
commit d469a1c
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 23 deletions.
diff --git a/presto-main/src/main/java/io/prestosql/operator/scalar/DateTimeFunctions.java b/presto-main/src/main/java/io/prestosql/operator/scalar/DateTimeFunctions.java
@@ -205,22 +205,24 @@ public static double toUnixTimeFromTimestampWithTimeZone(@SqlType(StandardTypes.
     }
 
     @ScalarFunction("to_iso8601")
-    @SqlType("varchar(35)")
-    // YYYY-MM-DDTHH:MM:SS.mmm+HH:MM is a standard notation, and it requires 29 characters.
-    // However extended notation with format ±(Y)+-MM-DDTHH:MM:SS.mmm+HH:MM is also acceptable and as
-    // the maximum year represented by 64bits timestamp is ~584944387 it may require up to 35 characters.
+    @SqlType("varchar(29)")
+    // YYYY-MM-DDTHH:MM:SS.mmm is a standard notation, and it requires 23 characters.
+    // However extended notation with format ±(Y)+-MM-DDTHH:MM:SS.mmm is also acceptable and as
+    // the maximum year represented by 64bits timestamp is ~584944387 it may require up to 29 characters.
     public static Slice toISO8601FromTimestamp(ConnectorSession session, @SqlType(StandardTypes.TIMESTAMP) long timestamp)
     {
+        ISOChronology chronology;
+
         if (session.isLegacyTimestamp()) {
-            DateTimeFormatter formatter = ISODateTimeFormat.dateTime()
-                    .withChronology(getChronology(session.getTimeZoneKey()));
-            return utf8Slice(formatter.print(timestamp));
+            chronology = getChronology(session.getTimeZoneKey());
         }
         else {
-            DateTimeFormatter formatter = ISODateTimeFormat.dateHourMinuteSecondMillis()
-                    .withChronology(UTC_CHRONOLOGY);
-            return utf8Slice(formatter.print(timestamp));
+            chronology = UTC_CHRONOLOGY;
         }
+
+        DateTimeFormatter formatter = ISODateTimeFormat.dateHourMinuteSecondMillis()
+                .withChronology(chronology);
+        return utf8Slice(formatter.print(timestamp));
     }
 
     @ScalarFunction("to_iso8601")

diff --git a/presto-main/src/test/java/io/prestosql/operator/scalar/TestDateTimeFunctions.java b/presto-main/src/test/java/io/prestosql/operator/scalar/TestDateTimeFunctions.java
@@ -32,12 +32,6 @@ public TestDateTimeFunctions()
         super(false);
     }
 
-    @Test
-    public void testToIso8601ForTimestampWithoutTimeZone()
-    {
-        assertFunction("to_iso8601(" + TIMESTAMP_LITERAL + ")", createVarcharType(35), TIMESTAMP_ISO8601_STRING_NO_TIME_ZONE);
-    }
-
     @Test
     public void testFormatDateCannotImplicitlyAddTimeZoneToTimestampLiteral()
     {

diff --git a/presto-main/src/test/java/io/prestosql/operator/scalar/TestDateTimeFunctionsBase.java b/presto-main/src/test/java/io/prestosql/operator/scalar/TestDateTimeFunctionsBase.java
@@ -130,6 +130,12 @@ protected TestDateTimeFunctionsBase(boolean legacyTimestamp)
         TIMESTAMP = legacyTimestamp ? LEGACY_TIMESTAMP : NEW_TIMESTAMP;
     }
 
+    @Test
+    public void testToIso8601ForTimestampWithoutTimeZone()
+    {
+        assertFunction("to_iso8601(" + TIMESTAMP_LITERAL + ")", createVarcharType(29), TIMESTAMP_ISO8601_STRING_NO_TIME_ZONE);
+    }
+
     @Test
     public void testCurrentDate()
     {

diff --git a/presto-main/src/test/java/io/prestosql/operator/scalar/TestDateTimeFunctionsLegacy.java b/presto-main/src/test/java/io/prestosql/operator/scalar/TestDateTimeFunctionsLegacy.java
@@ -23,7 +23,6 @@
 
 import static io.prestosql.spi.type.TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE;
 import static io.prestosql.spi.type.VarcharType.VARCHAR;
-import static io.prestosql.spi.type.VarcharType.createVarcharType;
 
 public class TestDateTimeFunctionsLegacy
         extends TestDateTimeFunctionsBase
@@ -33,12 +32,6 @@ public TestDateTimeFunctionsLegacy()
         super(true);
     }
 
-    @Test
-    public void testToIso8601ForTimestampWithoutTimeZone()
-    {
-        assertFunction("to_iso8601(" + TIMESTAMP_LITERAL + ")", createVarcharType(35), TIMESTAMP_ISO8601_STRING);
-    }
-
     @Test
     public void testFormatDateCanImplicitlyAddTimeZoneToTimestampLiteral()
     {