Skip to content

Commit

Permalink
Fix string '0' cast to decimal with scale 0
Browse files Browse the repository at this point in the history
Before the change, the cast used to fail or return null, depending on
cast options.
  • Loading branch information
findepi committed Oct 11, 2024
1 parent 021c216 commit 6240664
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 1 deletion.
47 changes: 46 additions & 1 deletion arrow-cast/src/cast/decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ where
}
};

let integers = first_part.trim_start_matches('0');
let integers = first_part;
let decimals = if parts.len() == 2 { parts[1] } else { "" };

if !integers.is_empty() && !integers.as_bytes()[0].is_ascii_digit() {
Expand Down Expand Up @@ -567,3 +567,48 @@ where
let array = array.unary::<_, T>(op);
Ok(Arc::new(array))
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_parse_string_to_decimal_native() -> Result<(), ArrowError> {
assert_eq!(
parse_string_to_decimal_native::<Decimal128Type>("0", 0)?,
0_i128
);
assert_eq!(
parse_string_to_decimal_native::<Decimal128Type>("0", 5)?,
0_i128
);

assert_eq!(
parse_string_to_decimal_native::<Decimal128Type>("123", 0)?,
123_i128
);
assert_eq!(
parse_string_to_decimal_native::<Decimal128Type>("123", 5)?,
12300000_i128
);

assert_eq!(
parse_string_to_decimal_native::<Decimal128Type>("123.45", 0)?,
123_i128
);
assert_eq!(
parse_string_to_decimal_native::<Decimal128Type>("123.45", 5)?,
12345000_i128
);

assert_eq!(
parse_string_to_decimal_native::<Decimal128Type>("123.4567891", 0)?,
123_i128
);
assert_eq!(
parse_string_to_decimal_native::<Decimal128Type>("123.4567891", 5)?,
12345679_i128
);
Ok(())
}
}
80 changes: 80 additions & 0 deletions arrow-cast/src/cast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8481,6 +8481,9 @@ mod tests {
assert!(decimal_arr.is_null(25));
assert!(decimal_arr.is_null(26));
assert!(decimal_arr.is_null(27));
assert_eq!("0.00", decimal_arr.value_as_string(28));
assert_eq!("0.00", decimal_arr.value_as_string(29));
assert_eq!(decimal_arr.len(), 30);

// Decimal256
let output_type = DataType::Decimal256(76, 3);
Expand Down Expand Up @@ -8517,6 +8520,9 @@ mod tests {
assert!(decimal_arr.is_null(25));
assert!(decimal_arr.is_null(26));
assert!(decimal_arr.is_null(27));
assert_eq!("0.000", decimal_arr.value_as_string(28));
assert_eq!("0.000", decimal_arr.value_as_string(29));
assert_eq!(decimal_arr.len(), 30);
}

#[test]
Expand Down Expand Up @@ -8550,10 +8556,28 @@ mod tests {
Some("1.-23499999"),
Some("-1.-23499999"),
Some("--1.23499999"),
Some("0"),
Some("000.000"),
]);
let array = Arc::new(str_array) as ArrayRef;

test_cast_string_to_decimal(array);

let test_cases = [
(None, None),
// (Some(""), None),
// (Some(" "), None),
(Some("0"), Some("0")),
(Some("000.000"), Some("0")),
(Some("12345"), Some("12345")),
(Some("-123"), Some("-123")),
(Some("+123"), Some("123")),
];
let inputs = test_cases.iter().map(|entry| entry.0).collect::<Vec<_>>();
let expected = test_cases.iter().map(|entry| entry.1).collect::<Vec<_>>();

let array = Arc::new(StringArray::from(inputs)) as ArrayRef;
test_cast_string_to_decimal_scale_zero(array, &expected);
}

#[test]
Expand Down Expand Up @@ -8587,10 +8611,66 @@ mod tests {
Some("1.-23499999"),
Some("-1.-23499999"),
Some("--1.23499999"),
Some("0"),
Some("000.000"),
]);
let array = Arc::new(str_array) as ArrayRef;

test_cast_string_to_decimal(array);

let test_cases = [
(None, None),
(Some(""), None),
(Some(" "), None),
(Some("0"), Some("0")),
(Some("000.000"), Some("0")),
(Some("12345"), Some("12345")),
(Some("-123"), Some("-123")),
(Some("+123"), Some("123")),
];
let inputs = test_cases.iter().map(|entry| entry.0).collect::<Vec<_>>();
let expected = test_cases.iter().map(|entry| entry.1).collect::<Vec<_>>();

let array = Arc::new(LargeStringArray::from(inputs)) as ArrayRef;
test_cast_string_to_decimal_scale_zero(array, &expected);
}

fn test_cast_string_to_decimal_scale_zero(
array: ArrayRef,
expected_as_string: &[Option<&str>],
) {
// Decimal128
let output_type = DataType::Decimal128(38, 0);
assert!(can_cast_types(array.data_type(), &output_type));
let casted_array = cast(&array, &output_type).unwrap();
let decimal_arr = casted_array.as_primitive::<Decimal128Type>();
assert_decimal_array_contents(decimal_arr, expected_as_string);

// Decimal256
let output_type = DataType::Decimal256(76, 0);
assert!(can_cast_types(array.data_type(), &output_type));
let casted_array = cast(&array, &output_type).unwrap();
let decimal_arr = casted_array.as_primitive::<Decimal256Type>();
assert_decimal_array_contents(decimal_arr, expected_as_string);
}

fn assert_decimal_array_contents<T>(
array: &PrimitiveArray<T>,
expected_as_string: &[Option<&str>],
) where
T: DecimalType + ArrowPrimitiveType,
{
assert_eq!(array.len(), expected_as_string.len());
for i in 0..expected_as_string.len() {
let expected = expected_as_string[i];
let actual = if array.is_null(i) {
None
} else {
Some(array.value_as_string(i))
};
let actual = actual.as_ref().map(|s| s.as_ref());
assert_eq!(expected, actual, "Expected at position {}", i);
}
}

#[test]
Expand Down

0 comments on commit 6240664

Please sign in to comment.