Copilot commented on code in PR #47377:
URL: https://github.com/apache/arrow/pull/47377#discussion_r3756017348
##########
cpp/src/arrow/compute/kernel.h:
##########
@@ -555,26 +555,53 @@ struct ARROW_EXPORT Kernel {
/// employed this may not be possible.
using ArrayKernelExec = Status (*)(KernelContext*, const ExecSpan&,
ExecResult*);
+/// \brief The optional scalar kernel selective execution API for SCALAR
kernel types.
+/// It's like ArrayKernelExec but with an additional SelectionSpan argument.
When a
+/// selection vector is specified in the batch, this API will be preferred, if
provided,
+/// over ArrayKernelExec.
+using ArrayKernelSelectiveExec = Status (*)(KernelContext*, const ExecSpan&,
+ const SelectionSpan&, ExecResult*);
+
/// \brief Kernel data structure for implementations of ScalarFunction. In
/// addition to the members found in Kernel, contains the null handling
/// and memory pre-allocation preferences.
struct ARROW_EXPORT ScalarKernel : public Kernel {
ScalarKernel() = default;
+ ScalarKernel(std::shared_ptr<KernelSignature> sig, ArrayKernelExec exec,
+ ArrayKernelSelectiveExec selective_exec, KernelInit init =
NULLPTR)
+ : Kernel(std::move(sig), std::move(init)),
+ exec(std::move(exec)),
+ selective_exec(std::move(selective_exec)) {}
+
+ ScalarKernel(std::vector<InputType> in_types, OutputType out_type,
ArrayKernelExec exec,
+ ArrayKernelSelectiveExec selective_exec, KernelInit init =
NULLPTR)
+ : Kernel(std::move(in_types), std::move(out_type), std::move(init)),
+ exec(std::move(exec)),
+ selective_exec(std::move(selective_exec)) {}
+
ScalarKernel(std::shared_ptr<KernelSignature> sig, ArrayKernelExec exec,
KernelInit init = NULLPTR)
- : Kernel(std::move(sig), init), exec(exec) {}
+ : ScalarKernel(std::move(sig), std::move(exec), NULLPTR,
std::move(init)) {}
ScalarKernel(std::vector<InputType> in_types, OutputType out_type,
ArrayKernelExec exec,
KernelInit init = NULLPTR)
- : Kernel(std::move(in_types), std::move(out_type), std::move(init)),
exec(exec) {}
+ : ScalarKernel(std::move(in_types), std::move(out_type),
std::move(exec), NULLPTR,
+ std::move(init)) {}
/// \brief Perform a single invocation of this kernel. Depending on the
/// implementation, it may only write into preallocated memory, while in some
/// cases it will allocate its own memory. Any required state is managed
/// through the KernelContext.
ArrayKernelExec exec;
+ /// \brief Optional and similar to `exec`, but providing a specialized
implementation
+ /// that takes a selection vector argument and performs the computation only
on the
+ /// selected indices. When this specialized kernel is not provided we
fallback to
+ /// logic that gathers all selected values into a dense array, call `exec`
on it
+ /// and then scather the values on the output array.
Review Comment:
Typo in the kernel selective-exec documentation: “scather” should be
“scatter”.
##########
cpp/src/arrow/compute/test_util_internal.cc:
##########
@@ -120,4 +121,15 @@ void ValidateOutput(const Datum& output) {
}
}
+std::shared_ptr<SelectionVector> SelectionVectorFromJSON(const std::string&
json) {
+ return SelectionVector::MakeIndices(*ArrayFromJSON(int32(), json));
+}
+
+std::shared_ptr<SelectionVector> MakeSelectionVectorTo(int64_t length) {
+ auto res = gen::Step<int32_t>()->Generate(length);
+ DCHECK_OK(res.status());
+ auto arr = res.ValueUnsafe();
+ return SelectionVector::MakeIndices(*arr);
+}
Review Comment:
`MakeSelectionVectorTo` uses `DCHECK_OK(res.status())` and then
`ValueUnsafe()`. In release builds `DCHECK_OK` is compiled out, so
`ValueUnsafe()` could be called on an error status and lead to undefined
behavior. Use a non-debug check here since this helper is used by
tests/benchmarks regardless of build type.
--
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]
