bkietz commented on code in PR #40223:
URL: https://github.com/apache/arrow/pull/40223#discussion_r1506472979
##########
cpp/src/arrow/compute/expression.cc:
##########
@@ -536,14 +566,21 @@ Result<Expression> BindNonRecursive(Expression::Call
call, bool insert_implicit_
std::vector<TypeHolder> types = GetTypes(call.arguments);
ARROW_ASSIGN_OR_RAISE(call.function, GetFunction(call, exec_context));
- // First try and bind exactly
- Result<const Kernel*> maybe_exact_match =
call.function->DispatchExact(types);
- if (maybe_exact_match.ok()) {
- call.kernel = *maybe_exact_match;
- } else {
- if (!insert_implicit_casts) {
- return maybe_exact_match.status();
+ bool maybe_need_dispatch_best = IsNeedDispatchBest(types,
call.function_name);
Review Comment:
Ah, I see- sorry for the confusion. In addition to the changes I've
suggested, we need to ensure output type resolution (and kernel init) must
succeed before the exact kernel is accepted. For that the following should be
sufficient
```diff
diff --git a/cpp/src/arrow/compute/expression.cc
b/cpp/src/arrow/compute/expression.cc
index 8c59ad1df..a42bac175 100644
--- a/cpp/src/arrow/compute/expression.cc
+++ b/cpp/src/arrow/compute/expression.cc
@@ -536,14 +536,39 @@ Result<Expression> BindNonRecursive(Expression::Call
call, bool insert_implicit_
std::vector<TypeHolder> types = GetTypes(call.arguments);
ARROW_ASSIGN_OR_RAISE(call.function, GetFunction(call, exec_context));
+ auto FinishBind = [&] {
+ compute::KernelContext kernel_context(exec_context, call.kernel);
+
+ if (call.kernel->init) {
+ const FunctionOptions* options =
+ call.options ? call.options.get() :
call.function->default_options();
+ ARROW_ASSIGN_OR_RAISE(
+ call.kernel_state,
+ call.kernel->init(&kernel_context, {call.kernel, types,
options}));
+
+ kernel_context.SetState(call.kernel_state.get());
+ }
+
+ ARROW_ASSIGN_OR_RAISE(
+ call.type,
call.kernel->signature->out_type().Resolve(&kernel_context, types));
+
+ return Status::OK();
+ };
+
// First try and bind exactly
Result<const Kernel*> maybe_exact_match =
call.function->DispatchExact(types);
if (maybe_exact_match.ok()) {
call.kernel = *maybe_exact_match;
- } else {
+
+ if (FinishBind().ok()) {
+ return Expression(std::move(call));
+ }
+ }
+ {
if (!insert_implicit_casts) {
return maybe_exact_match.status();
}
+
// If exact binding fails, and we are allowed to cast, then prefer
casting literals
// first. Since DispatchBest generally prefers up-casting the best way
to do this is
// first down-cast the literals as much as possible
@@ -576,20 +601,7 @@ Result<Expression> BindNonRecursive(Expression::Call
call, bool insert_implicit_
}
}
- compute::KernelContext kernel_context(exec_context, call.kernel);
- if (call.kernel->init) {
- const FunctionOptions* options =
- call.options ? call.options.get() :
call.function->default_options();
- ARROW_ASSIGN_OR_RAISE(
- call.kernel_state,
- call.kernel->init(&kernel_context, {call.kernel, types, options}));
-
- kernel_context.SetState(call.kernel_state.get());
- }
-
- ARROW_ASSIGN_OR_RAISE(
- call.type,
call.kernel->signature->out_type().Resolve(&kernel_context, types));
-
+ RETURN_NOT_OK(FinishBind());
return Expression(std::move(call));
}
```
--
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]
