[C++] Add systematic tests for Builder::AppendArraySlice

[pitrou]

Describe the enhancement requested

Builder::AppendArraySlice is only tested on an ad-hoc basis currently, even though its implementation is non-trivial.

We can probably set up systematic tests against, for example, Concatenate, perhaps using random array generation.

Component(s)

C++

[MuteBardTison]

Hi! I’m interested in working on this issue. Since it was opened a while ago, I just wanted to check if it’s still relevant.
Thanks!

[kou]

The current tests:

arrow/cpp/src/arrow/array/array_test.cc

Lines 905 to 1001 in 34a984c

	TEST_F(TestArray, TestAppendArraySlice) {
	auto scalars = GetScalars();
	ArraySpan span;
	for (const auto& scalar : scalars) {
	ARROW_SCOPED_TRACE(*scalar->type);
	ASSERT_OK_AND_ASSIGN(auto array, MakeArrayFromScalar(*scalar, 16));
	ASSERT_OK_AND_ASSIGN(auto nulls, MakeArrayOfNull(scalar->type, 16));
	std::unique_ptr<arrow::ArrayBuilder> builder;
	ASSERT_OK(MakeBuilder(pool_, scalar->type, &builder));
	span.SetMembers(*array->data());
	ASSERT_OK(builder->AppendArraySlice(span, 0, 4));
	ASSERT_EQ(4, builder->length());
	ASSERT_OK(builder->AppendArraySlice(span, 0, 0));
	ASSERT_EQ(4, builder->length());
	ASSERT_OK(builder->AppendArraySlice(span, 1, 0));
	ASSERT_EQ(4, builder->length());
	ASSERT_OK(builder->AppendArraySlice(span, 1, 4));
	ASSERT_EQ(8, builder->length());
	span.SetMembers(*nulls->data());
	ASSERT_OK(builder->AppendArraySlice(span, 0, 4));
	ASSERT_EQ(12, builder->length());
	const bool has_validity_bitmap =
	internal::may_have_validity_bitmap(scalar->type->id());
	if (has_validity_bitmap) {
	ASSERT_EQ(4, builder->null_count());
	}
	ASSERT_OK(builder->AppendArraySlice(span, 0, 0));
	ASSERT_EQ(12, builder->length());
	if (has_validity_bitmap) {
	ASSERT_EQ(4, builder->null_count());
	}
	ASSERT_OK(builder->AppendArraySlice(span, 1, 0));
	ASSERT_EQ(12, builder->length());
	if (has_validity_bitmap) {
	ASSERT_EQ(4, builder->null_count());
	}
	ASSERT_OK(builder->AppendArraySlice(span, 1, 4));
	ASSERT_EQ(16, builder->length());
	if (has_validity_bitmap) {
	ASSERT_EQ(8, builder->null_count());
	}
	std::shared_ptr<Array> result;
	ASSERT_OK(builder->Finish(&result));
	ASSERT_OK(result->ValidateFull());
	ASSERT_EQ(16, result->length());
	if (has_validity_bitmap) {
	ASSERT_EQ(8, result->null_count());
	}
	}
	{
	ASSERT_OK_AND_ASSIGN(auto array, MakeArrayOfNull(null(), 16));
	NullBuilder builder(pool_);
	span.SetMembers(*array->data());
	ASSERT_OK(builder.AppendArraySlice(span, 0, 4));
	ASSERT_EQ(4, builder.length());
	ASSERT_OK(builder.AppendArraySlice(span, 0, 0));
	ASSERT_EQ(4, builder.length());
	ASSERT_OK(builder.AppendArraySlice(span, 1, 0));
	ASSERT_EQ(4, builder.length());
	ASSERT_OK(builder.AppendArraySlice(span, 1, 4));
	ASSERT_EQ(8, builder.length());
	std::shared_ptr<Array> result;
	ASSERT_OK(builder.Finish(&result));
	ASSERT_OK(result->ValidateFull());
	ASSERT_EQ(8, result->length());
	ASSERT_EQ(8, result->null_count());
	}
	}
	// GH-39976: Test out-of-line data size calculation in
	// BinaryViewBuilder::AppendArraySlice.
	TEST_F(TestArray, TestBinaryViewAppendArraySlice) {
	BinaryViewBuilder src_builder(pool_);
	ASSERT_OK(src_builder.AppendNull());
	ASSERT_OK(src_builder.Append("long string; not inlined"));
	ASSERT_EQ(2, src_builder.length());
	ASSERT_OK_AND_ASSIGN(auto src, src_builder.Finish());
	ASSERT_OK(src->ValidateFull());
	ArraySpan span;
	span.SetMembers(*src->data());
	BinaryViewBuilder dst_builder(pool_);
	ASSERT_OK(dst_builder.AppendArraySlice(span, 0, 1));
	ASSERT_EQ(1, dst_builder.length());
	ASSERT_OK(dst_builder.AppendArraySlice(span, 1, 1));
	ASSERT_EQ(2, dst_builder.length());
	ASSERT_OK_AND_ASSIGN(auto dst, dst_builder.Finish());
	ASSERT_OK(dst->ValidateFull());
	AssertArraysEqual(*src, *dst);
	}

The referenced Concatenate tests: https://github.com/apache/arrow/blob/main/cpp/src/arrow/array/concatenate_test.cc

[abhishek593]

take

[abhishek593]

@pitrou @kou Can you please review the PR? Thanks!