diff --git a/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessor.java b/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessor.java index a23883baf1e..f8688335ba1 100644 --- a/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessor.java +++ b/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessor.java @@ -25,7 +25,7 @@ import java.sql.Time; import java.sql.Timestamp; import java.time.LocalDateTime; -import java.time.temporal.ChronoUnit; +import java.time.ZoneId; import java.util.Calendar; import java.util.TimeZone; import java.util.concurrent.TimeUnit; @@ -45,13 +45,13 @@ public class ArrowFlightJdbcTimeStampVectorAccessor extends ArrowFlightJdbcAcces private final TimeZone timeZone; private final Getter getter; private final TimeUnit timeUnit; - private final LongToLocalDateTime longToLocalDateTime; + private final LongToUTCDateTime longToUTCDateTime; private final Holder holder; /** * Functional interface used to convert a number (in any time resolution) to LocalDateTime. */ - interface LongToLocalDateTime { + interface LongToUTCDateTime { LocalDateTime fromLong(long value); } @@ -67,7 +67,7 @@ public ArrowFlightJdbcTimeStampVectorAccessor(TimeStampVector vector, this.timeZone = getTimeZoneForVector(vector); this.timeUnit = getTimeUnitForVector(vector); - this.longToLocalDateTime = getLongToLocalDateTimeForVector(vector, this.timeZone); + this.longToUTCDateTime = getLongToUTCDateTimeForVector(vector); } @Override @@ -90,15 +90,19 @@ private LocalDateTime getLocalDateTime(Calendar calendar) { long value = holder.value; - LocalDateTime localDateTime = this.longToLocalDateTime.fromLong(value); + LocalDateTime localDateTime = this.longToUTCDateTime.fromLong(value); + ZoneId defaultTimeZone = Calendar.getInstance().getTimeZone().toZoneId(); + ZoneId sourceTimeZone; - if (calendar != null) { - TimeZone timeZone = calendar.getTimeZone(); - long millis = this.timeUnit.toMillis(value); - localDateTime = localDateTime - .minus(timeZone.getOffset(millis) - this.timeZone.getOffset(millis), ChronoUnit.MILLIS); + if (this.timeZone != null) { + sourceTimeZone = this.timeZone.toZoneId(); + } else if (calendar != null) { + sourceTimeZone = calendar.getTimeZone().toZoneId(); + } else { + sourceTimeZone = defaultTimeZone; } - return localDateTime; + + return localDateTime.atZone(sourceTimeZone).withZoneSameInstant(defaultTimeZone).toLocalDateTime(); } @Override @@ -149,23 +153,22 @@ protected static TimeUnit getTimeUnitForVector(TimeStampVector vector) { } } - protected static LongToLocalDateTime getLongToLocalDateTimeForVector(TimeStampVector vector, - TimeZone timeZone) { - String timeZoneID = timeZone.getID(); + protected static LongToUTCDateTime getLongToUTCDateTimeForVector(TimeStampVector vector) { + String timeZoneID = "UTC"; ArrowType.Timestamp arrowType = (ArrowType.Timestamp) vector.getField().getFieldType().getType(); switch (arrowType.getUnit()) { case NANOSECOND: - return nanoseconds -> DateUtility.getLocalDateTimeFromEpochNano(nanoseconds, timeZoneID); + return DateUtility::getLocalDateTimeFromEpochNano; case MICROSECOND: - return microseconds -> DateUtility.getLocalDateTimeFromEpochMicro(microseconds, timeZoneID); + return DateUtility::getLocalDateTimeFromEpochMicro; case MILLISECOND: - return milliseconds -> DateUtility.getLocalDateTimeFromEpochMilli(milliseconds, timeZoneID); + return DateUtility::getLocalDateTimeFromEpochMilli; case SECOND: return seconds -> DateUtility.getLocalDateTimeFromEpochMilli( - TimeUnit.SECONDS.toMillis(seconds), timeZoneID); + TimeUnit.SECONDS.toMillis(seconds)); default: throw new UnsupportedOperationException("Invalid Arrow time unit"); } @@ -177,7 +180,7 @@ protected static TimeZone getTimeZoneForVector(TimeStampVector vector) { String timezoneName = arrowType.getTimezone(); if (timezoneName == null) { - return TimeZone.getTimeZone("UTC"); + return null; } return TimeZone.getTimeZone(timezoneName); diff --git a/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtils.java b/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtils.java index dd94a09256d..c2f19610a79 100644 --- a/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtils.java +++ b/java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtils.java @@ -20,9 +20,11 @@ import static org.apache.calcite.avatica.util.DateTimeUtils.MILLIS_PER_DAY; import java.sql.Timestamp; +import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; import java.time.LocalTime; +import java.time.ZonedDateTime; import java.util.Calendar; import java.util.TimeZone; import java.util.concurrent.TimeUnit; @@ -36,21 +38,17 @@ private DateTimeUtils() { } /** - * Subtracts given Calendar's TimeZone offset from epoch milliseconds. + * Apply calendar timezone to epoch milliseconds. */ public static long applyCalendarOffset(long milliseconds, Calendar calendar) { if (calendar == null) { calendar = Calendar.getInstance(); } - - final TimeZone tz = calendar.getTimeZone(); - final TimeZone defaultTz = TimeZone.getDefault(); - - if (tz != defaultTz) { - milliseconds -= tz.getOffset(milliseconds) - defaultTz.getOffset(milliseconds); - } - - return milliseconds; + Instant currInstant = Instant.ofEpochMilli(milliseconds); + LocalDateTime getTimestampWithoutTZ = LocalDateTime.ofInstant(currInstant, + TimeZone.getTimeZone("UTC").toZoneId()); + ZonedDateTime parsedTime = getTimestampWithoutTZ.atZone(calendar.getTimeZone().toZoneId()); + return parsedTime.toEpochSecond() * 1000; } diff --git a/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessorTest.java b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessorTest.java index 38d842724b9..83a7e72a1cc 100644 --- a/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessorTest.java +++ b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessorTest.java @@ -17,6 +17,7 @@ package org.apache.arrow.driver.jdbc.accessor.impl.calendar; +import static java.util.Optional.ofNullable; import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcTimeStampVectorAccessor.getTimeUnitForVector; import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcTimeStampVectorAccessor.getTimeZoneForVector; import static org.hamcrest.CoreMatchers.equalTo; @@ -25,7 +26,9 @@ import java.sql.Date; import java.sql.Time; import java.sql.Timestamp; +import java.time.Instant; import java.time.LocalDateTime; +import java.time.ZonedDateTime; import java.util.Arrays; import java.util.Calendar; import java.util.Collection; @@ -170,21 +173,22 @@ public void testShouldGetTimestampReturnValidTimestampWithoutCalendar() throws E @Test public void testShouldGetTimestampReturnValidTimestampWithCalendar() throws Exception { + TimeZone.setDefault(TimeZone.getTimeZone("Asia/Hong_Kong")); + TimeZone timeZone = TimeZone.getTimeZone(AMERICA_SAO_PAULO); Calendar calendar = Calendar.getInstance(timeZone); - TimeZone timeZoneForVector = getTimeZoneForVector(vector); + TimeZone finalTimeZoneForResultWithoutCalendar = ofNullable(getTimeZoneForVector(vector)) + .orElse(TimeZone.getTimeZone("Asia/Hong_Kong")); accessorIterator.iterate(vector, (accessor, currentRow) -> { final Timestamp resultWithoutCalendar = accessor.getTimestamp(null); final Timestamp result = accessor.getTimestamp(calendar); - long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) - - timeZoneForVector.getOffset(resultWithoutCalendar.getTime()); - - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); - collector.checkThat(accessor.wasNull(), is(false)); + assertOffsetIsConsistentWithAccessorGetters(timeZone, finalTimeZoneForResultWithoutCalendar, result.getTime(), + resultWithoutCalendar.getTime(), accessor); }); + TimeZone.setDefault(null); } @Test @@ -206,17 +210,15 @@ public void testShouldGetDateReturnValidDateWithCalendar() throws Exception { TimeZone timeZone = TimeZone.getTimeZone(AMERICA_SAO_PAULO); Calendar calendar = Calendar.getInstance(timeZone); - TimeZone timeZoneForVector = getTimeZoneForVector(vector); + TimeZone finalTimeZoneForResultWithoutCalendar = ofNullable(getTimeZoneForVector(vector)) + .orElse(TimeZone.getDefault()); accessorIterator.iterate(vector, (accessor, currentRow) -> { final Date resultWithoutCalendar = accessor.getDate(null); final Date result = accessor.getDate(calendar); - long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) - - timeZoneForVector.getOffset(resultWithoutCalendar.getTime()); - - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); - collector.checkThat(accessor.wasNull(), is(false)); + assertOffsetIsConsistentWithAccessorGetters(timeZone, finalTimeZoneForResultWithoutCalendar, result.getTime(), + resultWithoutCalendar.getTime(), accessor); }); } @@ -239,17 +241,15 @@ public void testShouldGetTimeReturnValidTimeWithCalendar() throws Exception { TimeZone timeZone = TimeZone.getTimeZone(AMERICA_SAO_PAULO); Calendar calendar = Calendar.getInstance(timeZone); - TimeZone timeZoneForVector = getTimeZoneForVector(vector); + TimeZone finalTimeZoneForResultWithoutCalendar = ofNullable(getTimeZoneForVector(vector)) + .orElse(TimeZone.getDefault()); accessorIterator.iterate(vector, (accessor, currentRow) -> { final Time resultWithoutCalendar = accessor.getTime(null); final Time result = accessor.getTime(calendar); - long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) - - timeZoneForVector.getOffset(resultWithoutCalendar.getTime()); - - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); - collector.checkThat(accessor.wasNull(), is(false)); + assertOffsetIsConsistentWithAccessorGetters(timeZone, finalTimeZoneForResultWithoutCalendar, result.getTime(), + resultWithoutCalendar.getTime(), accessor); }); } @@ -270,8 +270,11 @@ private Timestamp getTimestampForVector(int currentRow) { } else if (object instanceof Long) { TimeUnit timeUnit = getTimeUnitForVector(vector); long millis = timeUnit.toMillis((Long) object); - long offset = TimeZone.getTimeZone(timeZone).getOffset(millis); - expectedTimestamp = new Timestamp(millis + offset); + + ZonedDateTime sourceTZDateTime = LocalDateTime + .ofInstant(Instant.ofEpochMilli(millis), TimeZone.getTimeZone("UTC").toZoneId()) + .atZone(TimeZone.getTimeZone(timeZone).toZoneId()); + expectedTimestamp = new Timestamp(sourceTZDateTime.toEpochSecond() * 1000); } return expectedTimestamp; } @@ -319,4 +322,18 @@ private void assertGetStringIsConsistentWithVarCharAccessor(Calendar calendar) t }); } } + + private void assertOffsetIsConsistentWithAccessorGetters(TimeZone timeZone, + TimeZone finalTimeZoneForResultWithoutCalendar, long result, + long resultWithoutCalendar, + ArrowFlightJdbcTimeStampVectorAccessor accessor) { + final TimeZone timeZoneForResult = getTimeZoneForVector(vector) == null ? timeZone : + finalTimeZoneForResultWithoutCalendar; + + long offset = timeZoneForResult.getOffset(result) - + finalTimeZoneForResultWithoutCalendar.getOffset(resultWithoutCalendar); + + collector.checkThat(resultWithoutCalendar - result, is(offset)); + collector.checkThat(accessor.wasNull(), is(false)); + } } diff --git a/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtilsTest.java b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtilsTest.java index adb892fcdc7..15e8fa2645e 100644 --- a/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtilsTest.java +++ b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtilsTest.java @@ -67,9 +67,9 @@ public void testShouldGetOffsetWithDifferentTimeZone() { TimeZone.setDefault(alternateTimezone); try { // Trying to guarantee timezone returns to its original value - final long expectedEpochMillis = epochMillis + offset; + final long expectedEpochMillis = epochMillis - offset; final long actualEpochMillis = DateTimeUtils.applyCalendarOffset(epochMillis, Calendar.getInstance( - defaultTimezone)); + alternateTimezone)); collector.checkThat(actualEpochMillis, is(expectedEpochMillis)); } finally {