From 2e8791f7b3c481aa2c3d9144b9f7f3b8ed1ced49 Mon Sep 17 00:00:00 2001 From: Abdul Azeem Makarim <114302821+A-makarim@users.noreply.github.com> Date: Tue, 8 Sep 2026 16:39:16 +0100 Subject: [PATCH 1/2] GH-51223: [C++] Fix copying sliced boolean arrays Fixes #51223 Signed-off-by: Abdul Azeem Makarim <114302821+A-makarim@users.noreply.github.com> --- .../arrow/compute/kernels/copy_data_internal.h | 4 +++- .../compute/kernels/vector_replace_test.cc | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/kernels/copy_data_internal.h b/cpp/src/arrow/compute/kernels/copy_data_internal.h index 735c2034c3f3..3dd8e14c6999 100644 --- a/cpp/src/arrow/compute/kernels/copy_data_internal.h +++ b/cpp/src/arrow/compute/kernels/copy_data_internal.h @@ -42,8 +42,10 @@ struct CopyDataUtils { static void CopyData(const DataType&, const ArraySpan& in, const int64_t in_offset, uint8_t* out, const int64_t out_offset, const int64_t length) { + // Boolean values are bit-packed, so apply the array's offset to the bit index + // rather than to the data pointer. const auto in_arr = in.GetValues(1, /*absolute_offset=*/0); - CopyData(*in.type, in_arr, in_offset, out, out_offset, length); + CopyData(*in.type, in_arr, in.offset + in_offset, out, out_offset, length); } }; diff --git a/cpp/src/arrow/compute/kernels/vector_replace_test.cc b/cpp/src/arrow/compute/kernels/vector_replace_test.cc index dc63bae39a54..0f248d2b1703 100644 --- a/cpp/src/arrow/compute/kernels/vector_replace_test.cc +++ b/cpp/src/arrow/compute/kernels/vector_replace_test.cc @@ -545,6 +545,14 @@ TEST_F(TestReplaceBoolean, ReplaceWithMask) { } } +TEST_F(TestReplaceBoolean, ReplaceWithMaskSlicedInput) { + auto input = + this->array("[true, false, null, true, true, false, null, true]")->Slice(3, 5); + + this->Assert(ReplaceWithMask, input, this->mask("[true, false, false, false, false]"), + this->array("[false]"), this->array("[false, true, false, null, true]")); +} + // Regression test: ReplaceMaskChunked (the ChunkedArray path of replace_with_mask) // sized each output chunk's data buffer via byte_width(), which is 0 for boolean // (bit-packed), the same GH-45086 buffer-overflow pattern fixed elsewhere in this @@ -2124,6 +2132,16 @@ TYPED_TEST(TestFillNullBinary, FillBackwardChunkedArray) { // the same, and filling the chunk wrote past the end of the (near-)empty buffer. // The corruption/crash only reliably manifests once a chunk is large enough to // write past the buffer's small built-in padding, hence the large pad length here. +TEST_F(TestFillNullBoolean, FillNullSlicedArray) { + auto input = + this->array("[true, false, null, true, true, false, null, true]")->Slice(3, 5); + + this->AssertFillNullArray(FillNullForward, input, + this->array("[true, true, false, false, true]")); + this->AssertFillNullArray(FillNullBackward, input, + this->array("[true, true, false, true, true]")); +} + TEST_F(TestFillNullBoolean, FillNullForwardChunkedArray) { constexpr int64_t kPadLength = 4096; ASSERT_OK_AND_ASSIGN(auto null_pad, MakeArrayOfNull(boolean(), kPadLength)); From efb7d1ffbdf6dd7dedb553d90f6c38d5b758415a Mon Sep 17 00:00:00 2001 From: Abdul Azeem Makarim <114302821+A-makarim@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:55:25 +0100 Subject: [PATCH 2/2] GH-51223: Move chunked fill regression comment --- .../arrow/compute/kernels/vector_replace_test.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/vector_replace_test.cc b/cpp/src/arrow/compute/kernels/vector_replace_test.cc index 0f248d2b1703..65cb9126b15f 100644 --- a/cpp/src/arrow/compute/kernels/vector_replace_test.cc +++ b/cpp/src/arrow/compute/kernels/vector_replace_test.cc @@ -2125,13 +2125,6 @@ TYPED_TEST(TestFillNullBinary, FillBackwardChunkedArray) { R"(["qup"])", R"(["qup", "mnz"])"})); } -// Regression test for GH-45086: FillNullForwardChunked/FillNullBackwardChunked -// size each output chunk's data buffer as `type->byte_width() * chunk->length()`. -// For BooleanType, byte_width() returns 0 (it is bit-packed, not byte-addressable), -// so the buffer was allocated with 0 bytes while the chunk's declared length stayed -// the same, and filling the chunk wrote past the end of the (near-)empty buffer. -// The corruption/crash only reliably manifests once a chunk is large enough to -// write past the buffer's small built-in padding, hence the large pad length here. TEST_F(TestFillNullBoolean, FillNullSlicedArray) { auto input = this->array("[true, false, null, true, true, false, null, true]")->Slice(3, 5); @@ -2142,6 +2135,13 @@ TEST_F(TestFillNullBoolean, FillNullSlicedArray) { this->array("[true, true, false, true, true]")); } +// Regression test for GH-45086: FillNullForwardChunked/FillNullBackwardChunked +// size each output chunk's data buffer as `type->byte_width() * chunk->length()`. +// For BooleanType, byte_width() returns 0 (it is bit-packed, not byte-addressable), +// so the buffer was allocated with 0 bytes while the chunk's declared length stayed +// the same, and filling the chunk wrote past the end of the (near-)empty buffer. +// The corruption/crash only reliably manifests once a chunk is large enough to +// write past the buffer's small built-in padding, hence the large pad length here. TEST_F(TestFillNullBoolean, FillNullForwardChunkedArray) { constexpr int64_t kPadLength = 4096; ASSERT_OK_AND_ASSIGN(auto null_pad, MakeArrayOfNull(boolean(), kPadLength));