From 846af7e068e3766004532e52a05319a8989ebe69 Mon Sep 17 00:00:00 2001 From: Abhishek Bansal Date: Wed, 4 Feb 2026 00:05:44 +0530 Subject: [PATCH 1/5] GH-38184: [C++] Add systematic tests for Builder::AppendArraySlice --- cpp/src/arrow/array/array_test.cc | 134 ++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index dc82488f6a3..701a72d7d3b 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -35,6 +35,7 @@ #include "arrow/array/array_binary.h" #include "arrow/array/array_decimal.h" #include "arrow/array/array_primitive.h" +#include "arrow/array/concatenate.h" #include "arrow/array/builder_binary.h" #include "arrow/array/builder_decimal.h" #include "arrow/array/builder_dict.h" @@ -996,6 +997,139 @@ TEST_F(TestArray, TestAppendArraySlice) { ASSERT_EQ(8, result->null_count()); } } + +class TestBuilderAppendArraySlice : public TestArray { + public: + virtual void AssertResult(const Array& expected, const Array& actual) { + AssertArraysEqual(expected, actual, true); + } + + void CheckAppendArraySlice(const std::shared_ptr& type) { + auto rag = random::RandomArrayGenerator(0xdeadbeef); + const int64_t total_length = 100; + + for (auto null_probability : {0.0, 0.1, 0.5, 1.0}) { + auto array = rag.ArrayOf(type, total_length, null_probability); + + std::unique_ptr builder; + ASSERT_OK(MakeBuilder(pool_, type, &builder)); + + // Slice the array into multiple pieces + ArrayVector slices; + std::vector offsets = {0, 10, 10, 25, 60, 100}; + for (size_t i = 0; i < offsets.size() - 1; ++i) { + int64_t start = offsets[i]; + int64_t length = offsets[i + 1] - offsets[i]; + auto slice = array->Slice(start, length); + slices.push_back(slice); + + ArraySpan span(*slice->data()); + ASSERT_OK(builder->AppendArraySlice(span, 0, slice->length())); + } + + std::shared_ptr actual; + ASSERT_OK(builder->Finish(&actual)); + ASSERT_OK(actual->ValidateFull()); + + ASSERT_OK_AND_ASSIGN(auto expected, Concatenate(slices, pool_)); + AssertResult(*expected, *actual); + } + } +}; + +template +class TestBuilderAppendArraySliceTyped : public TestBuilderAppendArraySlice {}; + +TYPED_TEST_SUITE(TestBuilderAppendArraySliceTyped, PrimitiveArrowTypes); +TYPED_TEST(TestBuilderAppendArraySliceTyped, Primitives) { + this->CheckAppendArraySlice(TypeTraits::type_singleton()); +} + +template +class TestBuilderAppendArraySliceBinary : public TestBuilderAppendArraySlice {}; + +using BinaryAppendArraySliceTypes = + ::testing::Types; +TYPED_TEST_SUITE(TestBuilderAppendArraySliceBinary, BinaryAppendArraySliceTypes); +TYPED_TEST(TestBuilderAppendArraySliceBinary, Binary) { + this->CheckAppendArraySlice(TypeTraits::type_singleton()); +} + +TEST_F(TestBuilderAppendArraySlice, List) { + CheckAppendArraySlice(list(int32())); +} + +TEST_F(TestBuilderAppendArraySlice, LargeList) { + CheckAppendArraySlice(large_list(int32())); +} + +TEST_F(TestBuilderAppendArraySlice, ListView) { + CheckAppendArraySlice(list_view(int32())); +} + +TEST_F(TestBuilderAppendArraySlice, LargeListView) { + CheckAppendArraySlice(large_list_view(int32())); +} + +TEST_F(TestBuilderAppendArraySlice, FixedSizeBinary) { + CheckAppendArraySlice(fixed_size_binary(10)); +} + +TEST_F(TestBuilderAppendArraySlice, Decimal128) { + CheckAppendArraySlice(decimal128(10, 2)); +} + +TEST_F(TestBuilderAppendArraySlice, Decimal256) { + CheckAppendArraySlice(decimal256(10, 2)); +} + +TEST_F(TestBuilderAppendArraySlice, FixedSizeList) { + CheckAppendArraySlice(fixed_size_list(int32(), 3)); +} + +TEST_F(TestBuilderAppendArraySlice, Struct) { + CheckAppendArraySlice(struct_({field("a", int32()), field("b", utf8())})); +} + +TEST_F(TestBuilderAppendArraySlice, SparseUnion) { + CheckAppendArraySlice(sparse_union({field("a", int32()), field("b", utf8())})); +} + +TEST_F(TestBuilderAppendArraySlice, DenseUnion) { + CheckAppendArraySlice(dense_union({field("a", int32()), field("b", utf8())})); +} + +TEST_F(TestBuilderAppendArraySlice, RunEndEncoded) { + CheckAppendArraySlice(run_end_encoded(int32(), int32())); +} + + +class TestBuilderAppendArraySliceDictionary : public TestBuilderAppendArraySlice { + public: + void AssertResult(const Array& expected, const Array& actual) override { + const auto& expected_dict = internal::checked_cast(expected); + const auto& actual_dict = internal::checked_cast(actual); + const auto& expected_values = *expected_dict.dictionary(); + const auto& actual_values = *actual_dict.dictionary(); + + ASSERT_EQ(expected.length(), actual.length()); + for (int64_t i = 0; i < expected.length(); ++i) { + if (expected.IsNull(i)) { + ASSERT_TRUE(actual.IsNull(i)); + } else { + ASSERT_FALSE(actual.IsNull(i)); + ASSERT_OK_AND_ASSIGN(auto expected_val, expected_values.GetScalar(expected_dict.GetValueIndex(i))); + ASSERT_OK_AND_ASSIGN(auto actual_val, actual_values.GetScalar(actual_dict.GetValueIndex(i))); + AssertScalarsEqual(*expected_val, *actual_val); + } + } + } +}; + +TEST_F(TestBuilderAppendArraySliceDictionary, Dictionary) { + CheckAppendArraySlice(dictionary(int8(), utf8())); + CheckAppendArraySlice(dictionary(int32(), utf8())); +} // GH-39976: Test out-of-line data size calculation in // BinaryViewBuilder::AppendArraySlice. From 8b8176a7e6ad44acb3a0cf7d4d277deb53ec2c21 Mon Sep 17 00:00:00 2001 From: Abhishek Bansal Date: Wed, 4 Feb 2026 00:39:09 +0530 Subject: [PATCH 2/5] GH-38184: [C++] Add systematic tests for Builder::AppendArraySlice --- cpp/src/arrow/array/array_test.cc | 57 +++++++++++++++---------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 701a72d7d3b..4391c3c3b68 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -35,13 +35,13 @@ #include "arrow/array/array_binary.h" #include "arrow/array/array_decimal.h" #include "arrow/array/array_primitive.h" -#include "arrow/array/concatenate.h" #include "arrow/array/builder_binary.h" #include "arrow/array/builder_decimal.h" #include "arrow/array/builder_dict.h" #include "arrow/array/builder_primitive.h" #include "arrow/array/builder_run_end.h" #include "arrow/array/builder_time.h" +#include "arrow/array/concatenate.h" #include "arrow/array/data.h" #include "arrow/array/util.h" #include "arrow/buffer.h" @@ -997,23 +997,23 @@ TEST_F(TestArray, TestAppendArraySlice) { ASSERT_EQ(8, result->null_count()); } } - + class TestBuilderAppendArraySlice : public TestArray { public: virtual void AssertResult(const Array& expected, const Array& actual) { AssertArraysEqual(expected, actual, true); } - + void CheckAppendArraySlice(const std::shared_ptr& type) { auto rag = random::RandomArrayGenerator(0xdeadbeef); const int64_t total_length = 100; - + for (auto null_probability : {0.0, 0.1, 0.5, 1.0}) { auto array = rag.ArrayOf(type, total_length, null_probability); - + std::unique_ptr builder; ASSERT_OK(MakeBuilder(pool_, type, &builder)); - + // Slice the array into multiple pieces ArrayVector slices; std::vector offsets = {0, 10, 10, 25, 60, 100}; @@ -1022,45 +1022,43 @@ class TestBuilderAppendArraySlice : public TestArray { int64_t length = offsets[i + 1] - offsets[i]; auto slice = array->Slice(start, length); slices.push_back(slice); - + ArraySpan span(*slice->data()); ASSERT_OK(builder->AppendArraySlice(span, 0, slice->length())); } - + std::shared_ptr actual; ASSERT_OK(builder->Finish(&actual)); ASSERT_OK(actual->ValidateFull()); - + ASSERT_OK_AND_ASSIGN(auto expected, Concatenate(slices, pool_)); AssertResult(*expected, *actual); } } }; - + template class TestBuilderAppendArraySliceTyped : public TestBuilderAppendArraySlice {}; - + TYPED_TEST_SUITE(TestBuilderAppendArraySliceTyped, PrimitiveArrowTypes); TYPED_TEST(TestBuilderAppendArraySliceTyped, Primitives) { this->CheckAppendArraySlice(TypeTraits::type_singleton()); } - + template class TestBuilderAppendArraySliceBinary : public TestBuilderAppendArraySlice {}; - + using BinaryAppendArraySliceTypes = ::testing::Types; TYPED_TEST_SUITE(TestBuilderAppendArraySliceBinary, BinaryAppendArraySliceTypes); TYPED_TEST(TestBuilderAppendArraySliceBinary, Binary) { this->CheckAppendArraySlice(TypeTraits::type_singleton()); } - -TEST_F(TestBuilderAppendArraySlice, List) { - CheckAppendArraySlice(list(int32())); -} -TEST_F(TestBuilderAppendArraySlice, LargeList) { - CheckAppendArraySlice(large_list(int32())); +TEST_F(TestBuilderAppendArraySlice, List) { CheckAppendArraySlice(list(int32())); } + +TEST_F(TestBuilderAppendArraySlice, LargeList) { + CheckAppendArraySlice(large_list(int32())); } TEST_F(TestBuilderAppendArraySlice, ListView) { @@ -1070,17 +1068,17 @@ TEST_F(TestBuilderAppendArraySlice, ListView) { TEST_F(TestBuilderAppendArraySlice, LargeListView) { CheckAppendArraySlice(large_list_view(int32())); } - + TEST_F(TestBuilderAppendArraySlice, FixedSizeBinary) { CheckAppendArraySlice(fixed_size_binary(10)); } -TEST_F(TestBuilderAppendArraySlice, Decimal128) { - CheckAppendArraySlice(decimal128(10, 2)); +TEST_F(TestBuilderAppendArraySlice, Decimal128) { + CheckAppendArraySlice(decimal128(10, 2)); } -TEST_F(TestBuilderAppendArraySlice, Decimal256) { - CheckAppendArraySlice(decimal256(10, 2)); +TEST_F(TestBuilderAppendArraySlice, Decimal256) { + CheckAppendArraySlice(decimal256(10, 2)); } TEST_F(TestBuilderAppendArraySlice, FixedSizeList) { @@ -1103,7 +1101,6 @@ TEST_F(TestBuilderAppendArraySlice, RunEndEncoded) { CheckAppendArraySlice(run_end_encoded(int32(), int32())); } - class TestBuilderAppendArraySliceDictionary : public TestBuilderAppendArraySlice { public: void AssertResult(const Array& expected, const Array& actual) override { @@ -1111,21 +1108,23 @@ class TestBuilderAppendArraySliceDictionary : public TestBuilderAppendArraySlice const auto& actual_dict = internal::checked_cast(actual); const auto& expected_values = *expected_dict.dictionary(); const auto& actual_values = *actual_dict.dictionary(); - + ASSERT_EQ(expected.length(), actual.length()); for (int64_t i = 0; i < expected.length(); ++i) { if (expected.IsNull(i)) { ASSERT_TRUE(actual.IsNull(i)); } else { ASSERT_FALSE(actual.IsNull(i)); - ASSERT_OK_AND_ASSIGN(auto expected_val, expected_values.GetScalar(expected_dict.GetValueIndex(i))); - ASSERT_OK_AND_ASSIGN(auto actual_val, actual_values.GetScalar(actual_dict.GetValueIndex(i))); + ASSERT_OK_AND_ASSIGN(auto expected_val, + expected_values.GetScalar(expected_dict.GetValueIndex(i))); + ASSERT_OK_AND_ASSIGN(auto actual_val, + actual_values.GetScalar(actual_dict.GetValueIndex(i))); AssertScalarsEqual(*expected_val, *actual_val); } } } }; - + TEST_F(TestBuilderAppendArraySliceDictionary, Dictionary) { CheckAppendArraySlice(dictionary(int8(), utf8())); CheckAppendArraySlice(dictionary(int32(), utf8())); From 189b69b6d492d273923c3186b5ea37a36278b627 Mon Sep 17 00:00:00 2001 From: Abhishek Bansal Date: Sat, 7 Feb 2026 20:28:42 +0530 Subject: [PATCH 3/5] chore: trigger CI From e87de7f8a658d497876935bf449f303e2002889e Mon Sep 17 00:00:00 2001 From: Abhishek Bansal Date: Wed, 18 Feb 2026 00:55:02 +0530 Subject: [PATCH 4/5] GH-38184: [C++] Refactor AppendArraySlice tests --- cpp/src/arrow/array/array_test.cc | 45 ++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 4391c3c3b68..4492c64b2a0 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -1037,24 +1037,24 @@ class TestBuilderAppendArraySlice : public TestArray { } }; -template -class TestBuilderAppendArraySliceTyped : public TestBuilderAppendArraySlice {}; +TEST_F(TestBuilderAppendArraySlice, DynamicTypes) { + auto types = PrimitiveTypes(); + auto temporal_types = TemporalTypes(); + auto interval_types = IntervalTypes(); + auto duration_types = DurationTypes(); -TYPED_TEST_SUITE(TestBuilderAppendArraySliceTyped, PrimitiveArrowTypes); -TYPED_TEST(TestBuilderAppendArraySliceTyped, Primitives) { - this->CheckAppendArraySlice(TypeTraits::type_singleton()); -} - -template -class TestBuilderAppendArraySliceBinary : public TestBuilderAppendArraySlice {}; + types.insert(types.end(), temporal_types.begin(), temporal_types.end()); + types.insert(types.end(), interval_types.begin(), interval_types.end()); + types.insert(types.end(), duration_types.begin(), duration_types.end()); -using BinaryAppendArraySliceTypes = - ::testing::Types; -TYPED_TEST_SUITE(TestBuilderAppendArraySliceBinary, BinaryAppendArraySliceTypes); -TYPED_TEST(TestBuilderAppendArraySliceBinary, Binary) { - this->CheckAppendArraySlice(TypeTraits::type_singleton()); + for (const auto& type : types) { + ARROW_SCOPED_TRACE("type = ", type->ToString()); + this->CheckAppendArraySlice(type); + } } +TEST_F(TestBuilderAppendArraySlice, Float16) { CheckAppendArraySlice(float16()); } + TEST_F(TestBuilderAppendArraySlice, List) { CheckAppendArraySlice(list(int32())); } TEST_F(TestBuilderAppendArraySlice, LargeList) { @@ -1073,6 +1073,12 @@ TEST_F(TestBuilderAppendArraySlice, FixedSizeBinary) { CheckAppendArraySlice(fixed_size_binary(10)); } +TEST_F(TestBuilderAppendArraySlice, Decimal32) { CheckAppendArraySlice(decimal32(7, 2)); } + +TEST_F(TestBuilderAppendArraySlice, Decimal64) { + CheckAppendArraySlice(decimal64(12, 2)); +} + TEST_F(TestBuilderAppendArraySlice, Decimal128) { CheckAppendArraySlice(decimal128(10, 2)); } @@ -1098,9 +1104,18 @@ TEST_F(TestBuilderAppendArraySlice, DenseUnion) { } TEST_F(TestBuilderAppendArraySlice, RunEndEncoded) { - CheckAppendArraySlice(run_end_encoded(int32(), int32())); + CheckAppendArraySlice(run_end_encoded(int32(), utf8())); + CheckAppendArraySlice(run_end_encoded(int32(), int64())); } +// Dictionary types require a custom AssertResult because DictionaryBuilder +// re-encodes values based on discovery order. This can change both the +// dictionary and the indices, causing standard physical equality checks to fail. +// +// Example: Slicing values ["b", "a"] from an array with dictionary ["a", "b"] +// (indices [1, 0]) and appending them to a fresh builder results in a new +// dictionary ["b", "a"] (indices [0, 1]). Both represent the same logical +// data but differ physically. class TestBuilderAppendArraySliceDictionary : public TestBuilderAppendArraySlice { public: void AssertResult(const Array& expected, const Array& actual) override { From 06f00c7627c34c80784ff0e8103ced69fadda159 Mon Sep 17 00:00:00 2001 From: Abhishek Bansal Date: Wed, 18 Feb 2026 14:26:05 +0530 Subject: [PATCH 5/5] GH-38184: [C++] Refactor AppendArraySlice tests --- cpp/src/arrow/array/array_test.cc | 74 +++++++++---------------------- 1 file changed, 22 insertions(+), 52 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 4492c64b2a0..64ea3fd71a7 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -1035,73 +1035,43 @@ class TestBuilderAppendArraySlice : public TestArray { AssertResult(*expected, *actual); } } -}; - -TEST_F(TestBuilderAppendArraySlice, DynamicTypes) { - auto types = PrimitiveTypes(); - auto temporal_types = TemporalTypes(); - auto interval_types = IntervalTypes(); - auto duration_types = DurationTypes(); - types.insert(types.end(), temporal_types.begin(), temporal_types.end()); - types.insert(types.end(), interval_types.begin(), interval_types.end()); - types.insert(types.end(), duration_types.begin(), duration_types.end()); - - for (const auto& type : types) { - ARROW_SCOPED_TRACE("type = ", type->ToString()); - this->CheckAppendArraySlice(type); + void CheckAppendArraySlice(const std::vector>& types) { + for (const auto& type : types) { + ARROW_SCOPED_TRACE("type = ", type->ToString()); + CheckAppendArraySlice(type); + } } +}; + +TEST_F(TestBuilderAppendArraySlice, Primitives) { + CheckAppendArraySlice(PrimitiveTypes()); } -TEST_F(TestBuilderAppendArraySlice, Float16) { CheckAppendArraySlice(float16()); } +TEST_F(TestBuilderAppendArraySlice, Temporals) { CheckAppendArraySlice(TemporalTypes()); } -TEST_F(TestBuilderAppendArraySlice, List) { CheckAppendArraySlice(list(int32())); } +TEST_F(TestBuilderAppendArraySlice, Intervals) { CheckAppendArraySlice(IntervalTypes()); } -TEST_F(TestBuilderAppendArraySlice, LargeList) { - CheckAppendArraySlice(large_list(int32())); -} +TEST_F(TestBuilderAppendArraySlice, Durations) { CheckAppendArraySlice(DurationTypes()); } -TEST_F(TestBuilderAppendArraySlice, ListView) { - CheckAppendArraySlice(list_view(int32())); +TEST_F(TestBuilderAppendArraySlice, Decimals) { + CheckAppendArraySlice( + {decimal32(7, 2), decimal64(12, 2), decimal128(10, 2), decimal256(10, 2)}); } -TEST_F(TestBuilderAppendArraySlice, LargeListView) { - CheckAppendArraySlice(large_list_view(int32())); +TEST_F(TestBuilderAppendArraySlice, Nested) { + CheckAppendArraySlice({list(int32()), large_list(int32()), list_view(int32()), + large_list_view(int32()), fixed_size_list(int32(), 3), + struct_({field("a", int32()), field("b", utf8())}), + sparse_union({field("a", int32()), field("b", utf8())}), + dense_union({field("a", int32()), field("b", utf8())})}); } TEST_F(TestBuilderAppendArraySlice, FixedSizeBinary) { CheckAppendArraySlice(fixed_size_binary(10)); } -TEST_F(TestBuilderAppendArraySlice, Decimal32) { CheckAppendArraySlice(decimal32(7, 2)); } - -TEST_F(TestBuilderAppendArraySlice, Decimal64) { - CheckAppendArraySlice(decimal64(12, 2)); -} - -TEST_F(TestBuilderAppendArraySlice, Decimal128) { - CheckAppendArraySlice(decimal128(10, 2)); -} - -TEST_F(TestBuilderAppendArraySlice, Decimal256) { - CheckAppendArraySlice(decimal256(10, 2)); -} - -TEST_F(TestBuilderAppendArraySlice, FixedSizeList) { - CheckAppendArraySlice(fixed_size_list(int32(), 3)); -} - -TEST_F(TestBuilderAppendArraySlice, Struct) { - CheckAppendArraySlice(struct_({field("a", int32()), field("b", utf8())})); -} - -TEST_F(TestBuilderAppendArraySlice, SparseUnion) { - CheckAppendArraySlice(sparse_union({field("a", int32()), field("b", utf8())})); -} - -TEST_F(TestBuilderAppendArraySlice, DenseUnion) { - CheckAppendArraySlice(dense_union({field("a", int32()), field("b", utf8())})); -} +TEST_F(TestBuilderAppendArraySlice, Float16) { CheckAppendArraySlice(float16()); } TEST_F(TestBuilderAppendArraySlice, RunEndEncoded) { CheckAppendArraySlice(run_end_encoded(int32(), utf8()));