feat: add ListView and LargeListView support to arrow-ord#9347
feat: add ListView and LargeListView support to arrow-ord#9347codephage2020 wants to merge 6 commits intoapache:mainfrom
Conversation
arrow-ord/src/comparison.rs
Outdated
| } | ||
|
|
||
| /// Checks if each value in the [`PrimitiveArray`] is contained in the corresponding [`GenericListViewArray`] element | ||
| pub fn in_list_view<T, OffsetSize>( |
There was a problem hiding this comment.
Personally I'm not sure about adding these kernels; I don't know if in_list() and in_list_utf8() are commonly used (they don't seem to be used in DataFusion for example) and they do seem a bit oddly implemented (in that the base version accepts only primitive, and the string version accepts only Utf8/LargeUtf8 and not Utf8View) 🤔
Perhaps we can hold off on implementing these unless there's a motivating usecase for them
There was a problem hiding this comment.
@Jefffrey Thank you! I completely agree that we should prioritize the core sorting functionality. I'll update the PR .
Regarding the API inconsistency you mentioned with in_list() and in_list_utf8(), I think that's a valid technical debt issue that should be discussed separately. Perhaps we should consider unifying these APIs or designing a more generic contains-style kernel?
There was a problem hiding this comment.
That would be a reasonable approach; I don't have any strong feelings on it (whether keep it as is or try improve it) other than not following in its footsteps 👍
Which issue does this PR close?
Closes #9341.
Rationale for this change
arrow-ord doesnot support ListView
What changes are included in this PR?
add ListView/LargeListView support to the arrow-ord module
Are these changes tested?
Yes, all changes are thoroughly tested
Are there any user-facing changes?
New APIs:
Enhanced Functionality: