kou commented on code in PR #47522:
URL: https://github.com/apache/arrow/pull/47522#discussion_r2338294672


##########
cpp/src/arrow/array/array_struct_test.cc:
##########
@@ -748,4 +748,94 @@ TEST(TestFieldRef, GetChildren) {
   AssertArraysEqual(*a, *expected_a);
 }
 
+TEST(TestStructBuilderUnsafe, UnsafeAppend) {
+  auto int_type = int32();
+  auto str_type = utf8();
+  auto struct_type = struct_({field("a", int_type), field("b", str_type)});
+  auto pool = default_memory_pool();
+  std::shared_ptr<Array> final_array;
+  auto int_builder = std::make_shared<Int32Builder>(pool);
+  auto str_builder = std::make_shared<StringBuilder>(pool);
+  StructBuilder builder(struct_type, pool, {int_builder, str_builder});
+
+  ASSERT_OK(builder.Reserve(2));
+
+  builder.UnsafeAppend();
+  ASSERT_OK(int_builder->Append(1));
+  ASSERT_OK(str_builder->Append("hello"));
+
+  builder.UnsafeAppend();
+  ASSERT_OK(int_builder->Append(2));
+  ASSERT_OK(str_builder->Append("arrow"));
+
+  ASSERT_OK(builder.Finish(&final_array));

Review Comment:
   ```suggestion
     ASSERT_OK_AND_ASSIGN(auto final_array, builder.Finish());
   ```



##########
cpp/src/arrow/array/builder_nested.h:
##########
@@ -791,6 +808,22 @@ class ARROW_EXPORT StructBuilder : public ArrayBuilder {
     return Status::OK();
   }
 
+  /// This method is "unsafe" because it does not check for capacity.
+  /// The caller is responsible for calling Reserve() beforehand to ensure
+  /// there is enough space. This method will append nulls to all child
+  /// builders to maintain a consistent state.
+  /// param length The number of null slots to append.
+  Status UnsafeAppendNulls(int64_t length) {
+    for (int64_t i = 0; i < length; ++i) {
+      UnsafeAppend(false);
+    }
+
+    for (const auto& child : children_) {
+      ARROW_RETURN_NOT_OK(child->AppendEmptyValues(length));
+    }

Review Comment:
   ```suggestion
       for (const auto& child : children_) {
         ARROW_RETURN_NOT_OK(child->AppendEmptyValues(length));
       }
       UnsafeAppendToBitmap(length, false);
   ```



##########
cpp/src/arrow/array/array_struct_test.cc:
##########
@@ -748,4 +748,94 @@ TEST(TestFieldRef, GetChildren) {
   AssertArraysEqual(*a, *expected_a);
 }
 
+TEST(TestStructBuilderUnsafe, UnsafeAppend) {
+  auto int_type = int32();
+  auto str_type = utf8();
+  auto struct_type = struct_({field("a", int_type), field("b", str_type)});

Review Comment:
   How about using meaningful field name for readability?
   
   ```suggestion
     auto struct_type = struct_({field("int", int_type), field("str", 
str_type)});
   ```



##########
cpp/src/arrow/array/array_struct_test.cc:
##########
@@ -748,4 +748,94 @@ TEST(TestFieldRef, GetChildren) {
   AssertArraysEqual(*a, *expected_a);
 }
 
+TEST(TestStructBuilderUnsafe, UnsafeAppend) {
+  auto int_type = int32();
+  auto str_type = utf8();
+  auto struct_type = struct_({field("a", int_type), field("b", str_type)});
+  auto pool = default_memory_pool();
+  std::shared_ptr<Array> final_array;
+  auto int_builder = std::make_shared<Int32Builder>(pool);
+  auto str_builder = std::make_shared<StringBuilder>(pool);
+  StructBuilder builder(struct_type, pool, {int_builder, str_builder});
+
+  ASSERT_OK(builder.Reserve(2));
+
+  builder.UnsafeAppend();
+  ASSERT_OK(int_builder->Append(1));
+  ASSERT_OK(str_builder->Append("hello"));
+
+  builder.UnsafeAppend();
+  ASSERT_OK(int_builder->Append(2));
+  ASSERT_OK(str_builder->Append("arrow"));
+
+  ASSERT_OK(builder.Finish(&final_array));
+  ASSERT_EQ(2, final_array->length());
+  ASSERT_EQ(0, final_array->null_count());
+  auto expected_json = R"([{"a": 1, "b": "hello"}, {"a": 2, "b": "arrow"}])";
+
+  auto expected_array = ArrayFromJSON(struct_type, expected_json);
+  AssertArraysEqual(*final_array, *expected_array);

Review Comment:
   ```suggestion
     AssertArraysEqual(*expected_array, *final_array);
   ```



##########
cpp/src/arrow/array/array_struct_test.cc:
##########
@@ -748,4 +748,94 @@ TEST(TestFieldRef, GetChildren) {
   AssertArraysEqual(*a, *expected_a);
 }
 
+TEST(TestStructBuilderUnsafe, UnsafeAppend) {
+  auto int_type = int32();
+  auto str_type = utf8();
+  auto struct_type = struct_({field("a", int_type), field("b", str_type)});
+  auto pool = default_memory_pool();
+  std::shared_ptr<Array> final_array;
+  auto int_builder = std::make_shared<Int32Builder>(pool);
+  auto str_builder = std::make_shared<StringBuilder>(pool);
+  StructBuilder builder(struct_type, pool, {int_builder, str_builder});
+
+  ASSERT_OK(builder.Reserve(2));
+
+  builder.UnsafeAppend();
+  ASSERT_OK(int_builder->Append(1));
+  ASSERT_OK(str_builder->Append("hello"));
+
+  builder.UnsafeAppend();
+  ASSERT_OK(int_builder->Append(2));
+  ASSERT_OK(str_builder->Append("arrow"));
+
+  ASSERT_OK(builder.Finish(&final_array));
+  ASSERT_EQ(2, final_array->length());
+  ASSERT_EQ(0, final_array->null_count());
+  auto expected_json = R"([{"a": 1, "b": "hello"}, {"a": 2, "b": "arrow"}])";
+

Review Comment:
   ```suggestion
   
     auto expected_json = R"([{"a": 1, "b": "hello"}, {"a": 2, "b": "arrow"}])";
   ```



-- 
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]

Reply via email to