support cast signed numeric to decimal#1044
Conversation
liukun4515
changed the title
liukun4515
force-pushed
the
support_cast_decimal
branch
from
6d8e602 to
8713645
Compare
liukun4515
changed the title
liukun4515
force-pushed
the
support_cast_decimal
branch
from
8713645 to
a927859
Compare
|
@alamb PTAL |
Codecov Report
@@ Coverage Diff @@
## master #1044 +/- ##
==========================================
+ Coverage 82.30% 82.34% +0.03%
==========================================
Files 168 168
Lines 49046 49135 +89
==========================================
+ Hits 40368 40459 +91
+ Misses 8678 8676 -2
Continue to review full report at Codecov.
|
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @liukun4515 -- I looked at the code and it looks correct to me.
I think we need a little more test coverage but otherwise 👍
arrow/src/compute/kernels/cast.rs
Outdated
| } | ||
|
|
||
| // test i32 to decimal type | ||
| let array = Int32Array::from(vec![1, 2, 3, 4, 5]); |
There was a problem hiding this comment.
Rather than just copy / pasting the integer tests when the differ only in the array type, what do you think about making a look like
let input = vec![
Arc::new(Int8Array::from(vec![1, 2, 3, 4, 5])),
Arc::new(Int16Array::from(vec![1, 2, 3, 4, 5])),
...
]
And then testing that in a loop? It would also allow coverage of Int16Array more easily
There was a problem hiding this comment.
grate suggestion.
I will try it.
arrow/src/compute/kernels/cast.rs
Outdated
| assert!(!can_cast_types(&DataType::UInt64, &decimal_type)); | ||
|
|
||
| // test i8 to decimal type | ||
| let array = Int8Array::from(vec![1, 2, 3, 4, 5]); |
There was a problem hiding this comment.
I also think we should test Nulls here too
There was a problem hiding this comment.
add the Null element for test, please check.
|
@alamb PTAL again, If it looks good for you, please merge this pull request. |
liukun4515
force-pushed
the
support_cast_decimal
branch
from
951b3a9 to
f27dde4
Compare
|
@alamb I'm confused about the CI, this pull request is blocked by the CI. |
|
CI looks good for this one now |
* support cast signed numeric to decimal * add test for i8,i16,i32,i64,f32,f64 casted to decimal * change format of float64 * add none test; merge integer test together
* support cast signed numeric to decimal * add test for i8,i16,i32,i64,f32,f64 casted to decimal * change format of float64 * add none test; merge integer test together Co-authored-by: Kun Liu <[email protected]>
* support cast signed numeric to decimal * add test for i8,i16,i32,i64,f32,f64 casted to decimal * change format of float64 * add none test; merge integer test together Can drop this after rebase on commit 4b3d928, first released in 7.0.0
3 participants
Which issue does this PR close?
part of #1043
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?