fix(compute): handle odd temporal multiples - #1119
Conversation
zeroshade
left a comment
There was a problem hiding this comment.
LGTM — odd-multiple temporal rounding now chooses the nearest representable multiple without disturbing even-multiple tie semantics.
The sign-sensitive integer paths and public temporal coverage look correct. I ran the targeted regression tests, go test ./arrow/compute/... -count=1 (4 packages), and an exhaustive nearest-multiple property probe over odd multiples 1–999 and values −5000–5000; all passed on head a5441022cba85.
This review was drafted by an AI-assisted tool and confirmed by an Apache Arrow Go maintainer. The maintainer approving this PR has read the findings and signed off. If something feels off, please reply on the PR and a maintainer will follow up.
More on how Apache Arrow Go handles maintainer review: CONTRIBUTING.md.
Rationale for this change
Half-up temporal rounding treats the lower remainder as an exact tie when Multiple is odd. For example, rounding 1 second to multiples of 3 seconds returns 3 seconds instead of 0.
What changes are included in this PR?
Only even multiples use an exact halfway tie. Odd multiples compare the remainder to the two neighboring multiples, including negative values.
Are these changes tested?
go test ./arrow/computeAre there any user-facing changes?
No API changes. Temporal rounding now returns the nearest result for odd multiples.