-
Notifications
You must be signed in to change notification settings - Fork 933
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
Add JNI for splitting groups in a table after groupby [skip-ci] #7954
Conversation
Along with its unit tests. Signed-off-by: Firestarman <[email protected]>
Codecov Report
@@ Coverage Diff @@
## branch-0.20 #7954 +/- ##
===============================================
+ Coverage 82.30% 82.71% +0.41%
===============================================
Files 101 103 +2
Lines 17053 17711 +658
===============================================
+ Hits 14035 14650 +615
- Misses 3018 3061 +43
Continue to review full report at Codecov.
|
rerun tests |
rerun tests |
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.
Overall the code looks good. My only other real concern is that there is no code reuse between the new API and the existing aggregations API.
Signed-off-by: Firestarman <[email protected]>
Now at least the new API and the existing 'aggregate()' are sharing the groupby options. Is it OK ? @revans2 |
Signed-off-by: Firestarman <[email protected]>
Signed-off-by: Firestarman <[email protected]>
@gpucibot merge |
This PR is to add an API named
contiguousSplitGroups
in JNI which will split the groups in a table after agroupby
operation, instead of executing an aggregate on each group, along with its unit tests.This API will be used by some Spark operators ( e.g. Python UDFs ) to process the data group by group.
Other changes:
AggregateOperation
toGroupByOperation
which sounds better, since it is retuned from exactly agroupby
call.GroupByOptions
which will be used by nativegroupby
to propably achieve a better performance.Signed-off-by: Firestarman [email protected]