From 869fa1ec7875c4cfc38bf70b14fc33ef63299f9b Mon Sep 17 00:00:00 2001 From: ZhangHuiGui <106943008+ZhangHuiGui@users.noreply.github.com> Date: Tue, 6 Feb 2024 06:11:36 +0800 Subject: [PATCH] GH-39860: [C++] Expression ExecuteScalarExpression execute empty args function with a wrong result (#39908) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 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 Co-authored-by: 张回归 Signed-off-by: Benjamin Kietzman --- 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 b47e0a35525c5..8c59ad1df86f2 100644 --- a/cpp/src/arrow/compute/expression.cc +++ b/cpp/src/arrow/compute/expression.cc @@ -761,6 +761,15 @@ Result 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 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 44159e76600fb..d33c348cd77da 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")),