ARROW-1712: [C++] Modify test case for BinaryBuilder::ReserveData() a…

…nd change arguments for offsets_builder_.Resize()
apache · Jan 22, 2018 · d4bbd15 · d4bbd15
1 parent bc5db7d
commit d4bbd15
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 7 deletions.
diff --git a/cpp/src/arrow/array-test.cc b/cpp/src/arrow/array-test.cc
@@ -1162,7 +1162,6 @@ TEST_F(TestBinaryBuilder, TestCapacityReserve) {
   int64_t length = 0;
   int64_t capacity = 1000;
 
-
   ASSERT_OK(builder_->ReserveData(capacity));
 
   ASSERT_EQ(length, builder_->value_data_length());
@@ -1177,11 +1176,18 @@ TEST_F(TestBinaryBuilder, TestCapacityReserve) {
       ASSERT_EQ(BitUtil::RoundUpToMultipleOf64(capacity), builder_->value_data_capacity());
     }
   }
+
+  int extra_capacity = 500;
+  ASSERT_OK(builder_->ReserveData(extra_capacity));
+
+  ASSERT_EQ(length, builder_->value_data_length());
+  ASSERT_EQ(BitUtil::RoundUpToMultipleOf64(length + extra_capacity), builder_->value_data_capacity());
+
   Done();
   ASSERT_EQ(reps * N, result_->length());
   ASSERT_EQ(0, result_->null_count());
   ASSERT_EQ(reps * 60, result_->value_data()->size());
-  ASSERT_EQ(BitUtil::RoundUpToMultipleOf64(capacity), result_->value_data()->capacity());
+  ASSERT_EQ(BitUtil::RoundUpToMultipleOf64(length + extra_capacity), result_->value_data()->capacity());
 }
 
 TEST_F(TestBinaryBuilder, TestZeroLength) {

diff --git a/cpp/src/arrow/builder.cc b/cpp/src/arrow/builder.cc
@@ -1165,13 +1165,13 @@ Status ListBuilder::Init(int64_t elements) {
   DCHECK_LT(elements, std::numeric_limits<int32_t>::max());
   RETURN_NOT_OK(ArrayBuilder::Init(elements));
   // one more then requested for offsets
-  return offsets_builder_.Resize((elements + 1) * sizeof(int64_t));
+  return offsets_builder_.Resize((elements + 1) * sizeof(int32_t));
 }
 
 Status ListBuilder::Resize(int64_t capacity) {
   DCHECK_LT(capacity, std::numeric_limits<int32_t>::max());
   // one more then requested for offsets
-  RETURN_NOT_OK(offsets_builder_.Resize((capacity + 1) * sizeof(int64_t)));
+  RETURN_NOT_OK(offsets_builder_.Resize((capacity + 1) * sizeof(int32_t)));
   return ArrayBuilder::Resize(capacity);
 }
 
@@ -1216,7 +1216,7 @@ Status BinaryBuilder::Init(int64_t elements) {
   DCHECK_LT(elements, std::numeric_limits<int32_t>::max());
   RETURN_NOT_OK(ArrayBuilder::Init(elements));
   // one more then requested for offsets
-  return offsets_builder_.Resize((elements + 1) * sizeof(int64_t));
+  return offsets_builder_.Resize((elements + 1) * sizeof(int32_t));
 }
 
 Status BinaryBuilder::Resize(int64_t capacity) {

diff --git a/cpp/src/arrow/builder.h b/cpp/src/arrow/builder.h
@@ -682,8 +682,8 @@ class ARROW_EXPORT BinaryBuilder : public ArrayBuilder {
 
   Status Init(int64_t elements) override;
   Status Resize(int64_t capacity) override;
-  /// Ensures there is enough space for adding the number of value elements 
-  /// by checking value buffer capacity and resizing if necessary.
+  /// \brief Ensures there is enough allocated capacity to append the indicated 
+  /// number of bytes to the value data buffer without additional allocations
   Status ReserveData(int64_t elements);
   Status FinishInternal(std::shared_ptr<ArrayData>* out) override;