search

apache / arrow

Apache Arrow is a multi-language toolbox for accelerated data interchange and in-memory processing
https://arrow.apache.org/
Apache License 2.0
14.3k stars 3.48k forks source link

[C++][Compute] Extend compute layer to support ternary scalar functions #18659

Open asfimport opened 3 years ago

asfimport commented 3 years ago

Compute layer describes ternary functions but the scalar API is missing support for this. Specifically, generator dispatchers and structs with ternary-based Exec methods. Refer to   https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/kernels/codegen_internal.h and https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/scalar_arithmetic.cc

 

This Between kernel PR adds some of the ternary functionalities.

Reporter: Eduardo Ponce / @edponce

Note: This issue was originally created as ARROW-12698. Please see the migration documentation for further details.

asfimport commented 3 years ago

David Li / @lidavidm: Sorry, what's missing here? AFAIK ternary functions can be defined just fine and a few are even implemented.

asfimport commented 3 years ago

Eduardo Ponce / @edponce: I opened this issue in the context of scalar ternary arithmetic functions. The only one that comes to mind is multiply-accumulate (aka fused-multiply add). Actually, now that I know more of Arrow's codebase, the missing support is not in api_scalar.cc but rather in https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/kernels/codegen_internal.h and https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/scalar_arithmetic.cc where there are no generator dispatchers for ternary functions nor structs with ternary-based Exec methods.

I encountered a similar situation when adding the first scalar unary arithmetic function, negate, and added this support in the same PR. If you think that this Jira is not meaningful and it is better to let the ternary support be added along with its corresponding functions/kernels, then we can remove it.

asfimport commented 3 years ago

David Li / @lidavidm: Ah, thanks for the clarification. No, I think it would be useful to have such helpers and we can keep this open, I just wanted to double-check what exactly was missing here.

Though as you point out, there are relatively few of these and so they may end up being just done ad-hoc.

asfimport commented 3 years ago

Eduardo Ponce / @edponce: I updated the Jira description to be more specific. Thanks!