kszucs commented on a change in pull request #7341:
URL: https://github.com/apache/arrow/pull/7341#discussion_r435630182



##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -37,68 +37,275 @@
 namespace arrow {
 namespace compute {
 
-template <typename ArrowType>
-class TestArithmeticKernel : public TestBase {
- private:
-  void AssertAddArrays(const std::shared_ptr<Array> lhs, const 
std::shared_ptr<Array> rhs,
-                       const std::shared_ptr<Array> expected) {
-    ASSERT_OK_AND_ASSIGN(Datum out, arrow::compute::Add(lhs, rhs));
-    std::shared_ptr<Array> actual = out.make_array();
-    ASSERT_OK(actual->ValidateFull());
-    AssertArraysEqual(*expected, *actual);
-  }
+template <typename TypePair>
+class TestBinaryArithmetics;
 
+template <typename I, typename O>
+class TestBinaryArithmetics<std::pair<I, O>> : public TestBase {
  protected:
-  virtual void AssertAdd(const std::string& lhs, const std::string& rhs,
-                         const std::string& expected) {
-    auto type = TypeTraits<ArrowType>::type_singleton();
-    AssertAddArrays(ArrayFromJSON(type, lhs), ArrayFromJSON(type, rhs),
-                    ArrayFromJSON(type, expected));
+  using InputType = I;
+  using OutputType = O;
+  using InputCType = typename I::c_type;
+  using OutputCType = typename O::c_type;
+
+  using BinaryFunction =
+      std::function<Result<Datum>(const Datum&, const Datum&, ExecContext*)>;
+
+  std::shared_ptr<Array> MakeInputArray(const std::vector<InputCType>& values) 
{
+    std::shared_ptr<Array> out;
+    ArrayFromVector<InputType>(values, &out);
+    return out;
+  }
+
+  std::shared_ptr<Array> MakeOutputArray(const std::vector<OutputCType>& 
values) {
+    std::shared_ptr<Array> out;
+    ArrayFromVector<OutputType>(values, &out);
+    return out;
+  }
+
+  // (Scalar, Array)
+  void AssertBinop(BinaryFunction func, InputCType lhs, const std::string& rhs,
+                   const std::string& expected) {
+    auto input_type = TypeTraits<InputType>::type_singleton();
+    auto output_type = TypeTraits<OutputType>::type_singleton();
+
+    ASSERT_OK_AND_ASSIGN(auto left, MakeScalar(input_type, lhs));
+    auto right = ArrayFromJSON(input_type, rhs);
+    auto exp = ArrayFromJSON(output_type, expected);
+
+    ASSERT_OK_AND_ASSIGN(auto result, func(left, right, nullptr));
+    std::shared_ptr<Array> out = result.make_array();
+    ASSERT_OK(out->ValidateFull());
+    AssertArraysEqual(*exp, *out);
+  }
+
+  // (Array, Array)
+  void AssertBinop(BinaryFunction func, const std::shared_ptr<Array>& lhs,
+                   const std::shared_ptr<Array>& rhs,
+                   const std::shared_ptr<Array>& expected) {
+    ASSERT_OK_AND_ASSIGN(Datum result, func(lhs, rhs, nullptr));
+    std::shared_ptr<Array> out = result.make_array();
+    ASSERT_OK(out->ValidateFull());
+    AssertArraysEqual(*expected, *out);
+  }
+
+  void AssertBinop(BinaryFunction func, const std::string& lhs, const 
std::string& rhs,
+                   const std::string& expected) {
+    auto input_type = TypeTraits<InputType>::type_singleton();
+    auto output_type = TypeTraits<OutputType>::type_singleton();
+    AssertBinop(func, ArrayFromJSON(input_type, lhs), 
ArrayFromJSON(input_type, rhs),
+                ArrayFromJSON(output_type, expected));
   }
 };
 
-template <typename ArrowType>
-class TestArithmeticKernelFloating : public TestArithmeticKernel<ArrowType> {};
-TYPED_TEST_SUITE(TestArithmeticKernelFloating, RealArrowTypes);
+template <typename TypePair>
+class TestBinaryArithmeticsIntegral : public TestBinaryArithmetics<TypePair> 
{};
+
+template <typename TypePair>
+class TestBinaryArithmeticsSigned : public 
TestBinaryArithmeticsIntegral<TypePair> {};
+
+template <typename TypePair>
+class TestBinaryArithmeticsUnsigned : public 
TestBinaryArithmeticsIntegral<TypePair> {};
 
-template <typename ArrowType>
-class TestArithmeticKernelIntegral : public TestArithmeticKernel<ArrowType> {};
-TYPED_TEST_SUITE(TestArithmeticKernelIntegral, IntegralArrowTypes);
+template <typename TypePair>
+class TestBinaryArithmeticsFloating : public TestBinaryArithmetics<TypePair> 
{};
 
-TYPED_TEST(TestArithmeticKernelFloating, Add) {
-  this->AssertAdd("[]", "[]", "[]");
+// InputType - OutputType pairs
+using IntegralPairs =
+    testing::Types<std::pair<Int8Type, Int8Type>, std::pair<Int16Type, 
Int16Type>,
+                   std::pair<Int32Type, Int32Type>, std::pair<Int64Type, 
Int64Type>,
+                   std::pair<UInt8Type, UInt8Type>, std::pair<UInt16Type, 
UInt16Type>,
+                   std::pair<UInt32Type, UInt32Type>, std::pair<UInt64Type, 
UInt64Type>>;
 
-  this->AssertAdd("[3.4, 2.6, 6.3]", "[1, 0, 2]", "[4.4, 2.6, 8.3]");
+using SignedIntegerPairs =
+    testing::Types<std::pair<Int8Type, Int8Type>, std::pair<Int16Type, 
Int16Type>,
+                   std::pair<Int32Type, Int32Type>, std::pair<Int64Type, 
Int64Type>>;
 
-  this->AssertAdd("[1.1, 2.4, 3.5, 4.3, 5.1, 6.8, 7.3]", "[0, 1, 2, 3, 4, 5, 
6]",
-                  "[1.1, 3.4, 5.5, 7.3, 9.1, 11.8, 13.3]");
+using UnsignedIntegerPairs =
+    testing::Types<std::pair<UInt8Type, UInt8Type>, std::pair<UInt16Type, 
UInt16Type>,
+                   std::pair<UInt32Type, UInt32Type>, std::pair<UInt64Type, 
UInt64Type>>;
 
-  this->AssertAdd("[7, 6, 5, 4, 3, 2, 1]", "[6, 5, 4, 3, 2, 1, 0]",
-                  "[13, 11, 9, 7, 5, 3, 1]");
+// TODO(kszucs): add half-float
+using FloatingPairs =
+    testing::Types<std::pair<FloatType, FloatType>, std::pair<DoubleType, 
DoubleType>>;
 
-  this->AssertAdd("[10.4, 12, 4.2, 50, 50.3, 32, 11]", "[2, 0, 6, 1, 5, 3, 4]",
-                  "[12.4, 12, 10.2, 51, 55.3, 35, 15]");
+TYPED_TEST_SUITE(TestBinaryArithmeticsIntegral, IntegralPairs);
+TYPED_TEST_SUITE(TestBinaryArithmeticsSigned, SignedIntegerPairs);
+TYPED_TEST_SUITE(TestBinaryArithmeticsUnsigned, UnsignedIntegerPairs);
+TYPED_TEST_SUITE(TestBinaryArithmeticsFloating, FloatingPairs);
 
-  this->AssertAdd("[null, 1, 3.3, null, 2, 5.3]", "[1, 4, 2, 5, 0, 3]",
-                  "[null, 5, 5.3, null, 2, 8.3]");
+TYPED_TEST(TestBinaryArithmeticsIntegral, Add) {
+  this->AssertBinop(arrow::compute::Add, "[]", "[]", "[]");
+  this->AssertBinop(arrow::compute::Add, "[null]", "[null]", "[null]");
+  this->AssertBinop(arrow::compute::Add, "[3, 2, 6]", "[1, 0, 2]", "[4, 2, 
8]");
+
+  this->AssertBinop(arrow::compute::Add, "[1, 2, 3, 4, 5, 6, 7]", "[0, 1, 2, 
3, 4, 5, 6]",
+                    "[1, 3, 5, 7, 9, 11, 13]");
+
+  this->AssertBinop(arrow::compute::Add, "[10, 12, 4, 50, 50, 32, 11]",
+                    "[2, 0, 6, 1, 5, 3, 4]", "[12, 12, 10, 51, 55, 35, 15]");
+
+  this->AssertBinop(arrow::compute::Add, "[null, 1, 3, null, 2, 5]", "[1, 4, 
2, 5, 0, 3]",
+                    "[null, 5, 5, null, 2, 8]");
+
+  this->AssertBinop(arrow::compute::Add, 10, "[null, 1, 3, null, 2, 5]",
+                    "[null, 11, 13, null, 12, 15]");
+}
+
+TYPED_TEST(TestBinaryArithmeticsIntegral, Sub) {
+  this->AssertBinop(arrow::compute::Subtract, "[]", "[]", "[]");
+  this->AssertBinop(arrow::compute::Subtract, "[null]", "[null]", "[null]");
+  this->AssertBinop(arrow::compute::Subtract, "[3, 2, 6]", "[1, 0, 2]", "[2, 
2, 4]");
+
+  this->AssertBinop(arrow::compute::Subtract, "[1, 2, 3, 4, 5, 6, 7]",
+                    "[0, 1, 2, 3, 4, 5, 6]", "[1, 1, 1, 1, 1, 1, 1]");
+
+  this->AssertBinop(arrow::compute::Subtract, 10, "[null, 1, 3, null, 2, 5]",
+                    "[null, 9, 7, null, 8, 5]");
 }
 
-TYPED_TEST(TestArithmeticKernelIntegral, Add) {
-  this->AssertAdd("[]", "[]", "[]");
+TYPED_TEST(TestBinaryArithmeticsIntegral, Mul) {
+  this->AssertBinop(arrow::compute::Multiply, "[]", "[]", "[]");
+  this->AssertBinop(arrow::compute::Multiply, "[null]", "[null]", "[null]");
+  this->AssertBinop(arrow::compute::Multiply, "[3, 2, 6]", "[1, 0, 2]", "[3, 
0, 12]");
+
+  this->AssertBinop(arrow::compute::Multiply, "[1, 2, 3, 4, 5, 6, 7]",
+                    "[0, 1, 2, 3, 4, 5, 6]", "[0, 2, 6, 12, 20, 30, 42]");
+
+  this->AssertBinop(arrow::compute::Multiply, "[7, 6, 5, 4, 3, 2, 1]",
+                    "[6, 5, 4, 3, 2, 1, 0]", "[42, 30, 20, 12, 6, 2, 0]");
+
+  this->AssertBinop(arrow::compute::Multiply, "[null, 1, 3, null, 2, 5]",
+                    "[1, 4, 2, 5, 0, 3]", "[null, 4, 6, null, 0, 15]");
+
+  this->AssertBinop(arrow::compute::Multiply, 3, "[null, 1, 3, null, 2, 5]",
+                    "[null, 3, 9, null, 6, 15]");
+}
+
+TYPED_TEST(TestBinaryArithmeticsSigned, Add) {
+  this->AssertBinop(arrow::compute::Add, "[-7, 6, 5, 4, 3, 2, 1]",
+                    "[-6, 5, -4, 3, -2, 1, 0]", "[-13, 11, 1, 7, 1, 3, 1]");
+}
+
+TYPED_TEST(TestBinaryArithmeticsUnsigned, OverflowWraps) {
+  using InputCType = typename TestFixture::InputCType;
+
+  auto min = std::numeric_limits<InputCType>::min();
+  auto max = std::numeric_limits<InputCType>::max();
+  {
+    // Addition
+    auto left = this->MakeInputArray({max, min, max});
+    auto right = this->MakeInputArray({1, 1, max});
+    auto expected = this->MakeOutputArray({0, 1, static_cast<InputCType>(max - 
1)});
+    this->AssertBinop(arrow::compute::Add, left, right, expected);
+  }
+  {
+    // Subtraction
+    auto left = this->MakeInputArray({min, max});
+    auto right = this->MakeInputArray({1, max});
+    auto expected = this->MakeOutputArray({max, min});
+    this->AssertBinop(arrow::compute::Subtract, left, right, expected);
+  }
+  {
+    // Multiplication
+    auto left = this->MakeInputArray({min, max, max});
+    auto right = this->MakeInputArray({max, 2, max});
+    auto expected = this->MakeOutputArray({min, static_cast<InputCType>(max - 
1), 1});
+    this->AssertBinop(arrow::compute::Multiply, left, right, expected);
+  }
+}
+
+TYPED_TEST(TestBinaryArithmeticsSigned, OverflowWraps) {
+  using InputCType = typename TestFixture::InputCType;
+
+  auto min = std::numeric_limits<InputCType>::min();
+  auto max = std::numeric_limits<InputCType>::max();
+  {
+    // Addition
+    auto left = this->MakeInputArray({max, min});
+    auto right = this->MakeInputArray({1, -1});
+    auto expected = this->MakeOutputArray({min, max});
+    this->AssertBinop(arrow::compute::Add, left, right, expected);
+  }
+  {
+    // Subtraction
+    auto left = this->MakeInputArray({min, max});
+    auto right = this->MakeInputArray({1, -1});
+    auto expected = this->MakeOutputArray({max, min});
+    this->AssertBinop(arrow::compute::Subtract, left, right, expected);
+  }
+  {
+    // Multiplication
+    auto left = this->MakeInputArray({min, max});
+    auto right = this->MakeInputArray({-1, 2});
+    auto expected = this->MakeOutputArray({min, -2});
+    this->AssertBinop(arrow::compute::Multiply, left, right, expected);
+  }
+}
+
+TYPED_TEST(TestBinaryArithmeticsSigned, Sub) {
+  this->AssertBinop(arrow::compute::Subtract, "[0, 1, 2, 3, 4, 5, 6]",
+                    "[1, 2, 3, 4, 5, 6, 7]", "[-1, -1, -1, -1, -1, -1, -1]");
+
+  this->AssertBinop(arrow::compute::Subtract, "[0, 0, 0, 0, 0, 0, 0]",
+                    "[6, 5, 4, 3, 2, 1, 0]", "[-6, -5, -4, -3, -2, -1, 0]");
+
+  this->AssertBinop(arrow::compute::Subtract, "[10, 12, 4, 50, 50, 32, 11]",
+                    "[2, 0, 6, 1, 5, 3, 4]", "[8, 12, -2, 49, 45, 29, 7]");
+
+  this->AssertBinop(arrow::compute::Subtract, "[null, 1, 3, null, 2, 5]",
+                    "[1, 4, 2, 5, 0, 3]", "[null, -3, 1, null, 2, 2]");
+}
+
+TYPED_TEST(TestBinaryArithmeticsSigned, Mul) {
+  this->AssertBinop(arrow::compute::Multiply, "[-10, 12, 4, 50, -5, 32, 11]",
+                    "[-2, 0, -6, 1, 5, 3, 4]", "[20, 0, -24, 50, -25, 96, 
44]");
+}
+
+TYPED_TEST(TestBinaryArithmeticsFloating, Add) {
+  this->AssertBinop(arrow::compute::Add, "[]", "[]", "[]");
+
+  this->AssertBinop(arrow::compute::Add, "[3.4, 2.6, 6.3]", "[1, 0, 2]",
+                    "[4.4, 2.6, 8.3]");
+
+  this->AssertBinop(arrow::compute::Add, "[1.1, 2.4, 3.5, 4.3, 5.1, 6.8, 7.3]",
+                    "[0, 1, 2, 3, 4, 5, 6]", "[1.1, 3.4, 5.5, 7.3, 9.1, 11.8, 
13.3]");
+
+  this->AssertBinop(arrow::compute::Add, "[7, 6, 5, 4, 3, 2, 1]", "[6, 5, 4, 
3, 2, 1, 0]",
+                    "[13, 11, 9, 7, 5, 3, 1]");
+
+  this->AssertBinop(arrow::compute::Add, "[10.4, 12, 4.2, 50, 50.3, 32, 11]",
+                    "[2, 0, 6, 1, 5, 3, 4]", "[12.4, 12, 10.2, 51, 55.3, 35, 
15]");
+
+  this->AssertBinop(arrow::compute::Add, "[null, 1, 3.3, null, 2, 5.3]",
+                    "[1, 4, 2, 5, 0, 3]", "[null, 5, 5.3, null, 2, 8.3]");
+
+  this->AssertBinop(arrow::compute::Add, 1.1, "[null, 1, 3.3, null, 2, 5.3]",
+                    "[null, 2.1, 4.4, null, 3.1, 6.4]");
+}
+
+TYPED_TEST(TestBinaryArithmeticsFloating, Sub) {
+  this->AssertBinop(arrow::compute::Subtract, "[]", "[]", "[]");
+
+  this->AssertBinop(arrow::compute::Subtract, "[3.4, 2.6, 6.3]", "[1, 0, 2]",
+                    "[2.4, 2.6, 4.3]");
 
-  this->AssertAdd("[3, 2, 6]", "[1, 0, 2]", "[4, 2, 8]");
+  // this->AssertBinop(arrow::compute::Subtract,

Review comment:
       The array equality check fails despite that after printing out both 
sides the values are the same. I'm not sure why, perhaps a float precision 
problem? 




----------------------------------------------------------------
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]


Reply via email to