-
Notifications
You must be signed in to change notification settings - Fork 1k
Add cast support for (Large)ListView <-> (Large)List #8735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
3065079 to
36065f0
Compare
36065f0 to
5701fce
Compare
|
cc @connortsui20 in case you want to take a look at this casting code in datafusion, since you've done a bunch of related things in vortex. |
5701fce to
71e0c6c
Compare
| OffsetSize: OffsetSizeTrait, | ||
| { | ||
| let list = array.as_list::<OffsetSize>(); | ||
| let list_view: GenericListViewArray<OffsetSize> = list.clone().into(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses a new From impl, because I saw that was the pattern for other types into ListViewArray, but we don't do that for ListView to List. Just pointing out the inconsistency, but think it's fine.
| (ListView(list_from), List(list_to)) => { | ||
| can_cast_types(list_from.data_type(), list_to.data_type()) | ||
| } | ||
| (LargeListView(list_from), LargeList(list_to)) => { | ||
| can_cast_types(list_from.data_type(), list_to.data_type()) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to simplify this branch, will do
Which issue does this PR close?
Related to tracking issue #5375 for
ListView.Rationale for this change
We need cast support for
ListViewandLargeListView.Are these changes tested?
Yes