cyb70289 commented on a change in pull request #10686:
URL: https://github.com/apache/arrow/pull/10686#discussion_r667592918
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -1042,6 +1042,26 @@ TEST(TestUnaryArithmetic, DispatchBest) {
for (std::string name : {"negate", "negate_checked", "abs", "abs_checked"}) {
CheckDispatchFails(name, {null()});
}
+
+ for (std::string name :
+ {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos"}) {
+ for (std::string suffix : {"", "_checked"}) {
+ name += suffix;
+
+ CheckDispatchBest(name, {int32()}, {float64()});
+ CheckDispatchBest(name, {uint8()}, {float64()});
+
+ CheckDispatchBest(name, {dictionary(int8(), int64())}, {float64()});
+ }
+ }
+
+ CheckDispatchBest("atan", {int32()}, {float64()});
+ CheckDispatchBest("atan2", {int32(), float64()}, {float64(), float64()});
+ CheckDispatchBest("atan2", {int32(), uint8()}, {float64(), float64()});
+ CheckDispatchBest("atan2", {int32(), null()}, {float64(), float64()});
+ CheckDispatchBest("atan2", {float32(), float64()}, {float64(), float64()});
+ // Integer always promotes to double
+ CheckDispatchBest("atan2", {float32(), int8()}, {float64(), float64()});
Review comment:
Is it necessary to add some integer tests besides the implicit cast?
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -1076,6 +1076,38 @@ struct ArithmeticFunction : ScalarFunction {
}
};
+/// An ArithmeticFunction that promotes integer arguments to double.
+struct ArithmeticFloatingPointFunction : public ArithmeticFunction {
+ using ArithmeticFunction::ArithmeticFunction;
+
+ Result<const Kernel*> DispatchBest(std::vector<ValueDescr>* values) const
override {
+ RETURN_NOT_OK(CheckArity(*values));
+ RETURN_NOT_OK(CheckDecimals(values));
+
+ using arrow::compute::detail::DispatchExactImpl;
+ if (auto kernel = DispatchExactImpl(this, *values)) return kernel;
+
+ EnsureDictionaryDecoded(values);
+
+ // Only promote types for binary functions
Review comment:
This comment looks not correct.
--
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]