You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "WillAyd (via GitHub)" <gi...@apache.org> on 2024/03/11 03:02:01 UTC
[PR] GH-39386: [C++] Add floordiv compute kernel [arrow]
WillAyd opened a new pull request, #40450:
URL: https://github.com/apache/arrow/pull/40450
(no comment)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-39386: [C++] Add floordiv compute kernel [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd closed pull request #40450: GH-39386: [C++] Add floordiv compute kernel
URL: https://github.com/apache/arrow/pull/40450
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-39386: [C++] Add floordiv compute kernel [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #40450:
URL: https://github.com/apache/arrow/pull/40450#discussion_r1519090991
##########
cpp/src/arrow/compute/kernels/base_arithmetic_internal.h:
##########
@@ -464,6 +465,74 @@ struct FloatingDivideChecked {
// TODO: Add decimal
};
+struct FloorDiv {
+ template <typename T, typename Arg0, typename Arg1>
+ static enable_if_floating_value<T> Call(KernelContext*, Arg0 left, Arg1 right,
+ Status*) {
+ return floor(left / right);
+ }
+
+ template <typename T, typename Arg0, typename Arg1>
+ static enable_if_integer_value<T> Call(KernelContext*, Arg0 left, Arg1 right,
+ Status* st) {
+ T result;
+ if (ARROW_PREDICT_FALSE(DivideWithOverflow(left, right, &result))) {
+ if (right == 0) {
+ *st = Status::Invalid("divide by zero");
+ } else {
+ result = 0;
+ }
+ }
+ return static_cast<T>(floor(left / right));
+ }
+
+ template <typename T, typename Arg0, typename Arg1>
+ static enable_if_decimal_value<T> Call(KernelContext*, Arg0 left, Arg1 right,
+ Status* st) {
+ if (right == Arg1()) {
+ *st = Status::Invalid("Divide by zero");
+ return T();
+ } else {
+ // auto foo = left / right;
+ return 42;
Review Comment:
I wasn't sure how to convert the arguments here to doubles for floordiv. I originally tried placing the `ToDouble` method call in there but was getting this error:
```
In file included from /home/willayd/clones/arrow/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc:28:
/home/willayd/clones/arrow/cpp/src/arrow/compute/kernels/base_arithmetic_internal.h: In instantiation of ‘static arrow::compute::internal::enable_if_decimal_value<T> arrow::compute::internal::FloorDiv::Call(arrow::compute::KernelContext*, Arg0, Arg1, arrow::Status*) [with T = arrow::Decimal128; Arg0 = arrow::Decimal128; Arg1 = arrow::Decimal128; arrow::compute::internal::enable_if_decimal_value<T> = arrow::Decimal128]’:
/home/willayd/clones/arrow/cpp/src/arrow/compute/kernels/codegen_internal.h:814:72: required from ‘arrow::Status arrow::compute::internal::applicator::ScalarBinaryNotNullStateful<OutType, Arg0Type, Arg1Type, Op>::ArrayArray(arrow::compute::KernelContext*, const arrow::ArraySpan&, const arrow::ArraySpan&, arrow::compute::ExecResult*) [with OutType = arrow::Decimal128Type; Arg0Type = arrow::Decimal128Type; Arg1Type = arrow::Decimal128Type; Op = arrow::compute::internal::FloorDiv]’
/home/willayd/clones/arrow/cpp/src/arrow/compute/kernels/codegen_internal.h:863:16: required from ‘arrow::Status arrow::compute::internal::applicator::ScalarBinaryNotNullStateful<OutType, Arg0Type, Arg1Type, Op>::Exec(arrow::compute::KernelContext*, const arrow::compute::ExecSpan&, arrow::compute::ExecResult*) [with OutType = arrow::Decimal128Type; Arg0Type = arrow::Decimal128Type; Arg1Type = arrow::Decimal128Type; Op = arrow::compute::internal::FloorDiv]’
/home/willayd/clones/arrow/cpp/src/arrow/compute/kernels/codegen_internal.h:891:23: required from ‘static arrow::Status arrow::compute::internal::applicator::ScalarBinaryNotNull<OutType, Arg0Type, Arg1Type, Op>::Exec(arrow::compute::KernelContext*, const arrow::compute::ExecSpan&, arrow::compute::ExecResult*) [with OutType = arrow::Decimal128Type; Arg0Type = arrow::Decimal128Type; Arg1Type = arrow::Decimal128Type; Op = arrow::compute::internal::FloorDiv]’
/home/willayd/clones/arrow/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc:599:85: required from ‘void arrow::compute::internal::{anonymous}::AddDecimalBinaryKernels(const string&, arrow::compute::ScalarFunction*) [with Op = arrow::compute::internal::FloorDiv; std::string = std::__cxx11::basic_string<char>]’
/home/willayd/clones/arrow/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc:1592:36: required from here
/home/willayd/clones/arrow/cpp/src/arrow/compute/kernels/base_arithmetic_internal.h:497:24: error: ‘class arrow::BasicDecimal128’ has no member named ‘ToDouble’
```
I think there is something with C++ I am missing here, as I am surprised that the template argument thinks this is a `BasicDecimal128` instead of a `Decimal128`
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-39386: [C++] Add floordiv compute kernel [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40450:
URL: https://github.com/apache/arrow/pull/40450#issuecomment-1987540988
:warning: GitHub issue #39386 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-39386: [C++] Add floordiv compute kernel [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #40450:
URL: https://github.com/apache/arrow/pull/40450#issuecomment-1996283097
I don't think the current test failures are related?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org