Skip to content

Commit

Permalink
Fix handling of dates and timestamps before the 20th century
Browse files Browse the repository at this point in the history
  • Loading branch information
@aalbu
aalbu committed Jul 30, 2020
1 parent 6f133e7 commit 70c3998
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 2 deletions.
39 changes: 37 additions & 2 deletions presto-jdbc/src/main/java/io/prestosql/jdbc/AbstractPrestoResultSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.prestosql.client.QueryStatusInfo;
import io.prestosql.jdbc.ColumnInfo.Nullable;
import org.joda.time.DateTimeZone;
import org.joda.time.LocalDate;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
import org.joda.time.format.DateTimeFormatterBuilder;
Expand Down Expand Up @@ -52,10 +53,12 @@
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.util.Calendar;
import java.util.GregorianCalendar;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.TimeZone;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
Expand All @@ -69,6 +72,8 @@
import static java.math.BigDecimal.ROUND_HALF_UP;
import static java.util.Locale.ENGLISH;
import static java.util.Objects.requireNonNull;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.joda.time.DateTimeZone.UTC;

abstract class AbstractPrestoResultSet
implements ResultSet
Expand Down Expand Up @@ -107,6 +112,10 @@ abstract class AbstractPrestoResultSet

static final DateTimeFormatter TIMESTAMP_FORMATTER = DateTimeFormat.forPattern("yyyy-MM-dd HH:mm:ss.SSS");

// Before 1900, Java Time and Joda Time are not consistent with java.sql.Date and java.util.Calendar
// Since January 1, 1900 UTC is still December 31, 1899 in other zones, we are adding a 1 year margin.
private static final long START_OF_MODERN_ERA = new LocalDate(1901, 1, 1).toDateTimeAtStartOfDay(UTC).getMillis();

private final DateTimeZone resultTimeZone;
protected final Iterator<List<Object>> results;
private final Map<String, Integer> fieldMap;
Expand Down Expand Up @@ -248,7 +257,23 @@ private Date getDate(int columnIndex, DateTimeZone localTimeZone)
}

try {
return new Date(DATE_FORMATTER.withZone(localTimeZone).parseMillis(String.valueOf(value)));
long millis = DATE_FORMATTER.withZone(localTimeZone).parseMillis(String.valueOf(value));
if (millis >= START_OF_MODERN_ERA) {
return new Date(millis);
}

// The chronology used by default by Joda is not historically accurate for dates
// preceding the introduction of the Gregorian calendar and is not consistent with
// java.sql.Date (the same millisecond value represents a different year/month/day)
// before the 20th century. For such dates we are falling back to using the more
// expensive GregorianCalendar; note that Joda also has a chronology that works for
// older dates, but it uses a slightly different algorithm and yields results that
// are not compatible with java.sql.Date.
LocalDate localDate = DATE_FORMATTER.parseLocalDate(String.valueOf(value));
Calendar calendar = new GregorianCalendar(localDate.getYear(), localDate.getMonthOfYear() - 1, localDate.getDayOfMonth());
calendar.setTimeZone(TimeZone.getTimeZone(localTimeZone.getID()));

return new Date(calendar.getTimeInMillis());
}
catch (IllegalArgumentException e) {
throw new SQLException("Invalid date from server: " + value, e);
Expand Down Expand Up @@ -1786,8 +1811,18 @@ private static Timestamp parseTimestamp(String value, Function<String, ZoneId> t
long epochSecond = LocalDateTime.of(year, month, day, hour, minute, second, 0)
.atZone(zoneId)
.toEpochSecond();
long epochMillis = SECONDS.toMillis(epochSecond);

Timestamp timestamp = new Timestamp(epochSecond * 1000);
Timestamp timestamp;
if (epochMillis >= START_OF_MODERN_ERA) {
timestamp = new Timestamp(epochMillis);
}
else {
// slower path, but accurate for historical dates
GregorianCalendar calendar = new GregorianCalendar(year, month - 1, day, hour, minute, second);
calendar.setTimeZone(TimeZone.getTimeZone(zoneId));
timestamp = new Timestamp(calendar.getTimeInMillis());
}
timestamp.setNanos((int) rescale(fractionValue, precision, 9));
return timestamp;
}
Expand Down
71 changes: 71 additions & 0 deletions presto-jdbc/src/test/java/io/prestosql/jdbc/TestJdbcResultSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,38 @@ public void testDate()
assertThrows(IllegalArgumentException.class, () -> rs.getTime(column));
assertThrows(IllegalArgumentException.class, () -> rs.getTimestamp(column));
});

