diff --git a/cpp/src/arrow/compute/kernels/vector_statistics.cc b/cpp/src/arrow/compute/kernels/vector_statistics.cc index 074f2ec0a738..df997ac25bf9 100644 --- a/cpp/src/arrow/compute/kernels/vector_statistics.cc +++ b/cpp/src/arrow/compute/kernels/vector_statistics.cc @@ -32,6 +32,7 @@ #include "arrow/scalar.h" #include "arrow/status.h" #include "arrow/util/bit_run_reader.h" +#include "arrow/util/bitmap_ops.h" #include "arrow/util/checked_cast.h" #include "arrow/util/logging_internal.h" @@ -127,7 +128,21 @@ 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]; + // ExecChunked seeds the output from the input chunk, so it can arrive carrying that + // chunk's offset. The buffers below are built for this slice alone and are read from + // bit and element zero, so the output owns no offset of its own. + out->offset = 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. + if (data.buffers[0] && data.offset != 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] = data.buffers[0]; + } ARROW_ASSIGN_OR_RAISE(out->buffers[1], ctx->Allocate(out->length * sizeof(CType))); // Avoid leaving uninitialized memory under null entries std::memset(out->buffers[1]->mutable_data(), 0, out->length * sizeof(CType)); diff --git a/cpp/src/arrow/compute/kernels/vector_statistics_test.cc b/cpp/src/arrow/compute/kernels/vector_statistics_test.cc index 97715cdaedd2..119481f59493 100644 --- a/cpp/src/arrow/compute/kernels/vector_statistics_test.cc +++ b/cpp/src/arrow/compute/kernels/vector_statistics_test.cc @@ -87,6 +87,48 @@ TEST_F(TestWinsorize, FloatingPoint) { } } +TEST_F(TestWinsorize, SlicedInput) { + // GH-51224: the output is zero-offset, so a sliced input's validity bitmap must be + // copied from the slice rather than shared, otherwise it is read from bit 0. + for (auto type : FloatingPointTypes()) { + options_.lower_limit = 0.0; + options_.upper_limit = 1.0; + // The parent's leading nulls sit at different positions than the slice's, so sharing + // the bitmap would move the nulls. + auto parent = ArrayFromJSON(type, "[1.1, 2.2, null, 4.4, null, 6.6, 7.7, 8.8]"); + auto expected = ArrayFromJSON(type, "[null, 4.4, null, 6.6, 7.7]"); + CheckWinsorize(parent->Slice(2, 5), expected); + } + for (auto type : IntTypes()) { + options_.lower_limit = 0.0; + options_.upper_limit = 1.0; + auto parent = ArrayFromJSON(type, "[1, 2, null, 4, null, 6, 7, 8]"); + auto expected = ArrayFromJSON(type, "[null, 4, null, 6, 7]"); + CheckWinsorize(parent->Slice(2, 5), expected); + } + // A slice of an array with no nulls at all keeps the null-free fast path. + options_.lower_limit = 0.25; + options_.upper_limit = 0.75; + auto dense = ArrayFromJSON(float64(), "[1.0, 2.0, 3.0, 44.0, 55.0, 66.0, 77.0]"); + CheckWinsorize(dense->Slice(1, 5), + ArrayFromJSON(float64(), "[3.0, 3.0, 44.0, 55.0, 55.0]")); +} + +TEST_F(TestWinsorize, SlicedChunkedInput) { + // ExecChunked seeds each output from the input chunk, so a sliced chunk carries a + // non-zero offset into ClipValues. The output buffers cover the slice alone. + options_.lower_limit = 0.0; + options_.upper_limit = 1.0; + auto parent = ArrayFromJSON(float64(), "[1.1, 2.2, null, 4.4, null, 6.6, 7.7, 8.8]"); + auto chunked = std::make_shared( + ArrayVector{parent->Slice(2, 3), parent->Slice(5, 3)}); + auto expected = std::make_shared(ArrayVector{ + ArrayFromJSON(float64(), "[null, 4.4, null]"), + ArrayFromJSON(float64(), "[6.6, 7.7, 8.8]"), + }); + CheckWinsorize(chunked, expected); +} + TEST_F(TestWinsorize, Integral) { for (auto type : IntTypes()) { options_.lower_limit = 0.25;