From 76d4a0d5b24f99b0de244384c2a456a5822b4f42 Mon Sep 17 00:00:00 2001 From: Gang Wu Date: Sat, 15 Apr 2023 00:31:46 +0800 Subject: [PATCH] GH-35138: [Java] Fix ArrowFlightJdbcTimeStampVectorAccessor to deal with calendar --- ...rrowFlightJdbcTimeStampVectorAccessor.java | 2 +- .../driver/jdbc/utils/DateTimeUtils.java | 4 +- ...ArrowFlightJdbcDateVectorAccessorTest.java | 4 +- ...FlightJdbcTimeStampVectorAccessorTest.java | 6 +- ...ArrowFlightJdbcTimeVectorAccessorTest.java | 4 +- ...owFlightJdbcVarCharVectorAccessorTest.java | 60 +++++++++++++------ .../driver/jdbc/utils/DateTimeUtilsTest.java | 2 +- 7 files changed, 54 insertions(+), 28 deletions(-) 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..82655f3bb9d 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 @@ -96,7 +96,7 @@ private LocalDateTime getLocalDateTime(Calendar calendar) { TimeZone timeZone = calendar.getTimeZone(); long millis = this.timeUnit.toMillis(value); localDateTime = localDateTime - .minus(timeZone.getOffset(millis) - this.timeZone.getOffset(millis), ChronoUnit.MILLIS); + .plus(timeZone.getOffset(millis) - this.timeZone.getOffset(millis), ChronoUnit.MILLIS); } return localDateTime; } 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..0d5cd4443ac 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 @@ -36,7 +36,7 @@ private DateTimeUtils() { } /** - * Subtracts given Calendar's TimeZone offset from epoch milliseconds. + * Adds given Calendar's TimeZone offset from epoch milliseconds. */ public static long applyCalendarOffset(long milliseconds, Calendar calendar) { if (calendar == null) { @@ -47,7 +47,7 @@ public static long applyCalendarOffset(long milliseconds, Calendar calendar) { final TimeZone defaultTz = TimeZone.getDefault(); if (tz != defaultTz) { - milliseconds -= tz.getOffset(milliseconds) - defaultTz.getOffset(milliseconds); + milliseconds += tz.getOffset(milliseconds) - defaultTz.getOffset(milliseconds); } return milliseconds; diff --git a/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessorTest.java b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessorTest.java index 36af5134626..ea5518b412c 100644 --- a/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessorTest.java +++ b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessorTest.java @@ -130,7 +130,7 @@ public void testShouldGetTimestampReturnValidTimestampWithCalendar() throws Exce long offset = timeZone.getOffset(resultWithoutCalendar.getTime()); - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); + collector.checkThat(result.getTime() - resultWithoutCalendar.getTime(), is(offset)); collector.checkThat(accessor.wasNull(), is(false)); }); } @@ -160,7 +160,7 @@ public void testShouldGetDateReturnValidDateWithCalendar() throws Exception { long offset = timeZone.getOffset(resultWithoutCalendar.getTime()); - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); + collector.checkThat(result.getTime() - resultWithoutCalendar.getTime(), 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/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..b008cdc129e 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 @@ -182,7 +182,7 @@ public void testShouldGetTimestampReturnValidTimestampWithCalendar() throws Exce long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) - timeZoneForVector.getOffset(resultWithoutCalendar.getTime()); - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); + collector.checkThat(result.getTime() - resultWithoutCalendar.getTime(), is(offset)); collector.checkThat(accessor.wasNull(), is(false)); }); } @@ -215,7 +215,7 @@ public void testShouldGetDateReturnValidDateWithCalendar() throws Exception { long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) - timeZoneForVector.getOffset(resultWithoutCalendar.getTime()); - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); + collector.checkThat(result.getTime() - resultWithoutCalendar.getTime(), is(offset)); collector.checkThat(accessor.wasNull(), is(false)); }); } @@ -248,7 +248,7 @@ public void testShouldGetTimeReturnValidTimeWithCalendar() throws Exception { long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) - timeZoneForVector.getOffset(resultWithoutCalendar.getTime()); - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); + collector.checkThat(result.getTime() - resultWithoutCalendar.getTime(), 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/accessor/impl/calendar/ArrowFlightJdbcTimeVectorAccessorTest.java b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeVectorAccessorTest.java index d2f7eb336af..f5c7145581b 100644 --- a/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeVectorAccessorTest.java +++ b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeVectorAccessorTest.java @@ -137,7 +137,7 @@ public void testShouldGetTimestampReturnValidTimestampWithCalendar() throws Exce long offset = timeZone.getOffset(resultWithoutCalendar.getTime()); - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); + collector.checkThat(result.getTime() - resultWithoutCalendar.getTime(), is(offset)); collector.checkThat(accessor.wasNull(), is(false)); }); } @@ -170,7 +170,7 @@ public void testShouldGetTimeReturnValidTimeWithCalendar() throws Exception { long offset = timeZone.getOffset(resultWithoutCalendar.getTime()); - collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset)); + collector.checkThat(result.getTime() - resultWithoutCalendar.getTime(), 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/accessor/impl/text/ArrowFlightJdbcVarCharVectorAccessorTest.java b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/text/ArrowFlightJdbcVarCharVectorAccessorTest.java index 799c517dd56..23eb9129548 100644 --- a/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/text/ArrowFlightJdbcVarCharVectorAccessorTest.java +++ b/java/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/text/ArrowFlightJdbcVarCharVectorAccessorTest.java @@ -510,14 +510,23 @@ public void testShouldGetDateReturnValidDateWithCalendar() throws Exception { Text value = new Text("2021-07-02"); when(getter.get(0)).thenReturn(value.copyBytes()); - Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("America/Sao_Paulo")); - Date result = accessor.getDate(calendar); + { + Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("America/Sao_Paulo")); + Date result = accessor.getDate(calendar); + calendar.setTime(result); - calendar = Calendar.getInstance(TimeZone.getTimeZone("Etc/UTC")); - calendar.setTime(result); + collector.checkThat(dateTimeFormat.format(calendar.getTime()), + equalTo("2021-07-01T21:00:00.000Z")); + } - collector.checkThat(dateTimeFormat.format(calendar.getTime()), - equalTo("2021-07-02T03:00:00.000Z")); + { + Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("Etc/UTC")); + Date result = accessor.getDate(calendar); + calendar.setTime(result); + + collector.checkThat(dateTimeFormat.format(calendar.getTime()), + equalTo("2021-07-02T00:00:00.000Z")); + } } @Test @@ -547,13 +556,21 @@ public void testShouldGetTimeReturnValidDateWithCalendar() throws Exception { Text value = new Text("02:30:00"); when(getter.get(0)).thenReturn(value.copyBytes()); - Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("America/Sao_Paulo")); - Time result = accessor.getTime(calendar); + { + Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("America/Sao_Paulo")); + Time result = accessor.getTime(calendar); + calendar.setTime(result); - calendar = Calendar.getInstance(TimeZone.getTimeZone("Etc/UTC")); - calendar.setTime(result); + collector.checkThat(timeFormat.format(calendar.getTime()), equalTo("23:30:00.000Z")); + } - collector.checkThat(timeFormat.format(calendar.getTime()), equalTo("05:30:00.000Z")); + { + Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("Etc/UTC")); + Time result = accessor.getTime(calendar); + calendar.setTime(result); + + collector.checkThat(timeFormat.format(calendar.getTime()), equalTo("02:30:00.000Z")); + } } @Test @@ -584,14 +601,23 @@ public void testShouldGetTimestampReturnValidDateWithCalendar() throws Exception Text value = new Text("2021-07-02 02:30:00.000"); when(getter.get(0)).thenReturn(value.copyBytes()); - Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("America/Sao_Paulo")); - Timestamp result = accessor.getTimestamp(calendar); + { + Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("America/Sao_Paulo")); + Timestamp result = accessor.getTimestamp(calendar); + calendar.setTime(result); - calendar = Calendar.getInstance(TimeZone.getTimeZone("Etc/UTC")); - calendar.setTime(result); + collector.checkThat(dateTimeFormat.format(calendar.getTime()), + equalTo("2021-07-01T23:30:00.000Z")); + } - collector.checkThat(dateTimeFormat.format(calendar.getTime()), - equalTo("2021-07-02T05:30:00.000Z")); + { + Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("Etc/UTC")); + Timestamp result = accessor.getTimestamp(calendar); + calendar.setTime(result); + + collector.checkThat(dateTimeFormat.format(calendar.getTime()), + equalTo("2021-07-02T02:30:00.000Z")); + } } private void assertGetBoolean(Text value, boolean expectedResult) throws SQLException { 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..4769e86e856 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,7 +67,7 @@ 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));