-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-12914] [SQL] generate aggregation with grouping keys #10855
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
Conversation
|
Test build #49829 has finished for PR 10855 at commit
|
ca9a772 to
a98bc05
Compare
|
Test build #49847 has finished for PR 10855 at commit
|
|
Test build #49855 has finished for PR 10855 at commit
|
|
Test build #49862 has finished for PR 10855 at commit
|
|
Test build #50060 has finished for PR 10855 at commit
|
|
Test build #50063 has finished for PR 10855 at commit
|
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.
Update comment.
Conflicts: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/UnsafeRow.java
|
Test build #50178 has finished for PR 10855 at commit
|
Conflicts: sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegen.scala sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/TungstenAggregate.scala sql/core/src/main/scala/org/apache/spark/sql/execution/basicOperators.scala
|
Test build #50307 has finished for PR 10855 at commit
|
|
Test build #50312 has finished for PR 10855 at commit
|
Conflicts: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/TungstenAggregate.scala
|
Test build #50389 has finished for PR 10855 at commit
|
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.
I find it weird you've split this up this way. Can you comment that this is only used when its grouping with no agg?
Why do you manage this in side the TungstenAggregation class but create the hashmap so differently? They seem logically serving the same purpose.
|
Test build #2475 has finished for PR 10855 at commit
|
|
@davies Can you include the generated output? |
This reverts commit 6237d97.
|
For query Will generate the code as following: |
|
Test build #50400 has finished for PR 10855 at commit
|
|
LGTM |
|
Test build #2477 has finished for PR 10855 at commit
|
|
Test build #50412 has finished for PR 10855 at commit
|
|
Test build #2479 has finished for PR 10855 at commit
|
|
Test build #2482 has finished for PR 10855 at commit
|
|
Test build #50420 has finished for PR 10855 at commit
|
|
Test build #2483 has finished for PR 10855 at commit
|
|
Merging this into master. |
This PR add support for grouping keys for generated TungstenAggregate.
Spilling and performance improvements for BytesToBytesMap will be done by followup PR.