westonpace commented on code in PR #14434:
URL: https://github.com/apache/arrow/pull/14434#discussion_r1002079597
##########
cpp/src/arrow/engine/substrait/extension_set.cc:
##########
@@ -858,11 +867,34 @@ 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", "power", "sqrt", "abs"}) {
Review Comment:
`sqrt` isn't currently defined as overflowable in Substrait.
##########
cpp/src/arrow/engine/substrait/extension_set.cc:
##########
@@ -858,11 +867,34 @@ 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", "power", "sqrt", "abs"}) {
DCHECK_OK(
AddSubstraitCallToArrow({kSubstraitArithmeticFunctionsUri,
function_name},
DecodeOptionlessOverflowableArithmetic(function_name)));
}
+
+ // Mappings without a _checked variant
+ for (const auto& function_name : {"exp", "sign"}) {
+ DCHECK_OK(
+ AddSubstraitCallToArrow({kSubstraitArithmeticFunctionsUri,
function_name},
+
DecodeOptionlessUncheckedArithmetic(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)));
Review Comment:
The logrithmic functions are not technically overflowable either.
##########
cpp/src/arrow/engine/substrait/function_test.cc:
##########
@@ -177,6 +177,31 @@ TEST(FunctionMapping, ValidCases) {
{nullptr, int8(), int8()},
"0",
int8()},
+ {{kSubstraitArithmeticFunctionsUri, "sign"},
+ {"SILENT", "-1"},
Review Comment:
```suggestion
{"-1"},
```
`sign` does not have an overflow argument.
--
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]
