Skip to content

Commit 06ab323

Browse files
GH-1293: Use floor division when splitting epoch millis into day and time
DateTimeUtils.getTimestampValue split epoch millis into an epoch day and a time-of-day remainder using `/` and `%`, which truncate toward zero. Only the remainder half compensated for negative input, so for any negative value that was not an exact multiple of a day the two halves described different days and the returned timestamp was one day later than the instant given. This is reachable from ArrowFlightJdbcDateVectorAccessor.getDate(Calendar), where the calendar offset shifts a day-aligned date value off that alignment, making every pre-1970 date come back a day late. Use Math.floorDiv and Math.floorMod for both halves, which keeps the day and the remainder on the same day and makes the manual negative adjustment redundant. Positive input is unaffected, since both functions agree with `/` and `%` there. Closes #1293.
1 parent 91b4a2c commit 06ab323

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)