kosiew commented on code in PR #23910:
URL: https://github.com/apache/datafusion/pull/23910#discussion_r3703106719
##########
datafusion/core/tests/fuzz_cases/equivalence/ordering.rs:
##########
@@ -144,14 +144,17 @@ fn
test_ordering_satisfy_with_equivalence_complex_random() -> Result<()> {
let err_msg = format!(
"Error in test case requirement:{ordering:?}, expected:
{expected:?}, eq_properties: {eq_properties}",
);
- // Check whether ordering_satisfy API result and
- // experimental result matches.
-
- assert_eq!(
- eq_properties.ordering_satisfy(ordering)?,
- (expected | false),
- "{err_msg}"
- );
+ let may_overflow = ordering
Review Comment:
I wonder if we could narrow this exemption to only the specific arithmetic
ordering that may be conservatively rejected. Using `any` means we skip the
rejection assertion for the entire multi-key requirement whenever one key
contains `+` or `-`, which reduces coverage for any later non-arithmetic keys.
Not blocking, since it does not affect production behavior.
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -1307,6 +1354,68 @@ mod tests {
use crate::planner::logical2physical;
use arrow::array::BooleanArray;
use datafusion_expr::col as logical_col;
+
+ #[test]
+ fn test_arithmetic_ordering_overflow() -> Result<()> {
Review Comment:
Nice focused regression test. One small suggestion: could we also add a
couple of `Minus` cases covering signed minimum and unsigned underflow
boundaries? The implementation now guards both `Plus` and `Minus`, but this
test only exercises addition. It would help protect the subtraction path from
future regressions.
--
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]
