JNI bindings for NTH_ELEMENT window aggregation#11201
Merged
rapids-bot[bot] merged 15 commits intorapidsai:branch-22.08from Jul 8, 2022
Merged
JNI bindings for NTH_ELEMENT window aggregation#11201rapids-bot[bot] merged 15 commits intorapidsai:branch-22.08from
rapids-bot[bot] merged 15 commits intorapidsai:branch-22.08from
Conversation
This is to address spark-rapids/issues/4005 and spark-rapids/issues/5061. This change adds support for `NTH_ELEMENT` window aggregations. This should allow for the implementation of `FIRST()`, `LAST()`, and `NTH_VALUE()` window functions in Spark SQL. `NTH_ELEMENT` in window function returns the Nth element from the specified window for each row in a column. `N` is deemed to be zero based, so `NTH_ELEMENT(0)` translates to the first element in a window. Similarly, `NTH_ELEMENT(-1)` translates to the last. If for any window of size `W`, if the specified `N` falls outside the range `[ -W, W-1 ]`, a null element is returned for that row.
More readable.
This commit adds the JNI bindings required to invoke `NTH_ELEMENT` aggregations as a window aggregation. Java tests are included. Depends on rapidsai#11158.
github-actions
bot
added
CMake
mythrocks
added
feature request
Contributor
Author
Codecov Report
@@ Coverage Diff @@
## branch-22.08 #11201 +/- ##
===============================================
Coverage ? 86.30%
===============================================
Files ? 144
Lines ? 22698
Branches ? 0
===============================================
Hits ? 19589
Misses ? 3109
Partials ? 0 Continue to review full report at Codecov.
|
1. Using #pragma once. 2. Doxygen format. 3. Renamed gather map functor. 4. Materialized gather indices.
razajafri
reviewed
Jul 7, 2022
github-actions
bot
added
CMake
mythrocks
added
DO NOT MERGE
Contributor
Author
|
Setting |
rwlee
reviewed
Jul 7, 2022
Contributor
rwlee
left a comment
rwlee
left a comment
There was a problem hiding this comment.
LGTM pending dependency being merged. 1 formatting/whitespace nit
Also, switched to use gathered.front().
github-actions
bot
removed
CMake
Contributor
Author
|
@razajafri, @rwlee, I have addressed your concerns. I'd appreciate it if you could take another gander. |
rwlee
approved these changes
Jul 7, 2022
razajafri
approved these changes
Jul 8, 2022
Contributor
|
@gpucibot merge |
Contributor
Author
|
Thank you for the reviews, @rwlee, @razajafri. |
vyasr
added
4 - Needs Review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #11158.
This commit adds the JNI bindings required to invoke
NTH_ELEMENTaggregations as a window aggregation. Java tests are included.