westonpace commented on code in PR #14434:
URL: https://github.com/apache/arrow/pull/14434#discussion_r997559329
##########
cpp/src/arrow/compute/kernels/base_arithmetic_internal.h:
##########
@@ -485,6 +485,34 @@ struct NegateChecked {
}
};
+struct Exp {
+ template <typename T, typename Arg>
+ static enable_if_floating_value<Arg, T> Call(KernelContext*, Arg exp,
Status*) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ return std::exp(exp);
+ }
+
+ template <typename T, typename Arg>
+ static enable_if_decimal_value<Arg, T> Call(KernelContext*, Arg exp,
Status*) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ return std::exp(exp);
+ }
+};
+
+struct ExpChecked {
Review Comment:
I'm not sure there is any value in creating a checked variant that does the
same thing as the unchecked variant.
##########
cpp/src/arrow/compute/kernels/scalar_arithmetic.cc:
##########
@@ -2224,6 +2230,15 @@ void RegisterScalarArithmetic(FunctionRegistry*
registry) {
"power_checked", pow_checked_doc);
DCHECK_OK(registry->AddFunction(std::move(power_checked)));
+ // ----------------------------------------------------------------------
+ auto exp = MakeUnaryArithmeticFunctionFloatingPoint<Exp>("exp", exp_doc);
Review Comment:
Any new kernel function will need unit tests.
##########
cpp/src/arrow/compute/kernels/scalar_arithmetic.cc:
##########
@@ -1717,6 +1717,12 @@ const FunctionDoc pow_doc{
"wraps around. If either base or exponent is null the result will be
null."),
{"base", "exponent"}};
+const FunctionDoc exp_doc{
+ "Computes Euler's number raised to the specified power, element-wise",
Review Comment:
```suggestion
"Compute Euler's number raised to the specified power, element-wise",
```
##########
cpp/src/arrow/compute/kernels/scalar_arithmetic.cc:
##########
@@ -1717,6 +1717,12 @@ const FunctionDoc pow_doc{
"wraps around. If either base or exponent is null the result will be
null."),
{"base", "exponent"}};
+const FunctionDoc exp_doc{
+ "Computes Euler's number raised to the specified power, element-wise",
+ ("Negative integer power returns an error. However, integer overflow\n"
Review Comment:
```suggestion
("Negative integer power returns an error. However, integer overflow\n"
```
What does negative integer power mean? This function only has non-integral
kernels.
I'm not sure what you mean by integer overflow. Why "integer"? The return
value is a floating point. If there is an overflow then
"[+HUGE_VAL](https://en.cppreference.com/w/cpp/numeric/math/HUGE_VAL),
+HUGE_VALF, or +HUGE_VALL is returned."
##########
cpp/src/arrow/engine/substrait/extension_set.cc:
##########
@@ -858,11 +858,27 @@ struct DefaultExtensionIdRegistry :
ExtensionIdRegistryImpl {
// -------------- Substrait -> Arrow Functions -----------------
// Mappings with a _checked variant
- for (const auto& function_name : {"add", "subtract", "multiply",
"divide"}) {
+ for (const auto& function_name : {"add", "subtract", "multiply", "divide",
"sign",
+ "power", "sqrt", "exp", "abs"}) {
DCHECK_OK(
AddSubstraitCallToArrow({kSubstraitArithmeticFunctionsUri,
function_name},
DecodeOptionlessOverflowableArithmetic(function_name)));
}
+
+ // Mappings for log functions
+ for (const auto& function_name : {"ln", "log10", "log2", "logb", "log1p"})
{
+ DCHECK_OK(
+ AddSubstraitCallToArrow({kSubstraitLogarithmicFunctionsUri,
function_name},
+
DecodeOptionlessOverflowableArithmetic(function_name)));
+ }
+
+ // Mappings for rounding functions
+ for (const auto& function_name : {"ceil", "floor"}) {
+ DCHECK_OK(
+ AddSubstraitCallToArrow({kSubstraitRoundingFunctionsUri,
function_name},
+
DecodeOptionlessOverflowableArithmetic(function_name)));
Review Comment:
Why use "Overflowable" here? I don't think there is a `ceil_checked` or
`floor_checked`?
--
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]