// distant past, but apparently not an uncommon value in practice
checkRepresentation("DATE '0001-01-01'", Types.DATE, (rs, column) -> {
assertEquals(rs.getObject(column), Date.valueOf(LocalDate.of(1, 1, 1)));
assertEquals(rs.getDate(column), Date.valueOf(LocalDate.of(1, 1, 1)));
assertThrows(IllegalArgumentException.class, () -> rs.getTime(column));
assertThrows(IllegalArgumentException.class, () -> rs.getTimestamp(column));
});

// the Julian-Gregorian calendar "default cut-over"
checkRepresentation("DATE '1582-10-04'", Types.DATE, (rs, column) -> {
assertEquals(rs.getObject(column), Date.valueOf(LocalDate.of(1582, 10, 4)));
assertEquals(rs.getDate(column), Date.valueOf(LocalDate.of(1582, 10, 4)));
assertThrows(IllegalArgumentException.class, () -> rs.getTime(column));
assertThrows(IllegalArgumentException.class, () -> rs.getTimestamp(column));
});

// after the Julian-Gregorian calendar "default cut-over", but before the Gregorian calendar start
checkRepresentation("DATE '1582-10-10'", Types.DATE, (rs, column) -> {
assertEquals(rs.getObject(column), Date.valueOf(LocalDate.of(1582, 10, 10)));
assertEquals(rs.getDate(column), Date.valueOf(LocalDate.of(1582, 10, 10)));
assertThrows(IllegalArgumentException.class, () -> rs.getTime(column));
assertThrows(IllegalArgumentException.class, () -> rs.getTimestamp(column));
});

// the Gregorian calendar start
checkRepresentation("DATE '1582-10-15'", Types.DATE, (rs, column) -> {
assertEquals(rs.getObject(column), Date.valueOf(LocalDate.of(1582, 10, 15)));
assertEquals(rs.getDate(column), Date.valueOf(LocalDate.of(1582, 10, 15)));
assertThrows(IllegalArgumentException.class, () -> rs.getTime(column));
assertThrows(IllegalArgumentException.class, () -> rs.getTimestamp(column));
});
}

@Test
Expand Down Expand Up @@ -216,6 +248,45 @@ public void testTimestamp()
assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(2018, 2, 13, 13, 14, 15, 555_555_556)));
});

// distant past, but apparently not an uncommon value in practice
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)));
assertThrows(() -> rs.getDate(column));
assertThrows(() -> rs.getTime(column));
assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1, 1, 1, 0, 0, 0)));
});

// the Julian-Gregorian calendar "default cut-over"
checkRepresentation("TIMESTAMP '1582-10-04 00:00:00'", Types.TIMESTAMP, (rs, column) -> {
assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(1582, 10, 4, 0, 0, 0)));
assertThrows(() -> rs.getDate(column));
assertThrows(() -> rs.getTime(column));
assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1582, 10, 4, 0, 0, 0)));
});

// after the Julian-Gregorian calendar "default cut-over", but before the Gregorian calendar start
checkRepresentation("TIMESTAMP '1582-10-10 00:00:00'", Types.TIMESTAMP, (rs, column) -> {
assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(1582, 10, 10, 0, 0, 0)));
assertThrows(() -> rs.getDate(column));
assertThrows(() -> rs.getTime(column));
assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1582, 10, 10, 0, 0, 0)));
});

// the Gregorian calendar start
checkRepresentation("TIMESTAMP '1582-10-15 00:00:00'", Types.TIMESTAMP, (rs, column) -> {
assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(1582, 10, 15, 0, 0, 0)));
assertThrows(() -> rs.getDate(column));
assertThrows(() -> rs.getTime(column));
assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1582, 10, 15, 0, 0, 0)));
});

checkRepresentation("TIMESTAMP '1583-01-01 00:00:00'", Types.TIMESTAMP, (rs, column) -> {
assertEquals(rs.getObject(column), Timestamp.valueOf(LocalDateTime.of(1583, 1, 1, 0, 0, 0)));
assertThrows(() -> rs.getDate(column));
assertThrows(() -> rs.getTime(column));
assertEquals(rs.getTimestamp(column), Timestamp.valueOf(LocalDateTime.of(1583, 1, 1, 0, 0, 0)));
});

// TODO https://github.com/prestosql/presto/issues/37
// TODO line 1:8: '1970-01-01 00:14:15.123' is not a valid timestamp literal; the expected values will pro
// checkRepresentation("TIMESTAMP '1970-01-01 00:14:15.123'", Types.TIMESTAMP, (rs, column) -> {
Expand Down

0 comments on commit 70c3998

Please sign in to comment.