pitrou commented on code in PR #49132:
URL: https://github.com/apache/arrow/pull/49132#discussion_r2817892118
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -997,6 +998,138 @@ TEST_F(TestArray, TestAppendArraySlice) {
}
}
+class TestBuilderAppendArraySlice : public TestArray {
+ public:
+ virtual void AssertResult(const Array& expected, const Array& actual) {
+ AssertArraysEqual(expected, actual, true);
+ }
+
+ void CheckAppendArraySlice(const std::shared_ptr<DataType>& 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<ArrayBuilder> builder;
+ ASSERT_OK(MakeBuilder(pool_, type, &builder));
+
+ // Slice the array into multiple pieces
+ ArrayVector slices;
+ std::vector<int64_t> 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<Array> actual;
+ ASSERT_OK(builder->Finish(&actual));
+ ASSERT_OK(actual->ValidateFull());
+
+ ASSERT_OK_AND_ASSIGN(auto expected, Concatenate(slices, pool_));
+ AssertResult(*expected, *actual);
+ }
+ }
+};
+
+template <typename Type>
+class TestBuilderAppendArraySliceTyped : public TestBuilderAppendArraySlice {};
+
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceTyped, PrimitiveArrowTypes);
+TYPED_TEST(TestBuilderAppendArraySliceTyped, Primitives) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::type_singleton());
+}
+
+template <typename Type>
+class TestBuilderAppendArraySliceBinary : public TestBuilderAppendArraySlice
{};
+
+using BinaryAppendArraySliceTypes =
+ ::testing::Types<BinaryType, LargeBinaryType, StringType, LargeStringType>;
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceBinary,
BinaryAppendArraySliceTypes);
+TYPED_TEST(TestBuilderAppendArraySliceBinary, Binary) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::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 {
Review Comment:
Can you add a comment explaining why this is needed?
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -997,6 +998,138 @@ TEST_F(TestArray, TestAppendArraySlice) {
}
}
+class TestBuilderAppendArraySlice : public TestArray {
+ public:
+ virtual void AssertResult(const Array& expected, const Array& actual) {
+ AssertArraysEqual(expected, actual, true);
+ }
+
+ void CheckAppendArraySlice(const std::shared_ptr<DataType>& 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<ArrayBuilder> builder;
+ ASSERT_OK(MakeBuilder(pool_, type, &builder));
+
+ // Slice the array into multiple pieces
+ ArrayVector slices;
+ std::vector<int64_t> 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<Array> actual;
+ ASSERT_OK(builder->Finish(&actual));
+ ASSERT_OK(actual->ValidateFull());
+
+ ASSERT_OK_AND_ASSIGN(auto expected, Concatenate(slices, pool_));
+ AssertResult(*expected, *actual);
+ }
+ }
+};
+
+template <typename Type>
+class TestBuilderAppendArraySliceTyped : public TestBuilderAppendArraySlice {};
+
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceTyped, PrimitiveArrowTypes);
+TYPED_TEST(TestBuilderAppendArraySliceTyped, Primitives) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::type_singleton());
+}
+
+template <typename Type>
+class TestBuilderAppendArraySliceBinary : public TestBuilderAppendArraySlice
{};
+
+using BinaryAppendArraySliceTypes =
+ ::testing::Types<BinaryType, LargeBinaryType, StringType, LargeStringType>;
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceBinary,
BinaryAppendArraySliceTypes);
+TYPED_TEST(TestBuilderAppendArraySliceBinary, Binary) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::type_singleton());
+}
Review Comment:
We don't actually need templating, we can just generate the types
dynamically using `PrimitiveTypes()`, `BaseBinaryTypes()` etc. This will reduce
code generation and compile times slightly.
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -997,6 +998,138 @@ TEST_F(TestArray, TestAppendArraySlice) {
}
}
+class TestBuilderAppendArraySlice : public TestArray {
+ public:
+ virtual void AssertResult(const Array& expected, const Array& actual) {
+ AssertArraysEqual(expected, actual, true);
+ }
+
+ void CheckAppendArraySlice(const std::shared_ptr<DataType>& 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<ArrayBuilder> builder;
+ ASSERT_OK(MakeBuilder(pool_, type, &builder));
+
+ // Slice the array into multiple pieces
+ ArrayVector slices;
+ std::vector<int64_t> 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<Array> actual;
+ ASSERT_OK(builder->Finish(&actual));
+ ASSERT_OK(actual->ValidateFull());
+
+ ASSERT_OK_AND_ASSIGN(auto expected, Concatenate(slices, pool_));
+ AssertResult(*expected, *actual);
+ }
+ }
+};
+
+template <typename Type>
+class TestBuilderAppendArraySliceTyped : public TestBuilderAppendArraySlice {};
+
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceTyped, PrimitiveArrowTypes);
+TYPED_TEST(TestBuilderAppendArraySliceTyped, Primitives) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::type_singleton());
+}
+
+template <typename Type>
+class TestBuilderAppendArraySliceBinary : public TestBuilderAppendArraySlice
{};
+
+using BinaryAppendArraySliceTypes =
+ ::testing::Types<BinaryType, LargeBinaryType, StringType, LargeStringType>;
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceBinary,
BinaryAppendArraySliceTypes);
+TYPED_TEST(TestBuilderAppendArraySliceBinary, Binary) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::type_singleton());
+}
Review Comment:
Besides, we should also test binary_view and string_view types.
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -997,6 +998,138 @@ TEST_F(TestArray, TestAppendArraySlice) {
}
}
+class TestBuilderAppendArraySlice : public TestArray {
+ public:
+ virtual void AssertResult(const Array& expected, const Array& actual) {
+ AssertArraysEqual(expected, actual, true);
+ }
+
+ void CheckAppendArraySlice(const std::shared_ptr<DataType>& 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<ArrayBuilder> builder;
+ ASSERT_OK(MakeBuilder(pool_, type, &builder));
+
+ // Slice the array into multiple pieces
+ ArrayVector slices;
+ std::vector<int64_t> 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<Array> actual;
+ ASSERT_OK(builder->Finish(&actual));
+ ASSERT_OK(actual->ValidateFull());
+
+ ASSERT_OK_AND_ASSIGN(auto expected, Concatenate(slices, pool_));
+ AssertResult(*expected, *actual);
+ }
+ }
+};
+
+template <typename Type>
+class TestBuilderAppendArraySliceTyped : public TestBuilderAppendArraySlice {};
+
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceTyped, PrimitiveArrowTypes);
+TYPED_TEST(TestBuilderAppendArraySliceTyped, Primitives) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::type_singleton());
+}
+
+template <typename Type>
+class TestBuilderAppendArraySliceBinary : public TestBuilderAppendArraySlice
{};
+
+using BinaryAppendArraySliceTypes =
+ ::testing::Types<BinaryType, LargeBinaryType, StringType, LargeStringType>;
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceBinary,
BinaryAppendArraySliceTypes);
+TYPED_TEST(TestBuilderAppendArraySliceBinary, Binary) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::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) {
Review Comment:
Can we also test decimal32 and decimal64, or are they not implemented yet?
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -997,6 +998,138 @@ TEST_F(TestArray, TestAppendArraySlice) {
}
}
+class TestBuilderAppendArraySlice : public TestArray {
+ public:
+ virtual void AssertResult(const Array& expected, const Array& actual) {
+ AssertArraysEqual(expected, actual, true);
+ }
+
+ void CheckAppendArraySlice(const std::shared_ptr<DataType>& 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<ArrayBuilder> builder;
+ ASSERT_OK(MakeBuilder(pool_, type, &builder));
+
+ // Slice the array into multiple pieces
+ ArrayVector slices;
+ std::vector<int64_t> 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<Array> actual;
+ ASSERT_OK(builder->Finish(&actual));
+ ASSERT_OK(actual->ValidateFull());
+
+ ASSERT_OK_AND_ASSIGN(auto expected, Concatenate(slices, pool_));
+ AssertResult(*expected, *actual);
+ }
+ }
+};
+
+template <typename Type>
+class TestBuilderAppendArraySliceTyped : public TestBuilderAppendArraySlice {};
+
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceTyped, PrimitiveArrowTypes);
+TYPED_TEST(TestBuilderAppendArraySliceTyped, Primitives) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::type_singleton());
+}
+
+template <typename Type>
+class TestBuilderAppendArraySliceBinary : public TestBuilderAppendArraySlice
{};
+
+using BinaryAppendArraySliceTypes =
+ ::testing::Types<BinaryType, LargeBinaryType, StringType, LargeStringType>;
+TYPED_TEST_SUITE(TestBuilderAppendArraySliceBinary,
BinaryAppendArraySliceTypes);
+TYPED_TEST(TestBuilderAppendArraySliceBinary, Binary) {
+ this->CheckAppendArraySlice(TypeTraits<TypeParam>::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()));
Review Comment:
Let's use a more interesting value type that's not the same as the offset
type?
--
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]
