Fix return 0 while trying to divide or mod by 0 with decimal

[jackylee-ch]

No description provided.

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - How to contribute patches

[ccat3z]

Please add some unit tests:

diff --git a/cpp/src/gandiva/tests/decimal_single_test.cc b/cpp/src/gandiva/tests/decimal_single_test.cc
index 666ee4a68..66ef5dac5 100644
--- a/cpp/src/gandiva/tests/decimal_single_test.cc
+++ b/cpp/src/gandiva/tests/decimal_single_test.cc
@@ -31,11 +31,7 @@ using arrow::Decimal128;
 
 namespace gandiva {
 
-#define EXPECT_DECIMAL_RESULT(op, x, y, expected, actual)                                \
-  EXPECT_EQ(expected, actual) << op << " (" << (x).ToString() << "),(" << (y).ToString() \
-                              << ")"                                                     \
-                              << " expected : " << (expected).ToString()                 \
-                              << " actual : " << (actual).ToString();
+#define DECIMAL_TO_STRING(x, x_valid) ((x_valid) ? (x).ToString() : "null")
 
 DecimalScalar128 decimal_literal(const char* value, int precision, int scale) {
   std::string value_string = std::string(value);
@@ -46,10 +42,11 @@ class TestDecimalOps : public ::testing::Test {
  public:
   void SetUp() { pool_ = arrow::default_memory_pool(); }
 
-  ArrayPtr MakeDecimalVector(const DecimalScalar128& in);
+  ArrayPtr MakeDecimalVector(const DecimalScalar128& in, bool valid = true);
 
   void Verify(DecimalTypeUtil::Op, const std::string& function, const DecimalScalar128& x,
-              const DecimalScalar128& y, const DecimalScalar128& expected);
+              const DecimalScalar128& y, const DecimalScalar128& expected,
+              bool x_valid = true, bool y_valid = true, bool expected_valid = true);
 
   void AddAndVerify(const DecimalScalar128& x, const DecimalScalar128& y,
                     const DecimalScalar128& expected) {
@@ -67,31 +64,36 @@ class TestDecimalOps : public ::testing::Test {
   }
 
   void DivideAndVerify(const DecimalScalar128& x, const DecimalScalar128& y,
-                       const DecimalScalar128& expected) {
-    Verify(DecimalTypeUtil::kOpDivide, "divide", x, y, expected);
+                       const DecimalScalar128& expected, bool x_valid = true,
+                       bool y_valid = true, bool expected_valid = true) {
+    Verify(DecimalTypeUtil::kOpDivide, "divide", x, y, expected, x_valid, y_valid,
+           expected_valid);
   }
 
   void ModAndVerify(const DecimalScalar128& x, const DecimalScalar128& y,
-                    const DecimalScalar128& expected) {
-    Verify(DecimalTypeUtil::kOpMod, "mod", x, y, expected);
+                    const DecimalScalar128& expected, bool x_valid = true,
+                    bool y_valid = true, bool expected_valid = true) {
+    Verify(DecimalTypeUtil::kOpMod, "mod", x, y, expected, x_valid, y_valid,
+           expected_valid);
   }
 
  protected:
   arrow::MemoryPool* pool_;
 };
 
-ArrayPtr TestDecimalOps::MakeDecimalVector(const DecimalScalar128& in) {
+ArrayPtr TestDecimalOps::MakeDecimalVector(const DecimalScalar128& in, bool valid) {
   std::vector<arrow::Decimal128> ret;
 
   Decimal128 decimal_value = in.value();
 
   auto decimal_type = std::make_shared<arrow::Decimal128Type>(in.precision(), in.scale());
-  return MakeArrowArrayDecimal(decimal_type, {decimal_value}, {true});
+  return MakeArrowArrayDecimal(decimal_type, {decimal_value}, {valid});
 }
 
 void TestDecimalOps::Verify(DecimalTypeUtil::Op op, const std::string& function,
                             const DecimalScalar128& x, const DecimalScalar128& y,
-                            const DecimalScalar128& expected) {
+                            const DecimalScalar128& expected, bool x_valid, bool y_valid,
+                            bool expected_valid) {
   auto x_type = std::make_shared<arrow::Decimal128Type>(x.precision(), x.scale());
   auto y_type = std::make_shared<arrow::Decimal128Type>(y.precision(), y.scale());
   auto field_x = field("x", x_type);
@@ -114,8 +116,8 @@ void TestDecimalOps::Verify(DecimalTypeUtil::Op op, const std::string& function,
   ARROW_EXPECT_OK(status);
 
   // Create a row-batch with some sample data
-  auto array_a = MakeDecimalVector(x);
-  auto array_b = MakeDecimalVector(y);
+  auto array_a = MakeDecimalVector(x, x_valid);
+  auto array_b = MakeDecimalVector(y, y_valid);
 
   // prepare input record batch
   auto in_batch = arrow::RecordBatch::Make(schema, 1 /*num_records*/, {array_a, array_b});
@@ -132,8 +134,13 @@ void TestDecimalOps::Verify(DecimalTypeUtil::Op op, const std::string& function,
   auto dtype = dynamic_cast<arrow::Decimal128Type*>(out_array->type().get());
   std::string value_string = out_value.ToString(0);
   DecimalScalar128 actual{value_string, dtype->precision(), dtype->scale()};
+  auto actual_valid = out_array->IsValid(0);
 
-  EXPECT_DECIMAL_RESULT(function, x, y, expected, actual);
+  EXPECT_TRUE(expected_valid == actual_valid && (!actual_valid || expected == actual))
+      << op << " (" << DECIMAL_TO_STRING(x, x_valid) << "), ("
+      << DECIMAL_TO_STRING(y, y_valid) << ")"
+      << " expected: (" << DECIMAL_TO_STRING(expected, expected_valid) << ")"
+      << " actual: (" << DECIMAL_TO_STRING(actual, actual_valid) << ")";
 }
 
 TEST_F(TestDecimalOps, TestAdd) {
@@ -290,6 +297,30 @@ TEST_F(TestDecimalOps, TestDivide) {
   DivideAndVerify(DecimalScalar128(std::string(38, '9'), 38, 20),  // x
                   DecimalScalar128(std::string(35, '9'), 38, 20),  // x
                   DecimalScalar128("1000000000", 38, 6));
+
+  // x / 0 = null
+  DivideAndVerify(decimal_literal("201", 10, 3),  // x
+                  decimal_literal("0", 10, 2),    // y
+                  decimal_literal("0", 0, 0),     // expected
+                  true, true, false);             // valid
+
+  // null / x = null
+  DivideAndVerify(decimal_literal("201", 10, 3),  // x
+                  decimal_literal("301", 10, 2),  // y
+                  decimal_literal("0", 0, 0),     // expected
+                  false, true, false);            // valid
+
+  // x / null = null
+  DivideAndVerify(decimal_literal("201", 10, 3),  // x
+                  decimal_literal("301", 10, 2),  // y
+                  decimal_literal("0", 0, 0),     // expected
+                  true, false, false);            // valid
+
+  // null / null = null
+  DivideAndVerify(decimal_literal("201", 10, 3),  // x
+                  decimal_literal("301", 10, 2),  // y
+                  decimal_literal("0", 0, 0),     // expected
+                  false, false, false);           // valid
 }
 
 TEST_F(TestDecimalOps, TestMod) {
@@ -300,6 +331,30 @@ TEST_F(TestDecimalOps, TestMod) {
   ModAndVerify(DecimalScalar128(std::string(38, '9'), 38, 20),  // x
                DecimalScalar128(std::string(35, '9'), 38, 21),  // x
                DecimalScalar128("9990", 38, 21));
+
+  // x % 0 = null
+  ModAndVerify(decimal_literal("201", 10, 3),  // x
+               decimal_literal("0", 10, 2),    // y
+               decimal_literal("0", 0, 0),     // expected
+               true, true, false);             // valid
+
+  // null % x = null
+  ModAndVerify(decimal_literal("201", 10, 3),  // x
+               decimal_literal("301", 10, 2),  // y
+               decimal_literal("0", 0, 0),     // expected
+               false, true, false);            // valid
+
+  // x % null = null
+  ModAndVerify(decimal_literal("201", 10, 3),  // x
+               decimal_literal("301", 10, 2),  // y
+               decimal_literal("0", 0, 0),     // expected
+               true, false, false);            // valid
+
+  // null % null = null
+  ModAndVerify(decimal_literal("201", 10, 3),  // x
+               decimal_literal("301", 10, 2),  // y
+               decimal_literal("0", 0, 0),     // expected
+               false, false, false);           // valid
 }
 
 }  // namespace gandiva