GH-50524: [C++] Honor array offset in pairwise_diff - #50858
Conversation
|
|
|
|
|
LGTM |
|
I checked the ARM64 macOS failures. ARM64 macOS 14 C++: ARM64 macOS GLib & Ruby: the build died in vendored google-cloud-cpp ( 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 |
pitrou
left a comment
There was a problem hiding this comment.
Thanks for the fix @Krishnanand-G ! This looks good to me.
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.