[Variant]: Implement DataType::List/LargeList support for cast_to_variant kernel #8201
Conversation
…arge_list_to_variant
…arge_list_to_variant
…arge_list_to_variant
| } | ||
| DataType::List(_) => { | ||
| let list_array = input.as_list::<i32>(); | ||
| let values_variant_array = cast_to_variant(list_array.values().as_ref())?; |
There was a problem hiding this comment.
as a future optimization, we could avoid casting the entire values to variant if the list only refers to a subset
For example, if the list array is sliced, then start will not be zero, so we could avoid casting the all the elements of the list
The same comment applies below
There was a problem hiding this comment.
Should we do it in a separate issue or I can try to implement it here?
I implemented it here. See below.
…kl/arrow-rs into list/large_list_to_variant
| let values = list_array.values(); | ||
| let offsets = list_array.offsets(); | ||
|
|
||
| let first_offset = offsets.first().expect("There should be an offset"); | ||
| let length = offsets.last().expect("There should be an offset") - first_offset; | ||
| let sliced_values = values.slice(*first_offset as usize, length as usize); | ||
|
|
||
| let values_variant_array = cast_to_variant(sliced_values.as_ref())?; | ||
| let new_offsets = OffsetBuffer::new(ScalarBuffer::from_iter( | ||
| offsets.iter().map(|o| o - first_offset), | ||
| )); |
There was a problem hiding this comment.
I implemented the optimization you mentioned above like this.
I also added a test for sliced list arrays.
|
I think it also makes sense to move the logic out of list match arms into a help function that takes the offset type as a parameter. |
|
Thank you @sdf-jkl 🙏 |
Which issue does this PR close?
DataType::List/LargeListsupport forcast_to_variantkernel #8060.Rationale for this change
Need to implement
List,LargeListtypes support forcast_to_variantkernelWhat changes are included in this PR?
Added support for
List,LargeListincast_to_variantkernelAre these changes tested?
Yes, added unit tests
Are there any user-facing changes?
Yes, added changes to the
cast_to_variantkernel