Jefffrey commented on code in PR #7065:
URL: https://github.com/apache/arrow-rs/pull/7065#discussion_r1940758109
##########
arrow-cast/src/cast/list.rs:
##########
@@ -88,11 +88,16 @@ where
let mut mutable = MutableArrayData::new(vec![&values], nullable, cap);
// The end position in values of the last incorrectly-sized list slice
let mut last_pos = 0;
+ let mut null_first = false;
Review Comment:
I don't believe it's about nulls being first, rather about a potentially
empty list being first. Consider this test case which also panics during cast:
```rust
#[test]
fn test456() {
let array = Arc::new(ListArray::from_iter_primitive::<Int64Type, _,
_>(vec![
Some(vec![]),
Some(vec![Some(1), Some(2)]),
])) as ArrayRef;
let data_type =
DataType::FixedSizeList(FieldRef::new(Field::new("item",
DataType::Int64, true)), 2);
let opt = CastOptions::default();
let r = cast_with_options(&array, &data_type, &opt).unwrap();
let fixed_array =
Arc::new(FixedSizeListArray::from_iter_primitive::<Int64Type, _, _>(
vec![None, Some(vec![Some(1), Some(2)])],
2,
)) as ArrayRef;
assert_eq!(*fixed_array, *r);
}
```
This distinction is important as nullability isn't determined by the gap
between offsets, rather it's determined by the null buffer.
##########
arrow-cast/src/cast/list.rs:
##########
@@ -112,16 +117,15 @@ where
}
}
- let values = match last_pos {
- 0 => array.values().slice(0, cap), // All slices were the correct
length
- _ => {
- if mutable.len() != cap {
- // Remaining slices were all correct length
- let remaining = cap - mutable.len();
- mutable.extend(0, last_pos, last_pos + remaining)
- }
- make_array(mutable.freeze())
+ let values = if last_pos == 0 && !null_first {
+ array.values().slice(0, cap) // All slices were the correct length
+ } else {
+ if mutable.len() != cap {
+ // Remaining slices were all correct length
+ let remaining = cap - mutable.len();
+ mutable.extend(0, last_pos, last_pos + remaining)
}
+ make_array(mutable.freeze())
};
Review Comment:
```suggestion
let values = match last_pos {
0 if !null_first => array.values().slice(0, cap), // All slices were
the correct length
_ => {
if mutable.len() != cap {
// Remaining slices were all correct length
let remaining = cap - mutable.len();
mutable.extend(0, last_pos, last_pos + remaining)
}
make_array(mutable.freeze())
}
};
```
nit: using guard in the match arm would reduce the diff happening here
--
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]
