lidavidm commented on code in PR #14043:
URL: https://github.com/apache/arrow/pull/14043#discussion_r965240623
##########
cpp/src/arrow/compute/function.h:
##########
@@ -225,6 +233,15 @@ class ARROW_EXPORT Function {
/// required by the kernel.
virtual Result<const Kernel*> DispatchBest(std::vector<TypeHolder>* values)
const;
+ /// \brief Get a function executor with a best-matching kernel
+ virtual Result<std::shared_ptr<FunctionExecutor>> BestExecutor(
+ const std::vector<Datum>& args, const FunctionOptions* options,
+ ExecContext* ctx) const;
Review Comment:
Without looking at the impl, only the description, the signatures here look
weird: I'd expect them to take types, not datums, since that's what kernel
resolution uses.
##########
cpp/src/arrow/compute/function.h:
##########
@@ -225,6 +233,15 @@ class ARROW_EXPORT Function {
/// required by the kernel.
virtual Result<const Kernel*> DispatchBest(std::vector<TypeHolder>* values)
const;
+ /// \brief Get a function executor with a best-matching kernel
+ virtual Result<std::shared_ptr<FunctionExecutor>> BestExecutor(
+ const std::vector<Datum>& args, const FunctionOptions* options,
+ ExecContext* ctx) const;
Review Comment:
I can see having these as additional overloads for convenience, but from the
Jira, it sounds like it'd be most useful to be able to create an executor from
just types?
##########
cpp/src/arrow/compute/function.h:
##########
@@ -159,6 +159,14 @@ struct ARROW_EXPORT FunctionDoc {
static const FunctionDoc& Empty();
};
+/// \brief An executor of a function with a preconfigured kernel
+struct ARROW_EXPORT FunctionExecutor {
+ virtual ~FunctionExecutor() {}
Review Comment:
just `=default`?
--
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]
