Copilot commented on code in PR #48914:
URL: https://github.com/apache/arrow/pull/48914#discussion_r3908011101
##########
cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc:
##########
@@ -938,6 +938,173 @@ TYPED_TEST(TestBinaryArithmeticSigned,
DivideOverflowRaises) {
this->AssertBinop(Divide, MakeArray(min), MakeArray(-1), "[0]");
}
+// ============== REMAINDER (Truncated) Tests ==============
+
+TYPED_TEST(TestBinaryArithmeticIntegral, Remainder) {
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ // Empty arrays
+ this->AssertBinop(Remainder, "[]", "[]", "[]");
+ // Basic positive cases
+ this->AssertBinop(Remainder, "[7, 10, 20]", "[3, 4, 7]", "[1, 2, 6]");
+ // Array with nulls
+ this->AssertBinop(Remainder, "[null, 10, 30, null, 20]", "[1, 4, 2, 5,
10]",
+ "[null, 2, 0, null, 0]");
+ // Scalar % Array
+ this->AssertBinop(Remainder, 33, "[null, 1, 3, null, 2]", "[null, 0, 0,
null, 1]");
+ // Array % Scalar
+ this->AssertBinop(Remainder, "[null, 10, 30, null, 2]", 3, "[null, 1, 0,
null, 2]");
+ // Scalar % Scalar
+ this->AssertBinop(Remainder, 16, 7, 2);
+ }
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, Remainder) {
+ // Truncated semantics: sign follows dividend
+ this->AssertBinop(Remainder, "[7]", "[3]", "[1]");
+ this->AssertBinop(Remainder, "[-7]", "[3]", "[-1]");
+ this->AssertBinop(Remainder, "[7]", "[-3]", "[1]");
+ this->AssertBinop(Remainder, "[-7]", "[-3]", "[-1]");
+ // Mixed array
+ this->AssertBinop(Remainder, "[-3, 2, -7, 10]", "[1, 1, 2, 3]", "[0, 0, -1,
1]");
+}
+
+TYPED_TEST(TestBinaryArithmeticUnsigned, Remainder) {
+ this->AssertBinop(Remainder, "[7, 100, 255]", "[3, 30, 16]", "[1, 10, 15]");
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, RemainderOverflow) {
+ using CType = typename TestFixture::CType;
+ auto min = std::numeric_limits<CType>::lowest();
+
+ // Unchecked: returns 0 (the mathematically correct result)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, MakeArray(min), MakeArray(CType(-1)), "[0]");
+
+ // Checked: raises overflow error
+ this->SetOverflowCheck(true);
+ this->AssertBinopRaises(Remainder, MakeArray(min), MakeArray(CType(-1)),
"overflow");
+}
+
+TYPED_TEST(TestBinaryArithmeticIntegral, RemainderByZero) {
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ this->AssertBinopRaises(Remainder, "[3, 2, 6]", "[1, 1, 0]", "divide by
zero");
+ }
+}
+
+TYPED_TEST(TestBinaryArithmeticFloating, Remainder) {
+ SKIP_IF_HALF_FLOAT();
+
+ this->SetNansEqual(true);
+
+ // Basic cases
+ this->AssertBinop(Remainder, "[7.5, 10.0]", "[2.5, 3.0]", "[0.0, 1.0]");
+ // Negative numbers - truncated semantics: sign follows dividend
+ this->AssertBinop(Remainder, "[-7.5]", "[2.5]", "[-0.0]");
+ this->AssertBinop(Remainder, "[7.5]", "[-2.5]", "[0.0]");
+ this->AssertBinop(Remainder, "[-7.5]", "[-2.5]", "[-0.0]");
+
+ // Division by zero returns NaN (unchecked)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, "[1.0]", "[0.0]", "[NaN]");
+
+ // Division by zero raises error (checked)
+ this->SetOverflowCheck(true);
+ this->AssertBinopRaises(Remainder, "[1.0]", "[0.0]", "divide by zero");
+
+ // Infinity edge cases (unchecked)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, "[Inf]", "[2.0]", "[NaN]");
+ this->AssertBinop(Remainder, "[-Inf]", "[2.0]", "[NaN]");
+ this->AssertBinop(Remainder, "[2.0]", "[Inf]", "[2.0]");
+ this->AssertBinop(Remainder, "[2.0]", "[-Inf]", "[2.0]");
+ this->AssertBinop(Remainder, "[Inf]", "[Inf]", "[NaN]");
+}
+
+// ============== MOD (Floored) Tests ==============
+
+TYPED_TEST(TestBinaryArithmeticIntegral, Modulo) {
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ // Empty arrays
+ this->AssertBinop(Modulo, "[]", "[]", "[]");
+ // Basic positive cases (same as remainder for positive numbers)
+ this->AssertBinop(Modulo, "[7, 10, 20]", "[3, 4, 7]", "[1, 2, 6]");
+ // Array with nulls
+ this->AssertBinop(Modulo, "[null, 10, 30, null, 20]", "[1, 4, 2, 5, 10]",
+ "[null, 2, 0, null, 0]");
+ // Scalar % Array
+ this->AssertBinop(Modulo, 33, "[null, 1, 3, null, 2]", "[null, 0, 0, null,
1]");
+ // Array % Scalar
+ this->AssertBinop(Modulo, "[null, 10, 30, null, 2]", 3, "[null, 1, 0,
null, 2]");
+ }
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, Modulo) {
+ // Floored semantics: sign follows divisor
+ this->AssertBinop(Modulo, "[7]", "[3]", "[1]");
+ this->AssertBinop(Modulo, "[-7]", "[3]", "[2]");
+ this->AssertBinop(Modulo, "[7]", "[-3]", "[-2]");
+ this->AssertBinop(Modulo, "[-7]", "[-3]", "[-1]");
+ // Edge case: -1 mod positive
+ this->AssertBinop(Modulo, "[-1]", "[3]", "[2]");
+}
+
+TYPED_TEST(TestBinaryArithmeticUnsigned, Modulo) {
+ // Same as remainder for unsigned (no negative numbers)
+ this->AssertBinop(Modulo, "[7, 100, 255]", "[3, 30, 16]", "[1, 10, 15]");
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, ModOverflow) {
Review Comment:
Test name `ModOverflow` is inconsistent with the operation name (`Modulo`)
and the matching remainder test (`RemainderOverflow`), which makes test
output/search less clear. Consider renaming it to `ModuloOverflow`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
