Copilot commented on code in PR #51251:
URL: https://github.com/apache/arrow/pull/51251#discussion_r3963699746
##########
cpp/src/arrow/compute/kernels/vector_statistics.cc:
##########
@@ -127,7 +128,16 @@ 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];
+ // The output is zero-offset, so a sliced input's validity bitmap cannot
be shared as is:
+ // it would be read from bit 0 instead of from `data.offset`. Copy the
slice's bits out.
+ if (data.buffers[0]) {
+ ARROW_ASSIGN_OR_RAISE(
+ out->buffers[0], arrow::internal::CopyBitmap(ctx->memory_pool(),
+
data.buffers[0]->data(), data.offset,
+ data.length));
+ } else {
+ out->buffers[0] = nullptr;
+ }
Review Comment:
ClipValues now copies the validity bitmap even when `data.offset == 0`. For
non-sliced inputs with nulls this introduces an avoidable allocation+memcpy
(previously the bitmap could be safely shared because the output is also read
from bit 0). Consider only copying when `data.offset != 0` and otherwise
sharing the existing bitmap buffer.
--
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]
