wesm commented on a change in pull request #7748:
URL: https://github.com/apache/arrow/pull/7748#discussion_r454386790
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -74,6 +74,18 @@ class TestBinaryArithmetic : public TestBase {
ValidateAndAssertApproxEqual(actual.make_array(), expected);
}
+ // (Array, Scalar)
+ void AssertBinop(BinaryFunction func, const std::string& lhs, CType rhs,
+ const std::string& expected) {
+ auto left = ArrayFromJSON(type_singleton(), lhs);
+ ASSERT_OK_AND_ASSIGN(auto right, MakeScalar(type_singleton(), rhs));
+
+ auto exp = ArrayFromJSON(type_singleton(), expected);
+
+ ASSERT_OK_AND_ASSIGN(auto actual, func(left, right, options_, nullptr));
+ ValidateAndAssertApproxEqual(actual.make_array(), expected);
+ }
Review comment:
Need to check slices also
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -219,48 +219,89 @@ struct MultiplyChecked {
}
};
+struct Divide {
+ template <typename T>
+ static constexpr enable_if_floating_point<T> Call(KernelContext*, T left, T
right) {
+ return left / right;
+ }
+
+ template <typename T>
+ static constexpr enable_if_integer<T> Call(KernelContext*, T left, T right) {
+ return left / right;
+ }
+};
+
+struct DivideChecked {
+ template <typename T>
+ static enable_if_integer<T> Call(KernelContext* ctx, T left, T right) {
+ if (right == 0) {
+ ctx->SetStatus(Status::Invalid("divide by 0"));
+ return right;
+ }
+ return Divide::Call(ctx, left, right);
+ }
+
+ template <typename T>
+ static constexpr enable_if_floating_point<T> Call(KernelContext*, T left, T
right) {
+ return left * right;
+ }
+};
+
using applicator::ScalarBinaryEqualTypes;
+using applicator::ScalarBinaryNotNullEqualTypes;
// Generate a kernel given an arithmetic functor
//
// To avoid undefined behaviour of signed integer overflow treat the signed
// input argument values as unsigned then cast them to signed making them wrap
// around.
template <typename Op>
-ArrayKernelExec NumericEqualTypesBinary(detail::GetTypeId get_id) {
+ArrayKernelExec NumericEqualTypesBinary(detail::GetTypeId get_id,
+ bool skip_null = false) {
switch (get_id.id) {
case Type::INT8:
- return ScalarBinaryEqualTypes<Int8Type, Int8Type, Op>::Exec;
+ return skip_null ? ScalarBinaryNotNullEqualTypes<Int8Type, Int8Type,
Op>::Exec
Review comment:
I think this will cause both templates to always be instantiated, which
is not what we want
##########
File path: cpp/src/arrow/compute/kernels/scalar_cast_test.cc
##########
@@ -169,7 +169,6 @@ class TestCast : public TestBase {
ASSERT_EQ(in_values.size(), out_values.size());
std::shared_ptr<Array> input, expected;
if (is_valid.size() > 0) {
- ASSERT_EQ(is_valid.size(), out_values.size());
Review comment:
This change is out of place?
----------------------------------------------------------------
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:
us...@infra.apache.org
