[GitHub] [arrow] kou commented on a diff in pull request #13656: ARROW 17144: [C++][Gandiva] Add sqrt function

[GitBox <gi...@apache.org>]

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


##########
cpp/src/gandiva/tests/projector_test.cc:
##########
@@ -3384,4 +3384,111 @@ TEST_F(TestProjector, TestMaskDefault) {
   EXPECT_ARROW_ARRAY_EQUALS(exp_mask, outputs.at(0));
 }
 
+TEST_F(TestProjector, TestSqrt) {
+  // input fields
+  auto field1 = field("f1", arrow::int32());
+  auto field2 = field("f2", arrow::int64());
+  auto field3 = field("f3", arrow::float32());
+  auto field4 = field("f4", arrow::float64());
+
+  // schema fields
+  auto schema1 = arrow::schema({field1});
+  auto schema2 = arrow::schema({field2});
+  auto schema3 = arrow::schema({field3});
+  auto schema4 = arrow::schema({field4});
+
+  // output fields
+  auto field5 = field("sqrt_int32", arrow::float64());
+  auto field6 = field("sqrt_int64", arrow::float64());
+  auto field7 = field("sqrt_float32", arrow::float64());
+  auto field8 = field("sqrt_float64", arrow::float64());
+
+  // Build expression
+  auto sqrt_int32 = TreeExprBuilder::MakeExpression("sqrt", {field1}, field5);
+  auto sqrt_int64 = TreeExprBuilder::MakeExpression("sqrt", {field2}, field6);
+  auto sqrt_float32 = TreeExprBuilder::MakeExpression("sqrt", {field3}, field7);
+  auto sqrt_float64 = TreeExprBuilder::MakeExpression("sqrt", {field4}, field8);
+
+  std::shared_ptr<Projector> projector1;
+
+  auto status = Projector::Make(schema1, {sqrt_int32}, TestConfiguration(), &projector1);
+  EXPECT_TRUE(status.ok()) << status.message();

Review Comment:
   Can we use our `ASSERT_OK()` here?
   
   https://github.com/apache/arrow/blob/master/cpp/src/arrow/testing/gtest_util.h#L87
   
   ```suggestion
     ASSERT_OK(Projector::Make(schema1, {sqrt_int32}, TestConfiguration(), &projector1));
   ```



##########
cpp/src/gandiva/tests/projector_test.cc:
##########
@@ -3384,4 +3384,111 @@ TEST_F(TestProjector, TestMaskDefault) {
   EXPECT_ARROW_ARRAY_EQUALS(exp_mask, outputs.at(0));
 }
 
+TEST_F(TestProjector, TestSqrt) {
+  // input fields
+  auto field1 = field("f1", arrow::int32());
+  auto field2 = field("f2", arrow::int64());
+  auto field3 = field("f3", arrow::float32());
+  auto field4 = field("f4", arrow::float64());
+
+  // schema fields
+  auto schema1 = arrow::schema({field1});
+  auto schema2 = arrow::schema({field2});
+  auto schema3 = arrow::schema({field3});
+  auto schema4 = arrow::schema({field4});
+
+  // output fields
+  auto field5 = field("sqrt_int32", arrow::float64());
+  auto field6 = field("sqrt_int64", arrow::float64());
+  auto field7 = field("sqrt_float32", arrow::float64());
+  auto field8 = field("sqrt_float64", arrow::float64());
+
+  // Build expression
+  auto sqrt_int32 = TreeExprBuilder::MakeExpression("sqrt", {field1}, field5);
+  auto sqrt_int64 = TreeExprBuilder::MakeExpression("sqrt", {field2}, field6);
+  auto sqrt_float32 = TreeExprBuilder::MakeExpression("sqrt", {field3}, field7);
+  auto sqrt_float64 = TreeExprBuilder::MakeExpression("sqrt", {field4}, field8);
+
+  std::shared_ptr<Projector> projector1;
+
+  auto status = Projector::Make(schema1, {sqrt_int32}, TestConfiguration(), &projector1);
+  EXPECT_TRUE(status.ok()) << status.message();
+
+  // Create a row-batch with some sample data
+  int num_records = 4;
+
+  auto array1 = MakeArrowArrayInt32({1, 4, 9, 16}, {true, true, true, true});
+  auto in_batch1 = arrow::RecordBatch::Make(schema1, num_records, {array1});
+
+  auto out_int32 = MakeArrowArrayFloat64({1.0, 2.0, 3.0, 4.0}, {true, true, true, true});
+
+  arrow::ArrayVector outputs1;
+
+  // Evaluate expression
+  status = projector1->Evaluate(*in_batch1, pool_, &outputs1);
+  EXPECT_TRUE(status.ok()) << status.message();
+
+  EXPECT_ARROW_ARRAY_EQUALS(out_int32, outputs1.at(0));
+
+  std::shared_ptr<Projector> projector2;
+
+  status = Projector::Make(schema2, {sqrt_int64}, TestConfiguration(), &projector2);

Review Comment:
   Could you split `TestSqrt` to `TestSqrtInt32`, `TestSqrtInt64`, `TestSqrtFloat32` and `TestSqrtFloat64` for readability?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org