andygrove commented on code in PR #5081:
URL: https://github.com/apache/datafusion-comet/pull/5081#discussion_r3684145302
##########
native/spark-expr/src/math_funcs/modulo_expr.rs:
##########
@@ -441,4 +520,126 @@ mod tests {
verify_result(modulo_expr, batch, fail_on_error,
Some(expected_result));
})
}
+
+ fn run_float_modulo<T: ArrowPrimitiveType>(
Review Comment:
You're right — every one of those went through `Column` expressions, so only
the `Array/Array` arm was exercised. Added coverage for the rest in 6c815fea
using `Literal`, as suggested:
`Array/Scalar` (column dividend, literal divisor):
- `test_modulo_literal_zero_divisor_float64_ansi`
- `test_modulo_literal_negative_zero_divisor_float64_ansi`
- `test_modulo_all_null_lhs_literal_zero_divisor_float64_ansi` — all
dividends null, so no row pairs a non-null dividend with the zero divisor and
nothing should raise
- `test_modulo_literal_non_zero_divisor_float64` — runs in both ANSI and
non-ANSI mode and checks results, not just the error
`Scalar/Array` (literal dividend, column divisor):
- `test_modulo_literal_dividend_zero_divisor_float64_ansi`
- `test_modulo_null_literal_dividend_zero_divisor_float64_ansi`
`Scalar/Scalar`:
- `test_modulo_both_literals_zero_divisor_float64_ansi`
- `test_modulo_both_literals_non_zero_divisor_float64_ansi` — asserts the
result is a `Scalar`, not a one-row array, which is the shape `apply` produced
before this change
19 tests in `modulo_expr` now.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
