Fix the behavior of from_fixed_size_list when offset > 0
#1964
Conversation
Signed-off-by: remzi <[email protected]>
| #[test] | ||
| fn test_decimal_array_from_fixed_size_list() { | ||
| let value_data = ArrayData::builder(DataType::UInt8) | ||
| .offset(16) |
There was a problem hiding this comment.
Add offset in the child array
| let list_data = ArrayData::builder(list_data_type) | ||
| .len(2) | ||
| .null_bit_buffer(Some(null_buffer)) | ||
| .offset(1) |
There was a problem hiding this comment.
Add offset in the list array
|
|
||
| assert_eq!(decimal.len(), 2); | ||
| assert!(decimal.is_null(0)); | ||
| assert_eq!(decimal.value_as_string(1), "56".to_string()); |
There was a problem hiding this comment.
I think this is the expected result, because slicing a list array doesn't push down the offset to child
Codecov Report
@@ Coverage Diff @@
## master #1964 +/- ##
==========================================
+ Coverage 83.47% 83.48% +0.01%
==========================================
Files 221 221
Lines 57054 57114 +60
==========================================
+ Hits 47626 47682 +56
- Misses 9428 9432 +4
Continue to review full report at Codecov.
|
| @@ -831,22 +831,26 @@ impl DecimalArray { | |||
| precision: usize, | |||
| scale: usize, | |||
| ) -> Self { | |||
| let child_data = &v.data_ref().child_data()[0]; | |||
There was a problem hiding this comment.
Somewhat tangential to this PR, but what happens to the child data's null buffer? Perhaps worth a docstring saying it is ignored?
There was a problem hiding this comment.
Great Catch!
I am not sure whether we should drop it or do list_nulls & child_nulls.
That's why I want to drop this function and build decimal array from FixedSizeBinary array instead.
Co-authored-by: Raphael Taylor-Davies <[email protected]>
Signed-off-by: remzi [email protected]
Which issue does this PR close?
Closes #1958 .
Rationale for this change
Consider the impact of the offset of the list array (also the offset of the child array) when building decimal array.
What changes are included in this PR?
Are there any user-facing changes?