Repository navigation
GH-41584: [Java] ListView Implementation for C Data Interface - #43686
Conversation
| } | ||
|
|
||
| return maxOffsetSizeSum - minOffset; | ||
| return maxOffsetSizeSum; |
There was a problem hiding this comment.
@lidavidm what I have done in the first implementation is incorrect, I think the test case I had didn't tackle this case, but the C data interface tests caught it. Sorry about that.
There was a problem hiding this comment.
This need a little bit of work... I am improving it.
github-actions
Bot
added
awaiting committer review
|
|
||
| protected int getLengthOfChildVector() { | ||
| /** | ||
| * Get the maximum view length of child vector This method deduces the length by considering the |
There was a problem hiding this comment.
I'm not sure this is the "view length" (length of an individual view) but more like just the end of the child vector in general?
There was a problem hiding this comment.
Right, the function name is correct but the docs are not precise. I will update.
There was a problem hiding this comment.
I updated the docs, does it look okay?
| protected int getLengthOfChildVector() { | ||
| /** | ||
| * Get the maximum view length of child vector This method deduces the length by considering the | ||
| * condition i.e., 0 <= offsets[i] + size[i] <= length of the child array. |
There was a problem hiding this comment.
And the condition is more like argmax_i offsets[i] + size[i]
There was a problem hiding this comment.
Updated this as well.
github-actions
Bot
added
awaiting changes
github-actions
Bot
added
awaiting change review
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit e61c105. 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. |
…pache#43686) ### Rationale for this change C Data Interface is missing `ListView` and `LargeListView` after recently merging core functionalities. Also closes; - [x] apache#41585 ### What changes are included in this PR? This PR includes C Data interface related component additions to `ListView` and `LargeListView` along with the corresponding test cases. ### Are these changes tested? Yes ### Are there any user-facing changes? No * GitHub Issue: apache#41584 Authored-by: Vibhatha Abeykoon <vibhatha@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>
Rationale for this change
C Data Interface is missing
ListViewandLargeListViewafter recently merging core functionalities.Also closes;
What changes are included in this PR?
This PR includes C Data interface related component additions to
ListViewandLargeListViewalong with the corresponding test cases.Are these changes tested?
Yes
Are there any user-facing changes?
No