Fix ResultSet#getTimestamp for historical timestamps

Fix `getTimestamp` for dates before 1582. Also, fix `getTimestamp` for dates where time zone rules observed by `java.util.Date` and Java Time differ (for some zones, this is as late as beginning of XX century). The `getTimestamp` overload accepting a `Calendar` remains not fixed.
trinodb · Jun 12, 2020 · e6bd612 · e6bd612
1 parent 1949277
commit e6bd612
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 7 deletions.
diff --git a/presto-jdbc/src/main/java/io/prestosql/jdbc/PrestoResultSet.java b/presto-jdbc/src/main/java/io/prestosql/jdbc/PrestoResultSet.java
@@ -61,6 +61,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.CompletableFuture;
@@ -349,10 +350,10 @@ private Time getTime(int columnIndex, DateTimeZone localTimeZone)
     public Timestamp getTimestamp(int columnIndex)
             throws SQLException
     {
-        return getTimestamp(columnIndex, sessionTimeZone);
+        return getTimestamp(columnIndex, Optional.empty());
     }
 
-    private Timestamp getTimestamp(int columnIndex, DateTimeZone localTimeZone)
+    private Timestamp getTimestamp(int columnIndex, Optional<DateTimeZone> localTimeZone)
             throws SQLException
     {
         Object value = column(columnIndex);
@@ -1234,7 +1235,7 @@ public Time getTime(String columnLabel, Calendar cal)
     public Timestamp getTimestamp(int columnIndex, Calendar cal)
             throws SQLException
     {
-        return getTimestamp(columnIndex, DateTimeZone.forTimeZone(cal.getTimeZone()));
+        return getTimestamp(columnIndex, Optional.of(DateTimeZone.forTimeZone(cal.getTimeZone())));
     }
 
     @Override
@@ -1957,7 +1958,7 @@ private static List<ColumnInfo> getColumnInfo(List<Column> columns)
         return list.build();
     }
 
-    private static Timestamp parseTimestamp(String value, DateTimeZone localTimeZone)
+    private static Timestamp parseTimestamp(String value, Optional<DateTimeZone> localTimeZone)
     {
         Matcher matcher = DATETIME_PATTERN.matcher(value);
         if (!matcher.matches() || matcher.group("timezone") != null) {
@@ -1979,12 +1980,21 @@ private static Timestamp parseTimestamp(String value, DateTimeZone localTimeZone
             fractionValue = Long.parseLong(fraction);
         }
 
-        long epochSecond = LocalDateTime.of(year, month, day, hour, minute, second, 0)
-                .atZone(ZoneId.of(localTimeZone.getID()))
+        int nanos = (int) rescale(fractionValue, precision, 9);
+        LocalDateTime dateTime = LocalDateTime.of(year, month, day, hour, minute, second, 0);
+
+        if (!localTimeZone.isPresent()) {
+            Timestamp timestamp = Timestamp.valueOf(dateTime);
+            timestamp.setNanos(nanos);
+            return timestamp;
+        }
+
+        long epochSecond = dateTime
+                .atZone(ZoneId.of(localTimeZone.get().getID()))
                 .toEpochSecond();
 
         Timestamp timestamp = new Timestamp(epochSecond * 1000);
-        timestamp.setNanos((int) rescale(fractionValue, precision, 9));
+        timestamp.setNanos(nanos);
         return timestamp;
     }
 

diff --git a/presto-jdbc/src/test/java/io/prestosql/jdbc/TestJdbcResultSet.java b/presto-jdbc/src/test/java/io/prestosql/jdbc/TestJdbcResultSet.java
@@ -210,6 +210,18 @@ public void testObjectTypes()
 //            ...
 //        });
 
+        // distant past, but apparently not an uncommon value in practice; on this date Julian and Gregorian calendars should be in sync, but they appear not to be
+        checkRepresentation("TIMESTAMP '0001-01-01 00:00:00'", Types.TIMESTAMP, (rs, column) -> {
+            assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(1, 1, 1, 0, 0, 0)));
+            assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1, 1, 1, 0, 0, 0)));
+        });
+
+        // distant past, before Julian-Gregorian calendar "default cut-over", but after 0001-01-01 when Julian and Gregorian calendars are supposed to be in sync
+        checkRepresentation("TIMESTAMP '1300-01-01 00:00:00'", Types.TIMESTAMP, (rs, column) -> {
+            assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(1300, 1, 1, 0, 0, 0)));
+            assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1300, 1, 1, 0, 0, 0)));
+        });
+
         checkRepresentation("TIMESTAMP '2018-02-13 13:14:15.227 Europe/Warsaw'", Types.TIMESTAMP /* TODO TIMESTAMP_WITH_TIMEZONE */, (rs, column) -> {
             assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(2018, 2, 13, 6, 14, 15, 227_000_000))); // TODO this should represent TIMESTAMP '2018-02-13 13:14:15.227 Europe/Warsaw'
             assertThrows(() -> rs.getDate(column));