Support casting from String to Decimal #3281
Conversation
| ); | ||
| assert_eq!( | ||
| Decimal128Type::format_decimal( | ||
| parse_string_to_decimal_native::<Decimal128Type>("0.12345", 2).unwrap(), |
There was a problem hiding this comment.
I recommend test coverage for the following cases (apologies if they are already covered but I didn't see them):
- trim, eg,
". 0.123" - Multiple leading zeros:
"000.123" - Multiple trailing zeros:
"12.234000" - empty string
"" - whitespace string
" " - Invalid
"4.4.5"
There was a problem hiding this comment.
Thanks. Added more test coverage including these cases and Nulls.
arrow-cast/src/cast.rs
Outdated
| #[test] | ||
| fn test_cast_large_utf8_to_decimal() { | ||
| let str_array = LargeStringArray::from(vec![ | ||
| "123.45", "1.2345", "0.12345", "0.1267", "1.263", "12345.0", "12345", |
There was a problem hiding this comment.
I recommend a test for an input array that has Nulls in it as well
| } | ||
|
|
||
| #[test] | ||
| fn test_cast_utf8_to_decimal128_overflow() { |
viirya
force-pushed
the
cast_string_to_decimal
branch
from
73b1e9e to
bd99688
Compare
|
Thanks @viirya cc @liukun4515 |
| ); | ||
| assert_eq!( | ||
| Decimal128Type::format_decimal( | ||
| parse_string_to_decimal_native::<Decimal128Type>(".1265", 2).unwrap(), |
| let d = decimal_number.div_wrapping(div); | ||
| let r = decimal_number.mod_wrapping(div); | ||
|
|
||
| // Round result |
|
Benchmark runs are scheduled for baseline = a973d39 and contender = 2ad0705. 2ad0705 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Can drop this after rebase on commit 2ad0705 "Support casting from String to Decimal (apache#3281)", first released in 30.0.0
Can drop this after rebase on commit 2ad0705 "Support casting from String to Decimal (apache#3281)", first released in 30.0.0
Can drop this after rebase on commit 2ad0705 "Support casting from String to Decimal (apache#3281)", first released in 30.0.0
Which issue does this PR close?
Closes #3280.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?