pitrou commented on code in PR #50584:
URL: https://github.com/apache/arrow/pull/50584#discussion_r3628839257


##########
cpp/src/arrow/array/builder_run_end.cc:
##########
@@ -201,7 +201,6 @@ Status RunEndEncodedBuilder::AppendEmptyValues(int64_t 
length) {
   UpdateDimensions(committed_logical_length_, 0);
   return Status::OK();
 }
-

Review Comment:
   OTOH, let's please keep an empty line between function and method 
definitions.



##########
cpp/src/arrow/array/array_run_end_test.cc:
##########
@@ -366,6 +365,71 @@ TEST_P(TestRunEndEncodedArray, Builder) {
     }
   }
 }
+TEST_P(TestRunEndEncodedArray, BuilderAppendScalarsPrimitiveScalar) {
+  auto value_type = float32();
+  auto ree_type = run_end_encoded(run_end_type, value_type);
+
+  ASSERT_OK_AND_ASSIGN(std::shared_ptr<ArrayBuilder> builder, 
MakeBuilder(ree_type));
+
+  ASSERT_OK_AND_ASSIGN(auto v1, MakeScalar(float32(), 1.0f));
+  ASSERT_OK_AND_ASSIGN(auto v2, MakeScalar(float32(), 1.0f));
+  ASSERT_OK_AND_ASSIGN(auto v3, MakeScalar(float32(), 2.0f));
+  ASSERT_OK_AND_ASSIGN(auto v4, MakeScalar(float32(), 2.0f));
+  ASSERT_OK_AND_ASSIGN(auto v5, MakeScalar(float32(), 3.0f));
+
+  ScalarVector scalars = {v1, v2, v3, v4, v5};
+
+  ASSERT_OK(builder->AppendScalars(scalars));
+
+  ASSERT_EQ(builder->length(), 5);
+
+  ASSERT_OK_AND_ASSIGN(auto array, builder->Finish());

Review Comment:
   Let's also validate the result:
   
   ```suggestion
     ASSERT_OK_AND_ASSIGN(auto array, builder->Finish());
     ASSERT_OK(array->ValidateFull());
   ```



##########
cpp/src/arrow/array/array_run_end_test.cc:
##########
@@ -366,6 +365,71 @@ TEST_P(TestRunEndEncodedArray, Builder) {
     }
   }
 }
+TEST_P(TestRunEndEncodedArray, BuilderAppendScalarsPrimitiveScalar) {
+  auto value_type = float32();
+  auto ree_type = run_end_encoded(run_end_type, value_type);
+
+  ASSERT_OK_AND_ASSIGN(std::shared_ptr<ArrayBuilder> builder, 
MakeBuilder(ree_type));
+
+  ASSERT_OK_AND_ASSIGN(auto v1, MakeScalar(float32(), 1.0f));
+  ASSERT_OK_AND_ASSIGN(auto v2, MakeScalar(float32(), 1.0f));
+  ASSERT_OK_AND_ASSIGN(auto v3, MakeScalar(float32(), 2.0f));
+  ASSERT_OK_AND_ASSIGN(auto v4, MakeScalar(float32(), 2.0f));
+  ASSERT_OK_AND_ASSIGN(auto v5, MakeScalar(float32(), 3.0f));
+
+  ScalarVector scalars = {v1, v2, v3, v4, v5};
+

Review Comment:
   There's no need to add an empty line after each individual statement, can we 
compact this a bit and only keep empty lines to distinguish between logically 
different sequences?



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