Jefffrey commented on code in PR #10850:
URL: https://github.com/apache/arrow-rs/pull/10850#discussion_r3888883529
##########
arrow-cast/src/parse.rs:
##########
@@ -2865,19 +3001,454 @@ mod tests {
];
for (s, i, precision) in zero_scale_tests {
let result_128 = parse_decimal::<Decimal128Type>(s, precision,
0).unwrap();
- assert_eq!(i, result_128);
+ assert_eq!(i, result_128, "{s}");
}
let can_not_parse_zero_scale = [".", "blag", "", "+", "-", "e"];
for s in can_not_parse_zero_scale {
let result_128 = parse_decimal::<Decimal128Type>(s, 5, 0);
assert_eq!(
- format!("Parser error: can't parse the string value {s} to
decimal"),
+ format!("Parser error: Invalid decimal format: {s:?}"),
result_128.unwrap_err().to_string(),
);
}
}
+ #[test]
+ fn test_parse_decimal_rounds_half_away_from_zero() {
+ let tests = [
+ ("1.234", 2, 123),
+ ("1.235", 2, 124),
+ ("1.2350000", 2, 124),
+ ("1.2349999", 2, 123),
+ ("-1.234", 2, -123),
+ ("-1.235", 2, -124),
+ ("-0.004", 2, 0),
+ ("-0.005", 2, -1),
+ (".5", 0, 1),
+ ("-.5", 0, -1),
+ ("0.5", 0, 1),
+ ("1.5", 0, 2),
+ ("2.5", 0, 3),
+ ("-2.5", 0, -3),
+ ("1.99", 1, 20),
+ ("0.995", 2, 100),
+ ("9.99", 1, 100),
+ ("123.4567891", 5, 12345679),
+ ("123.45", 0, 123),
+ ("0.0000123", 3, 0),
+ ("12.", 2, 1200),
+ (".12", 2, 12),
+ ("+.12", 2, 12),
+ ("-.12", 2, -12),
+ ];
+ for (s, scale, expected) in tests {
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(s, 38, scale).unwrap(),
+ expected,
+ "{s} at scale {scale}"
+ );
+ assert_eq!(
+ parse_decimal::<Decimal256Type>(s, 76, scale).unwrap(),
+ i256::from_i128(expected),
+ "{s} at scale {scale}"
+ );
+ }
+ }
+
+ #[test]
+ fn test_parse_decimal_rounding_overflow() {
+ // Rounding up can push the value past the precision ...
+ assert!(parse_decimal::<Decimal128Type>("99999.5", 5, 0).is_err());
+ assert!(parse_decimal::<Decimal128Type>("9.995", 3, 2).is_err());
+ assert_eq!(parse_decimal::<Decimal128Type>("9.994", 3, 2).unwrap(),
999);
+ assert_eq!(parse_decimal::<Decimal128Type>("0.995", 3, 2).unwrap(),
100);
+
+ // ... or past the native type itself
+ assert_eq!(
+ parse_decimal_native::<Decimal32Type>("2147483647.5", 0),
+ Err(DecimalParseError::Overflow)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal32Type>("-2147483648.5", 0),
+ Err(DecimalParseError::Overflow)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal128Type>(&format!("{}.5",
i128::MAX), 0),
+ Err(DecimalParseError::Overflow)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal128Type>(&format!("{}.5",
i128::MIN), 0),
+ Err(DecimalParseError::Overflow)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal256Type>(&format!("{}.5",
i256::MAX), 0),
+ Err(DecimalParseError::Overflow)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal256Type>(&format!("{}.5",
i256::MIN), 0),
+ Err(DecimalParseError::Overflow)
+ );
+ }
+
+ #[test]
+ fn test_parse_decimal_native_full_range() {
+ // The native range exceeds the largest precision; the precision check
+ // is the caller's responsibility
+ assert_eq!(
+ parse_decimal_native::<Decimal32Type>("-2147483648", 0),
+ Ok(i32::MIN)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal32Type>("2147483648", 0),
+ Err(DecimalParseError::Overflow)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal64Type>("-9223372036854775808", 0),
+ Ok(i64::MIN)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal64Type>("9223372036854775808", 0),
+ Err(DecimalParseError::Overflow)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal128Type>(&i128::MAX.to_string(), 0),
+ Ok(i128::MAX)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal128Type>(&i128::MIN.to_string(), 0),
+ Ok(i128::MIN)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal256Type>(&i256::MAX.to_string(), 0),
+ Ok(i256::MAX)
+ );
+ assert_eq!(
+ parse_decimal_native::<Decimal256Type>(&i256::MIN.to_string(), 0),
+ Ok(i256::MIN)
+ );
+ // The unscaled value (integer digits scaled by 10^21) far exceeds the
+ // i256 range, so this must report overflow rather than wrapping to an
+ // arbitrary (possibly in-range) value
+ let input = format!("{}.12345678901234567890123", "7".repeat(71));
+ assert_eq!(
+ parse_decimal_native::<Decimal256Type>(&input, 21),
+ Err(DecimalParseError::Overflow)
+ );
+
+ assert!(parse_decimal::<Decimal128Type>(&i128::MAX.to_string(), 38,
0).is_err());
+ assert!(parse_decimal::<Decimal32Type>("-2147483648", 9, 0).is_err());
+ }
+
+ #[test]
+ fn test_parse_decimal_integer_widths() {
+ assert_eq!(
+ parse_decimal::<Decimal32Type>("123.45", 9, 2).unwrap(),
+ 12_345_i32
+ );
+ assert_eq!(
+ parse_decimal::<Decimal32Type>("-9999999.994", 9, 2).unwrap(),
+ -999_999_999_i32
+ );
+ assert!(parse_decimal::<Decimal32Type>("9999999.995", 9, 2).is_err());
+ assert!(parse_decimal::<Decimal32Type>("-9999999.995", 9, 2).is_err());
+ assert_eq!(
+ parse_decimal::<Decimal64Type>("123.45", 18, 2).unwrap(),
+ 12_345_i64
+ );
+ assert_eq!(
+ parse_decimal::<Decimal64Type>("9999999999999999.99", 18,
2).unwrap(),
+ 999_999_999_999_999_999_i64
+ );
+ assert!(parse_decimal::<Decimal64Type>("10000000000000000.00", 18,
2).is_err());
+ // Fractional parts longer than any native integer type parse fine;
+ // digits beyond the scale only matter for rounding
+ assert_eq!(
+ parse_decimal::<Decimal64Type>(&format!(".{}", "5".repeat(100)),
18, 4).unwrap(),
+ 5_556_i64
+ );
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(&format!(".{}", "1".repeat(100)),
38, 4).unwrap(),
+ 1_111_i128
+ );
+ }
+
+ #[test]
+ fn test_parse_decimal_exponent() {
+ let tests = [
+ ("1e2", 0, 100),
+ ("1E2", 0, 100),
+ ("1e+2", 0, 100),
+ ("1e+02", 0, 100),
+ ("1.5e2", 0, 150),
+ ("1.5e2", 2, 15000),
+ ("1.5e-1", 1, 2),
+ ("15e-1", 0, 2),
+ ("1e-2", 1, 0),
+ ("1e-3", 2, 0),
+ ("0e0", 2, 0),
+ ("-0e0", 2, 0),
+ ("0E5", 2, 0),
+ ("0e99999", 2, 0),
+ ("00e48", 8, 0),
+ ("+00.0E+41", 12, 0),
+ ("1.25e1", 0, 13),
+ ("1e-99999", 2, 0),
+ ("1.5e-400", 2, 0),
+ ("123456789e-9", 9, 123456789),
+ ("0.000000001e9", 0, 1),
+ ("5e-1", 0, 1),
+ ("4e-1", 0, 0),
+ ("-5e-1", 0, -1),
+ ];
+ for (s, scale, expected) in tests {
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(s, 38, scale).unwrap(),
+ expected,
+ "{s} at scale {scale}"
+ );
+ assert_eq!(
+ parse_decimal::<Decimal32Type>(s, 9, scale).unwrap(),
+ expected as i32,
+ "{s} at scale {scale}"
+ );
+ }
+
+ // Exponents shift digits across the decimal point without losing any
+ assert_eq!(
+
parse_decimal::<Decimal32Type>("4825037936439135476.2609835314269495255615E-14",
9, 4)
+ .unwrap(),
+ 482503794
+ );
+ assert_eq!(
+ parse_decimal::<Decimal32Type>(
+
"+18232335063972188138031550982650807591758238.0724251287782783777442440E-58",
+ 1,
+ 0
+ )
+ .unwrap(),
+ 0
+ );
+ assert_eq!(
+
parse_decimal::<Decimal128Type>("4825037936439135476.2609835314269495255615E-14",
9, 1)
+ .unwrap(),
+ 482504
+ );
+ assert!(
+
parse_decimal::<Decimal128Type>("4825037936439135476.2609835314269495255615E-14",
5, 1)
+ .is_err()
+ );
+ // Absurdly long exponents saturate rather than wrap
+ assert!(parse_decimal::<Decimal128Type>(&format!("1e{}",
"9".repeat(30)), 38, 0).is_err());
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(&format!("1e-{}", "9".repeat(30)),
38, 0).unwrap(),
+ 0
+ );
+ }
+
+ #[test]
+ fn test_parse_decimal_negative_scale() {
+ let tests = [
+ ("1234.5", -2, 12),
+ ("150", -2, 2),
+ ("149", -2, 1),
+ ("-150", -2, -2),
+ ("-149", -2, -1),
+ ("50", -2, 1),
+ ("49", -2, 0),
+ ("5", -1, 1),
+ ("4", -1, 0),
+ ("0.5", -1, 0),
+ ("5.9", -1, 1),
+ ("1e5", -2, 1000),
+ ("1.5e5", -2, 1500),
+ ("0.9e2", -1, 9),
+ (".5e3", -2, 5),
+ ("12345", -5, 0),
+ ("12345", -4, 1),
+ ("000123456", -3, 123),
+ ("0", -5, 0),
+ ("-0.0", -5, 0),
+ ];
+ for (s, scale, expected) in tests {
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(s, 38, scale).unwrap(),
+ expected,
+ "{s} at scale {scale}"
+ );
+ assert_eq!(
+ parse_decimal::<Decimal32Type>(s, 9, scale).unwrap(),
+ expected as i32,
+ "{s} at scale {scale}"
+ );
+ assert_eq!(
+ parse_decimal::<Decimal256Type>(s, 76, scale).unwrap(),
+ i256::from_i128(expected),
+ "{s} at scale {scale}"
+ );
+ }
+ // The integer part can be wider than the native type as long as the
+ // scaled value fits
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(&format!("1{}", "0".repeat(50)),
38, -40).unwrap(),
+ 10_000_000_000
+ );
+ assert_eq!(
+ parse_decimal::<Decimal32Type>("123456789012", 9, -5).unwrap(),
+ 1234568
+ );
+ assert!(parse_decimal::<Decimal32Type>("123456789012", 9,
-2).is_err());
+ }
+
+ #[test]
+ fn test_parse_decimal_whitespace_and_long_input() {
+ for s in [" 1.5", "1.5 ", " 1.5 ", "\t1.5\n", "\r\n1.5\x0c"] {
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(s, 38, 1).unwrap(),
+ 15,
+ "{s:?}"
+ );
+ }
+ // Only ASCII whitespace is trimmed, as for the other CSV parsers
+ assert!(parse_decimal::<Decimal128Type>("\u{a0}1.5", 38, 1).is_err());
+ assert!(parse_decimal::<Decimal128Type>("1.5\u{2003}", 38,
1).is_err());
+ assert!(parse_decimal::<Decimal128Type>(" ", 38, 1).is_err());
+
+ // Long inputs report overflow rather than wrapping or panicking
+ for s in [
+ "1".repeat(255),
+ "1".repeat(256),
+ "1".repeat(300),
+ format!("{}.5", "1".repeat(300)),
+ format!("1e{}", "9".repeat(300)),
+ ] {
+ let err = parse_decimal::<Decimal128Type>(&s, 38, 0).unwrap_err();
+ assert!(err.to_string().contains("does not fit"), "{err}");
+ }
+ // Long fractions only matter for rounding
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(&format!("0.{}", "0".repeat(200)),
38, 10).unwrap(),
+ 0
+ );
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(&format!("0.{}1",
"0".repeat(200)), 38, 10).unwrap(),
+ 0
+ );
+ assert_eq!(
+ parse_decimal::<Decimal128Type>(&format!("1.{}", "9".repeat(300)),
38, 2).unwrap(),
+ 200
+ );
+ // 10^scale overflows the native type, but zero is still representable
+ assert_eq!(parse_decimal::<Decimal32Type>("0", 9, 10).unwrap(), 0);
+ assert_eq!(parse_decimal::<Decimal32Type>("-0.0", 9, 10).unwrap(), 0);
+ assert_eq!(parse_decimal::<Decimal64Type>("0", 18, 20).unwrap(), 0);
+ assert_eq!(parse_decimal::<Decimal128Type>("0", 38, 40).unwrap(), 0);
+ assert!(parse_decimal::<Decimal32Type>("1", 9, 10).is_err());
+ }
+
+ #[test]
Review Comment:
```suggestion
#[test]
#[cfg_attr(miri, ignore)] // Takes too long
```
(took an hour on CI it seems)
--
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]
