AbhishekPathania opened a new issue, #1293:
URL: https://github.com/apache/arrow-java/issues/1293
### Describe the bug, including details regarding any error messages,
version, and platform.
`DateTimeUtils.getTimestampValue` splits epoch millis into an epoch day and
a time-of-day remainder, then reassembles them into a `LocalDateTime`. Both
halves use operators that truncate toward zero:
```java
public static Timestamp getTimestampValue(long millisWithCalendar) {
long milliseconds = millisWithCalendar;
if (milliseconds < 0) {
// LocalTime#ofNanoDay only accepts positive values
milliseconds -= ((milliseconds / MILLIS_PER_DAY) - 1) * MILLIS_PER_DAY;
}
return Timestamp.valueOf(
LocalDateTime.of(
LocalDate.ofEpochDay(millisWithCalendar / MILLIS_PER_DAY),
LocalTime.ofNanoOfDay(TimeUnit.MILLISECONDS.toNanos(milliseconds %
MILLIS_PER_DAY))));
}
```
The time-of-day half hand-patches negative input so the remainder stays
non-negative (`LocalTime.ofNanoOfDay` rejects negatives). The epoch-day half
gets no such treatment, so for any negative value that is not an exact multiple
of a day the two halves describe different days, and the returned date is one
day later than the instant it was given.
**Worked example.** `-618102000000` millis is 1950-06-01 01:00:00 UTC.
* `-618102000000 / 86400000` truncates to `-7153`, and
`LocalDate.ofEpochDay(-7153)` is **1950-06-02**.
* The patched remainder is 3600000 ms, i.e. 01:00:00 — correct, but relative
to 1950-06-01.
* Result: `1950-06-02 01:00:00`. Expected: `1950-06-01 01:00:00`.
* `Math.floorDiv(-618102000000, 86400000)` is `-7154`, which is 1950-06-01,
the day the remainder actually belongs to.
Positive values are unaffected: `Math.floorDiv`/`Math.floorMod` agree with
`/` and `%` there.
**How it is reached.** `ArrowFlightJdbcDateVectorAccessor.getDate(Calendar)`
calls `DateTimeUtils.applyCalendarOffset` and hands the result to
`getTimestampValue`. A date vector value is day-aligned on its own, but the
calendar offset shifts it off that alignment for any `Calendar` whose zone
differs from the JVM default, which is the documented way to read a date in a
specific zone. So with a non-default `Calendar`, every pre-1970 date comes back
a day late. The offset does not have to be a whole number of hours — historical
zone offsets include values such as +05:53:28 — but any non-zero offset is
enough to trigger it.
**Why the existing test does not catch it.**
`DateTimeUtilsTest.testShouldGetTimestampNegative` uses `-618105600000`, which
is exactly 1950-06-01 00:00:00 UTC. At an exact day boundary the truncating and
flooring divisions agree, so the negative path is covered only at the one input
where the bug cannot appear.
**Expected behaviour.** `getTimestampValue` should return the timestamp for
the instant it is given, for negative input as well as positive.
**Fix.** Use `Math.floorDiv` and `Math.floorMod` for both halves; the manual
negative adjustment then becomes redundant and can be dropped.
**Version and platform.** Present on `main` and in 19.0.0, module
`flight-sql-jdbc-core`. Platform independent.
Possibly related but broader: #732 and #324 discuss timestamp/temporal
semantics in general. This report is narrower — a pure integer-division sign
bug in one helper.
I am willing to submit a PR for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
