Skip to content

Commit

Permalink
apacheGH-39976: [C++] Fix out-of-line data size calculation in Binary…
Browse files Browse the repository at this point in the history
…ViewBuilder::AppendArraySlice (apache#39994)

### Rationale for this change

Fix the bug in `BinaryViewBuilder::AppendArraySlice` that, when calculating out-of-line data size, the array is wrongly iterated.

### What changes are included in this PR?

Fix and UT.

### Are these changes tested?

UT included.

### Are there any user-facing changes?

No.

* Closes: apache#39976

Authored-by: Ruoxi Sun <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
  • Loading branch information
zanmato1984 authored and thisisnic committed Mar 8, 2024
1 parent 6eec17f commit 8a5ed96
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
23 changes: 23 additions & 0 deletions cpp/src/arrow/array/array_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,29 @@ TEST_F(TestArray, TestAppendArraySlice) {
}
}

// 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);
}

TEST_F(TestArray, ValidateBuffersPrimitive) {
auto empty_buffer = std::make_shared<Buffer>("");
auto null_buffer = Buffer::FromString("\xff");
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/array/builder_binary.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Status BinaryViewBuilder::AppendArraySlice(const ArraySpan& array, int64_t offse

int64_t out_of_line_total = 0, i = 0;
VisitNullBitmapInline(
array.buffers[0].data, array.offset, array.length, array.null_count,
array.buffers[0].data, array.offset + offset, length, array.null_count,
[&] {
if (!values[i].is_inline()) {
out_of_line_total += static_cast<int64_t>(values[i].size());
Expand Down

0 comments on commit 8a5ed96

Please sign in to comment.