Skip to content

Commit 99140d7

Browse files
committed
fix naming, static_cast and forgotten assertion
1 parent 917c2a1 commit 99140d7

File tree

3 files changed

+16
-12
lines changed

3 files changed

+16
-12
lines changed

cpp/src/arrow/array/array_test.cc

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ TEST_F(TestArray, TestAppendArraySlice) {
977977
ASSERT_EQ(8, result->null_count());
978978
}
979979
}
980-
class TestBinaryViewBuilderAppendArraySlice : public TestArray {
980+
class TestArrayBinaryViewBuilderAppendArraySlice : public TestArray {
981981
public:
982982
struct StringOptions {
983983
int64_t size = 0;
@@ -987,7 +987,7 @@ class TestBinaryViewBuilderAppendArraySlice : public TestArray {
987987
double null_probability = 0.0;
988988
};
989989

990-
TestBinaryViewBuilderAppendArraySlice() : TestArray(), generator_(12) {}
990+
TestArrayBinaryViewBuilderAppendArraySlice() : TestArray(), generator_(12) {}
991991

992992
void SetUp() override {
993993
TestArray::SetUp();
@@ -1004,15 +1004,15 @@ class TestBinaryViewBuilderAppendArraySlice : public TestArray {
10041004
Result<std::shared_ptr<Array>> AppendArraySliceWithSpanOffset(
10051005
const std::shared_ptr<Array>& array) {
10061006
const ArraySpan span(*array->data());
1007-
for (int i = 0; i < array->length(); i++) {
1007+
for (int i = 0; i < array->length(); ++i) {
10081008
ARROW_RETURN_NOT_OK(builder_.AppendArraySlice(span, i, 1));
10091009
}
10101010
return builder_.Finish();
10111011
}
10121012

10131013
Result<std::shared_ptr<Array>> AppendArraySliceWithArrayOffset(
10141014
const std::shared_ptr<Array>& array) {
1015-
for (int i = 0; i < array->length(); i++) {
1015+
for (int i = 0; i < array->length(); ++i) {
10161016
auto slice = array->Slice(i, 1);
10171017
const ArraySpan span(*slice->data());
10181018
ARROW_RETURN_NOT_OK(builder_.AppendArraySlice(span, 0, 1));
@@ -1097,7 +1097,7 @@ class TestBinaryViewBuilderAppendArraySlice : public TestArray {
10971097
random::RandomArrayGenerator generator_;
10981098
};
10991099

1100-
TEST_F(TestBinaryViewBuilderAppendArraySlice, Inline) {
1100+
TEST_F(TestArrayBinaryViewBuilderAppendArraySlice, Inline) {
11011101
StringOptions options{};
11021102
options.size = 16;
11031103
options.min_length = 0;
@@ -1120,7 +1120,7 @@ TEST_F(TestBinaryViewBuilderAppendArraySlice, Inline) {
11201120
}
11211121
}
11221122

1123-
TEST_F(TestBinaryViewBuilderAppendArraySlice, NonInline) {
1123+
TEST_F(TestArrayBinaryViewBuilderAppendArraySlice, NonInline) {
11241124
StringOptions options{};
11251125
options.size = 200;
11261126
options.min_length = 13;
@@ -1144,7 +1144,7 @@ TEST_F(TestBinaryViewBuilderAppendArraySlice, NonInline) {
11441144
}
11451145
}
11461146

1147-
TEST_F(TestBinaryViewBuilderAppendArraySlice, NonInlineAndInline) {
1147+
TEST_F(TestArrayBinaryViewBuilderAppendArraySlice, NonInlineAndInline) {
11481148
StringOptions options{};
11491149
options.size = 200;
11501150
options.min_length = 0;
@@ -1168,7 +1168,7 @@ TEST_F(TestBinaryViewBuilderAppendArraySlice, NonInlineAndInline) {
11681168
}
11691169
}
11701170

1171-
TEST_F(TestBinaryViewBuilderAppendArraySlice, NonInlineAndInlineAndAppend) {
1171+
TEST_F(TestArrayBinaryViewBuilderAppendArraySlice, NonInlineAndInlineAndAppend) {
11721172
StringOptions options{};
11731173
options.size = 200;
11741174
options.min_length = 0;
@@ -1213,7 +1213,7 @@ TEST_F(TestBinaryViewBuilderAppendArraySlice, NonInlineAndInlineAndAppend) {
12131213
}
12141214

12151215
// Check the state of arrow::internal::StringHeapBuilder::current_block_
1216-
TEST_F(TestBinaryViewBuilderAppendArraySlice, Reset) {
1216+
TEST_F(TestArrayBinaryViewBuilderAppendArraySlice, Reset) {
12171217
StringOptions options{};
12181218
options.size = 200;
12191219
options.min_length = 0;
@@ -1263,6 +1263,10 @@ TEST_F(TestBinaryViewBuilderAppendArraySlice, Reset) {
12631263
view_1 = view_buffer[0];
12641264
view_2 = view_buffer[1];
12651265
view_3 = view_buffer[2];
1266+
1267+
ASSERT_EQ(view_1.ref.buffer_index, 0);
1268+
ASSERT_EQ(view_2.ref.buffer_index, 0);
1269+
ASSERT_EQ(view_3.ref.buffer_index, 1);
12661270
}
12671271

12681272
TEST_F(TestArray, ValidateBuffersPrimitive) {

cpp/src/arrow/array/builder_binary.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ Status BinaryViewBuilder::AppendArraySlice(const ArraySpan& array, int64_t offse
8181
if (view.is_inline()) {
8282
data_builder_.UnsafeAppend(&view, 1);
8383
} else {
84-
auto dst_buffer_index = TryAddBufferAndGetIndex(
84+
auto dst_data_buffer_index = TryAddBufferAndGetIndex(
8585
buffer_index_map, view.ref.buffer_index, data_heap_builder_,
8686
data_buffers[view.ref.buffer_index]);
8787
auto dst_view_index = data_builder_.length();
8888
data_builder_.UnsafeAppend(&view, 1);
8989
data_builder_.mutable_data()[dst_view_index].ref.buffer_index =
90-
dst_buffer_index;
90+
dst_data_buffer_index;
9191
}
9292
},
9393
[&]() { UnsafeAppendNull(); });

cpp/src/arrow/array/builder_binary.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ class ARROW_EXPORT StringHeapBuilder {
533533
current_offset_ = 0;
534534
current_out_buffer_ = new_block->mutable_data();
535535
blocks_.emplace_back(std::move(new_block));
536-
current_block_ = static_cast<int32_t>(blocks_.size() - 1);
536+
current_block_ = static_cast<int64_t>(blocks_.size() - 1);
537537
}
538538
return Status::OK();
539539
}

0 commit comments

Comments
 (0)