GH-1293: Use floor division when splitting epoch millis into day and time - #1294
Open
AbhishekPathania wants to merge 1 commit into
Open
GH-1293: Use floor division when splitting epoch millis into day and time#1294AbhishekPathania wants to merge 1 commit into
AbhishekPathania wants to merge 1 commit into
Conversation
…y 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 apache#1293.
AbhishekPathania
requested review from
jbonofre,
laurentgo,
lidavidm and
wgtmac
as code owners
September 11, 2026 08:41
This comment has been minimized.
This comment has been minimized.
Author
|
The "Ensure PR format" check is failing only on the missing label step. I tried to add |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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,
-618102000000ms 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.The method now uses
Math.floorDivfor the epoch day andMath.floorModfor the time within the day. This also removes the need for the manual negative-remainder adjustment. Positive values behave as before.DateTimeUtilsTestnow tests 1950-06-01 01:00:00 UTC. The old code returned 1950-06-02 01:00:00 for this input. The test passes with the new floor-based calculation.Closes #1293.