mbutrovich commented on code in PR #4937:
URL: https://github.com/apache/datafusion-comet/pull/4937#discussion_r3591206195
##########
native/spark-expr/src/math_funcs/internal/checkoverflow.rs:
##########
@@ -381,4 +402,79 @@ mod tests {
other => panic!("unexpected: {other:?}"),
}
}
+
+ // --- array path ---
+
+ fn array_batch(values: Vec<Option<i128>>, in_precision: u8, scale: i8) ->
RecordBatch {
+ let arr = values
+ .into_iter()
+ .collect::<Decimal128Array>()
+ .with_precision_and_scale(in_precision, scale)
+ .unwrap();
+ let schema = Schema::new(vec![Field::new("d", arr.data_type().clone(),
true)]);
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(arr)]).unwrap()
+ }
+
+ fn array_check_overflow(target_precision: u8, scale: i8, fail_on_error:
bool) -> CheckOverflow {
Review Comment:
Current tests cover no-overflow, single-overflow, and mixed-null, but not:
- an all-null batch (the fast path takes `flatten().all(...)` which returns
`true` on an empty iterator, so all-null must reuse the input and preserve the
null mask - worth pinning),
- an all-overflow batch in legacy mode (every slot nulled) and ANSI mode
(errors),
- a boundary value that exactly equals `MAX_FOR_EACH_PRECISION[precision]`
(for example `Some(999)` at precision 3 is already the max and passes; add
`Some(9999)` at precision 3 to confirm it is treated as overflow, since
off-by-one on the bound is the classic failure).
These are cheap to add and directly guard the fast-path condition.
--
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]
