Repository navigation
GH-55394: [C++][Compute] Check overflow when negating duration - #55395
Closed
DiadkoShmek wants to merge 1 commit into
Closed
DiadkoShmek wants to merge 1 commit into
DiadkoShmek wants to merge 1 commit into
Conversation
Signed-off-by: DiadkoShmek <207617272+DiadkoShmek@users.noreply.github.com>
|
|
Collaborator
|
We don't need AI only contribution. |
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
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.
Rationale for this change
Fixes GH-55394.
negate_checkedregisters duration kernels using uncheckedNegate, so a validINT64_MINduration silently wraps to the same negative value. Its mathematical negation is outside signed64 storage, and the documented checked behavior requires an overflow error.What changes are included in this PR?
Register duration kernels with the existing
NegateCheckedimplementation. Add regression coverage for all four duration units, scalar/array overflow, representable extrema and nulls, while keeping unchecked negation's wrap behavior.Are these changes tested?
The exact base and candidate production translation units were compiled separately and run in fresh private registries against the same pinned official PyArrow26.0.1 support libraries. An independent arbitrary precision integer oracle checks 90 input shapes/types and 5,265 outputs per build. The base fails 36 checked-duration overflow checks; the candidate passes all checks.
Coverage includes actual bitmap-valid duration minima, masked minimum payloads, four units, int64 controls, scalar/array/chunked/sliced/empty/all-null inputs, neighboring extrema and ordinary values. Checked absolute value and unchecked negation serve as controls.
A separate review oracle also checked 320 input representations and 26,925 native rows per build: the base has 456 mismatches, all checked-duration overflow, and the candidate has none.
The actual submitted native test translation unit passes syntax-only compilation. The added C++ test range also has zero replacements with the repository-pinned clang-format 18.1.8. These focused checks are not a full current-main build or an execution of the upstream gtest suite; full integration tests remain for CI.
Are there any user-facing changes?
negate_checkednow returns the promised overflow error for a valid minimum duration. Representable values, nulls, output duration units and unchecked negation remain unchanged.Was AI used for this PR?
I directed this work and take responsibility for the contribution and its follow-up. AI assistance was used in producing the change.
PR code and description written by:
Reviewed before submission by: