Jefffrey commented on code in PR #7065:
URL: https://github.com/apache/arrow-rs/pull/7065#discussion_r1944361803
##########
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 think I understand what's happening now. It seems its an edge case of the
algorithm used here, where a 0 sized list element precedes a list of the proper
size, e.g.
```
Cast to fixed sized 2:
[
[],
[],
[1, 2],
]
```
Where `last_pos` is never updated from 0 because `end_pos` for the first two
elements is `0` and `last_pos` doesn't get updated when the list element
already matches the fixed size to cast to.
(Sorry if this was already obvious, just needed to air my thoughts out)
Whilst this fix is correct, I'd like some more documentation, and especially
rename `null_first` to something more accurate (as the problem is with empty
size lists too), to explain this edge case so it's obvious to anyone else
modifying the code here in the future.
We could also look into moving the:
```rust
if start_pos == 0 && end_pos == 0 {
```
Check outside of the for loop as we only need to check it once upfront.
--
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]
