(arrow) branch main updated: GH-39860: [C++] Expression ExecuteScalarExpression execute empty args function with a wrong result (#39908)

[bkietz]

This is an automated email from the ASF dual-hosted git repository.

bkietz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new fd69d30744 GH-39860: [C++] Expression ExecuteScalarExpression execute 
empty args function with a wrong result (#39908)
fd69d30744 is described below

commit fd69d307447888101600376fa3016b727a3e0106
Author: ZhangHuiGui <106943008+zhanghui...@users.noreply.github.com>
AuthorDate: Tue Feb 6 06:11:36 2024 +0800

    GH-39860: [C++] Expression ExecuteScalarExpression execute empty args 
function with a wrong result (#39908)
    
    
    
    ### Rationale for this change
    
    Try to fix #39860.
    
    ### What changes are included in this PR?
    
    Deal with the call->arguments.size() == 0's condition in 
ExecuteScalarExpression when we call some functions
    has no arguments, like (random, hash_count ...).
    
    ### Are these changes tested?
    
    Yes
    
    ### Are there any user-facing changes?
    
    No.
    * Closes: #39860
    
    Lead-authored-by: hugo.zhang <hugo.zh...@openpie.com>
    Co-authored-by: 张回归 <zhanghuigui@zhanghuiguideMacBook-Pro-1681.local>
    Signed-off-by: Benjamin Kietzman <bengil...@gmail.com>
---
 cpp/src/arrow/compute/expression.cc      | 13 +++++++++++--
 cpp/src/arrow/compute/expression_test.cc | 19 +++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/cpp/src/arrow/compute/expression.cc 
b/cpp/src/arrow/compute/expression.cc
index b47e0a3552..8c59ad1df8 100644
--- a/cpp/src/arrow/compute/expression.cc
+++ b/cpp/src/arrow/compute/expression.cc
@@ -761,6 +761,15 @@ Result<Datum> ExecuteScalarExpression(const Expression& 
expr, const ExecBatch& i
     }
   }
 
+  int64_t input_length;
+  if (!arguments.empty() && all_scalar) {
+    // all inputs are scalar, so use a 1-long batch to avoid
+    // computing input.length equivalent outputs
+    input_length = 1;
+  } else {
+    input_length = input.length;
+  }
+
   auto executor = compute::detail::KernelExecutor::MakeScalar();
 
   compute::KernelContext kernel_context(exec_context, call->kernel);
@@ -772,8 +781,8 @@ Result<Datum> ExecuteScalarExpression(const Expression& 
expr, const ExecBatch& i
   RETURN_NOT_OK(executor->Init(&kernel_context, {kernel, types, options}));
 
   compute::detail::DatumAccumulator listener;
-  RETURN_NOT_OK(executor->Execute(
-      ExecBatch(std::move(arguments), all_scalar ? 1 : input.length), 
&listener));
+  RETURN_NOT_OK(
+      executor->Execute(ExecBatch(std::move(arguments), input_length), 
&listener));
   const auto out = executor->WrapResults(arguments, listener.values());
 #ifndef NDEBUG
   DCHECK_OK(executor->CheckResultType(out, call->function_name.c_str()));
diff --git a/cpp/src/arrow/compute/expression_test.cc 
b/cpp/src/arrow/compute/expression_test.cc
index 44159e7660..d33c348cd7 100644
--- a/cpp/src/arrow/compute/expression_test.cc
+++ b/cpp/src/arrow/compute/expression_test.cc
@@ -863,6 +863,25 @@ TEST(Expression, ExecuteCall) {
   ])"));
 }
 
+TEST(Expression, ExecuteCallWithNoArguments) {
+  const int kCount = 10;
+  auto random_options = RandomOptions::FromSeed(/*seed=*/0);
+  ExecBatch input({}, kCount);
+
+  Expression random_expr = call("random", {}, random_options);
+  ASSERT_OK_AND_ASSIGN(random_expr, random_expr.Bind(float64()));
+
+  ASSERT_OK_AND_ASSIGN(Datum actual, ExecuteScalarExpression(random_expr, 
input));
+  compute::ExecContext* exec_context = default_exec_context();
+  ASSERT_OK_AND_ASSIGN(auto function,
+                       exec_context->func_registry()->GetFunction("random"));
+  ASSERT_OK_AND_ASSIGN(Datum expected,
+                       function->Execute(input, &random_options, 
exec_context));
+  AssertDatumsEqual(actual, expected, /*verbose=*/true);
+
+  EXPECT_EQ(actual.length(), kCount);
+}
+
 TEST(Expression, ExecuteDictionaryTransparent) {
   ExpectExecute(
       equal(field_ref("a"), field_ref("b")),