You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
ArraySpan::SetSlice replaces the offset. pairwise_diff copied the input span and then sliced from left_start/right_start, so a sliced array was read from the parent buffer. With [99, 1, 4, 9, 16] sliced to [1, 4, 9, 16], period=1 produced [-98, 3, 5, 7] instead of [null, 3, 5, 7].
What changes are included in this PR?
The kernel passes input.offset + left_start (and the same for the right side). The regression test uses a sliced int64 array for both period signs, on pairwise_diff and pairwise_diff_checked.
Are these changes tested?
Yes, TestPairwiseDiff.SlicedInput in vector_pairwise_test.cc.
Are there any user-facing changes?
pairwise_diff on a sliced array now diffs the sliced values. Callers who passed a slice and got parent-buffer values will see different (correct) output.
ARM64 macOS 14 C++: arrow-s3fs-test timed out at 300s (two retries). arrow-compute-vector-test passed (4.56s), including vector_pairwise_test.cc.
ARM64 macOS GLib & Ruby: the build died in vendored google-cloud-cpp (absl::visit marked deprecated, compiled with -Werror) while compiling filesystem/gcsfs.cc. vector_pairwise.cc compiled before that.
Same commit is green on ARM64 Ubuntu C++ and AMD64 macOS 15-intel C++.
#50524 still has no component labels. I don't have permission to add Component: C++ / Component: Python on the issue. A committer can add those if needed.
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
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.
github-actions
Bot
added
awaiting committer review
Rationale for this change
ArraySpan::SetSlicereplaces the offset.pairwise_diffcopied the input span and then sliced fromleft_start/right_start, so a sliced array was read from the parent buffer. With[99, 1, 4, 9, 16]sliced to[1, 4, 9, 16], period=1 produced[-98, 3, 5, 7]instead of[null, 3, 5, 7].What changes are included in this PR?
The kernel passes
input.offset + left_start(and the same for the right side). The regression test uses a sliced int64 array for both period signs, onpairwise_diffandpairwise_diff_checked.Are these changes tested?
Yes,
TestPairwiseDiff.SlicedInputinvector_pairwise_test.cc.Are there any user-facing changes?
pairwise_diffon a sliced array now diffs the sliced values. Callers who passed a slice and got parent-buffer values will see different (correct) output.