cyb70289 commented on a change in pull request #10364:
URL: https://github.com/apache/arrow/pull/10364#discussion_r654172014
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -1148,5 +1148,326 @@ TYPED_TEST(TestUnaryArithmeticFloating, AbsoluteValue) {
}
}
+class TestBinaryArithmeticDecimal : public TestBase {
+ protected:
+ std::shared_ptr<Scalar> MakeScalar(const std::shared_ptr<DataType>& type,
+ const std::string& str) {
Review comment:
Yes
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -461,6 +706,8 @@ std::shared_ptr<ScalarFunction>
MakeArithmeticFunction(std::string name,
auto exec = ArithmeticExecFromOp<ScalarBinaryEqualTypes, Op>(ty);
DCHECK_OK(func->AddKernel({ty, ty}, ty, exec));
}
+ AddDecimalBinaryKernels<Op>(name, &func);
Review comment:
I moved `AddDecimalBinaryKernels` calls out of `MakeArithmeticFunction`
and `MakeArithmeticFunctionNotNull`, and put it under each kernel supporting
decimal operations.
https://github.com/apache/arrow/pull/10364/files#diff-3eafd7246f6a8c699f10d46e3276852fe44b6853b5517ef10396e561730c09f4R840
I'm afraid some arithmetic kernels (e.g., `power`) may not support decimals?
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -1148,5 +1148,326 @@ TYPED_TEST(TestUnaryArithmeticFloating, AbsoluteValue) {
}
}
+class TestBinaryArithmeticDecimal : public TestBase {
+ protected:
+ std::shared_ptr<Scalar> MakeScalar(const std::shared_ptr<DataType>& type,
+ const std::string& str) {
Review comment:
Done
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -1148,5 +1148,326 @@ TYPED_TEST(TestUnaryArithmeticFloating, AbsoluteValue) {
}
}
+class TestBinaryArithmeticDecimal : public TestBase {
+ protected:
+ std::shared_ptr<Scalar> MakeScalar(const std::shared_ptr<DataType>& type,
+ const std::string& str) {
+ std::shared_ptr<Scalar> scalar;
+ if (type->id() == Type::DECIMAL128) {
+ Decimal128 value;
+ int32_t dummy;
+ ABORT_NOT_OK(Decimal128::FromString(str, &value, &dummy));
+ ASSIGN_OR_ABORT(scalar, arrow::MakeScalar(type, value));
+ } else {
+ Decimal256 value;
+ int32_t dummy;
+ ABORT_NOT_OK(Decimal256::FromString(str, &value, &dummy));
+ ASSIGN_OR_ABORT(scalar, arrow::MakeScalar(type, value));
+ }
+ return scalar;
+ }
+};
+
Review comment:
Done
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -451,8 +655,49 @@ struct ArithmeticFunction : ScalarFunction {
if (auto kernel = DispatchExactImpl(this, *values)) return kernel;
return arrow::compute::detail::NoMatchingKernel(this, *values);
}
+
+ Status CheckDecimals(std::vector<ValueDescr>* values) const {
+ bool has_decimal = false;
+ for (const auto& value : *values) {
+ if (is_decimal(value.type->id())) {
+ has_decimal = true;
+ break;
+ }
+ }
+ if (!has_decimal) return Status::OK();
+
+ if (values->size() == 2) {
+ return CastBinaryDecimalArgs(name(), values);
+ }
+ return Status::OK();
+ }
+};
+
+// resolve decimal operation output type
+struct BinaryDecimalOutputResolver {
+ const std::string func_name;
+
+ explicit BinaryDecimalOutputResolver(std::string func_name)
+ : func_name(std::move(func_name)) {}
+
+ Result<ValueDescr> operator()(KernelContext*, const std::vector<ValueDescr>&
args) {
+ ARROW_ASSIGN_OR_RAISE(auto type, ResolveBinaryDecimalOutput(func_name,
args));
+ return ValueDescr(std::move(type), GetBroadcastShape(args));
+ }
};
+template <typename Op>
+void AddDecimalBinaryKernels(const std::string& name,
+ std::shared_ptr<ArithmeticFunction>* func) {
+ auto out_type = OutputType(BinaryDecimalOutputResolver(name));
Review comment:
Done
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
