Copilot commented on code in PR #51251:
URL: https://github.com/apache/arrow/pull/51251#discussion_r3965807721
##########
cpp/src/arrow/compute/kernels/vector_statistics.cc:
##########
@@ -127,7 +128,17 @@ struct Winsorize {
DCHECK_EQ(out->buffers.size(), data.buffers.size());
out->null_count = data.null_count.load();
out->length = data.length;
- out->buffers[0] = data.buffers[0];
+ // A zero-offset input can share its validity bitmap, because the output
is read from
+ // bit 0 as well. A sliced input cannot: sharing would read the bitmap
from bit 0
+ // instead of from `data.offset`, so copy the slice's bits out.
Review Comment:
`ClipValues` allocates `out->buffers[1]` for `out->length` elements and then
writes via `out->GetMutableValues<CType>(1)`, which applies `out->offset`. If
`ClipValues` is ever called with a non-zero `out->offset` (e.g. in
`ExecChunked`, `out_data` starts as `chunk->data()->Copy()`, so a chunk that is
itself a slice can carry a non-zero offset), this can shift the write pointer
past the allocated buffer and also makes the copied validity bitmap (which
starts at bit 0) misaligned when read back. Set `out->offset = 0` when
constructing the output in `ClipValues`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
