andygrove commented on code in PR #4939:
URL: https://github.com/apache/datafusion-comet/pull/4939#discussion_r3603757420
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -1160,6 +1153,60 @@ mod tests {
assert_eq!(decimal_array.value(1), -10000); // -100 * 10^2
assert!(decimal_array.is_null(2));
}
+
+ #[test]
+ fn test_cast_int_to_decimal128_overflow_legacy_nulls() {
+ // 1000 * 10^2 = 100000 does not fit precision 3 -> null (legacy).
Valid values and the
Review Comment:
Fixed. Replaced with "the vectorized null-on-overflow path."
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -1160,6 +1153,60 @@ mod tests {
assert_eq!(decimal_array.value(1), -10000); // -100 * 10^2
assert!(decimal_array.is_null(2));
}
+
+ #[test]
Review Comment:
Added `test_cast_int_to_decimal128_overflow_try_nulls` asserting the same
overflow-to-null behavior via `EvalMode::Try`.
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -663,48 +663,41 @@ where
T: ArrowPrimitiveType,
T::Native: Into<i128>,
{
- let mut builder = Decimal128Builder::with_capacity(array.len());
let multiplier = 10_i128.pow(scale as u32);
- for i in 0..array.len() {
- if array.is_null(i) {
- builder.append_null();
- } else {
- let v = array.value(i).into();
- let scaled = v.checked_mul(multiplier);
- match scaled {
- Some(scaled) => {
- if !is_validate_decimal_precision(scaled, precision) {
- match eval_mode {
- EvalMode::Ansi => {
- return Err(SparkError::NumericValueOutOfRange {
- value: v.to_string(),
- precision,
- scale,
- });
- }
- EvalMode::Try | EvalMode::Legacy =>
builder.append_null(),
- }
- } else {
- builder.append_value(scaled);
- }
+ // Single vectorized pass: a value that overflows the multiply or does not
fit the output
+ // precision maps to null. `unary_opt` only applies the closure to
non-null slots and carries
+ // the input null buffer over, replacing the per-element builder loop
without a second pass.
+ let result: Decimal128Array = array.unary_opt::<_, Decimal128Type>(|v| {
Review Comment:
Extracted a `fits` closure that is called from both the vectorized pass and
the ANSI rescan, so there is one spelling of the predicate.
--
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]
