Skip to content

Commit d035fc8

Browse files
GH-1293: Use floor division when splitting epoch millis into day and time (#1294)
## What's Changed `DateTimeUtils.getTimestampValue(long)` used `/` and `%` to split epoch milliseconds into an epoch day and a time within that day. These operators round toward zero. For negative values that were not exactly midnight, the existing code fixed the remainder but not the epoch day. The two parts then referred to different days, so the timestamp came back one day late. For example, `-618102000000` ms is 1950-06-01 01:00:00 UTC. The old division produced epoch day `-7153`, which is 1950-06-02, while the remainder was 01:00. The method returned 1950-06-02 01:00:00. This affects DATE values before 1970 when `ArrowFlightJdbcDateVectorAccessor.getDate(Calendar)` applies a non-zero calendar offset. The offset moves the value away from midnight and exposes the division bug. Closes #1293.
1 parent 91b4a2c commit d035fc8

2 files changed

Lines changed: 16 additions & 8 deletions

File tree

‎flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtils.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,12 @@ public static long applyCalendarOffset(long milliseconds, Calendar calendar) {
5555
* @return a {@link Timestamp} object representing the given Epoch millis
5656
*/
5757
public static Timestamp getTimestampValue(long millisWithCalendar) {
58-
long milliseconds = millisWithCalendar;
59-
if (milliseconds < 0) {
60-
// LocalTime#ofNanoDay only accepts positive values
61-
milliseconds -= ((milliseconds / MILLIS_PER_DAY) - 1) * MILLIS_PER_DAY;
62-
}
63-
58+
// Millis are negative before 1970, where only floor semantics keep the epoch day
59+
// and the time-of-day remainder on the same day (and the remainder non-negative).
6460
return Timestamp.valueOf(
6561
LocalDateTime.of(
66-
LocalDate.ofEpochDay(millisWithCalendar / MILLIS_PER_DAY),
67-
LocalTime.ofNanoOfDay(TimeUnit.MILLISECONDS.toNanos(milliseconds % MILLIS_PER_DAY))));
62+
LocalDate.ofEpochDay(Math.floorDiv(millisWithCalendar, MILLIS_PER_DAY)),
63+
LocalTime.ofNanoOfDay(
64+
TimeUnit.MILLISECONDS.toNanos(Math.floorMod(millisWithCalendar, MILLIS_PER_DAY)))));
6865
}
6966
}

‎flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtilsTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,15 @@ public void testShouldGetTimestampNegative() {
9595

9696
assertThat(expected, is(actual));
9797
}
98+
99+
@Test
100+
public void testShouldGetTimestampNegativeNotAlignedToDay() {
101+
final long epochMilli = negativeEpochMilli + 3600000L; // 1950-06-01 01:00:00 UTC
102+
final Instant instant = Instant.ofEpochMilli(epochMilli);
103+
104+
final Timestamp expected = Timestamp.from(instant);
105+
final Timestamp actual = DateTimeUtils.getTimestampValue(epochMilli);
106+
107+
assertThat(expected, is(actual));
108+
}
98109
}

0 commit comments

Comments
 (0)