GH-41272: [Java] LargeListViewVector Implementation #43516
Conversation
java/vector/src/main/java/org/apache/arrow/vector/TypeLayout.java
Outdated
Show resolved
Hide resolved
java/vector/src/main/java/org/apache/arrow/vector/complex/LargeListViewVector.java
Outdated
Show resolved
Hide resolved
java/vector/src/main/java/org/apache/arrow/vector/complex/LargeListViewVector.java
Outdated
Show resolved
Hide resolved
java/vector/src/main/java/org/apache/arrow/vector/complex/LargeListViewVector.java
Outdated
Show resolved
Hide resolved
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
|
@lidavidm Ready for another round of reviews. |
| @@ -53,6 +53,7 @@ private static void writeValue(FieldReader reader, FieldWriter writer) { | |||
| case LIST: | |||
| case LISTVIEW: | |||
| case LARGELIST: | |||
| case LARGELISTVIEW: | |||
There was a problem hiding this comment.
ListView is also referenced on lines 163, 187, 210. Do we need to add LargeListView there, too?
There was a problem hiding this comment.
Right, good point. Arrow Java legacy implementation doesn't seem to support 64-bit cases.The places where I dropped it are similar in that regard. But in case I have really missed, please correct me 😌
| this.parentContainer = null; | ||
| this.listVector = null; | ||
| this.listViewVector = null; | ||
| this.largeListVector = null; |
There was a problem hiding this comment.
ListView is also referenced on lines 279, 344, 389, 575. Do we need to add LargeListView, too?
There was a problem hiding this comment.
Same as the above point.
github-actions
bot
added
awaiting merge
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit b33f040. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
This PR includes the initial integration of
LargeListViewVector.What changes are included in this PR?
This PR includes the core functions associated with the
LargeListViewVector.Are these changes tested?
Yes.
Are there any user-facing changes?
No
LargeListViewVectorImplementation #41272