[SPARK-9241][SQL] Supporting multiple DISTINCT columns (2) - Rewriting Rule #9406
Conversation
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
Contributor
|
ok to test |
|
Test build #44915 has finished for PR 9406 at commit
|
hvanhovell
force-pushed
the
SPARK-9241-rewriter
branch
from
6139f47 to
1e705fe
Compare
|
Test build #45019 has finished for PR 9406 at commit
|
hvanhovell
force-pushed
the
SPARK-9241-rewriter
branch
from
1e705fe to
9be5b9d
Compare
|
Test build #45207 has finished for PR 9406 at commit
|
Contributor
Author
|
Hmmmm... this is a bit of a strange error. |
Contributor
Author
|
Jenkins retest this please |
Contributor
Author
|
Jenkins is not retesting... @marmbrus could you add me to the whitelist? |
|
Test build #45219 has finished for PR 9406 at commit
|
Contributor
There was a problem hiding this comment.
It would be really helpful if there was an example of what this rewrite looks like here.
Contributor
Author
There was a problem hiding this comment.
I'll add an example in the follow-up PR.
Contributor
|
@hvanhovell I have started to use this PR as the foundation of removing our old aggregation code path. |
Contributor
There was a problem hiding this comment.
Comment on what each tuple element is, or maybe even use a case class?
Contributor
Author
There was a problem hiding this comment.
I'll add documentation in a follow-up PR.
Contributor
|
Okay, I looked over this pretty quickly and it looks awesome. We need some tests and we are super close to me cutting a preview release. That said, I'd really like to include this in 1.6. Here is my proposal:
|
asfgit
pushed a commit
that referenced
this pull request
Nov 7, 2015
…g Rule The second PR for SPARK-9241, this adds support for multiple distinct columns to the new aggregation code path. This PR solves the multiple DISTINCT column problem by rewriting these Aggregates into an Expand-Aggregate-Aggregate combination. See the [JIRA ticket](https://issues.apache.org/jira/browse/SPARK-9241) for some information on this. The advantages over the - competing - [first PR](#9280) are: - This can use the faster TungstenAggregate code path. - It is impossible to OOM due to an ```OpenHashSet``` allocating to much memory. However, this will multiply the number of input rows by the number of distinct clauses (plus one), and puts a lot more memory pressure on the aggregation code path itself. The location of this Rule is a bit funny, and should probably change when the old aggregation path is changed. cc yhuai - Could you also tell me where to add tests for this? Author: Herman van Hovell <[email protected]> Closes #9406 from hvanhovell/SPARK-9241-rewriter. (cherry picked from commit 6d0ead3) Signed-off-by: Michael Armbrust <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Nov 7, 2015
This PR is a follow up for PR #9406. It adds more documentation to the rewriting rule, removes a redundant if expression in the non-distinct aggregation path and adds a multiple distinct test to the AggregationQuerySuite. cc yhuai marmbrus Author: Herman van Hovell <[email protected]> Closes #9541 from hvanhovell/SPARK-9241-followup. (cherry picked from commit ef36284) Signed-off-by: Yin Huai <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Nov 7, 2015
This PR is a follow up for PR #9406. It adds more documentation to the rewriting rule, removes a redundant if expression in the non-distinct aggregation path and adds a multiple distinct test to the AggregationQuerySuite. cc yhuai marmbrus Author: Herman van Hovell <[email protected]> Closes #9541 from hvanhovell/SPARK-9241-followup.
kiszk
pushed a commit
to kiszk/spark-gpu
that referenced
this pull request
Dec 26, 2015
This PR is a follow up for PR apache/spark#9406. It adds more documentation to the rewriting rule, removes a redundant if expression in the non-distinct aggregation path and adds a multiple distinct test to the AggregationQuerySuite. cc yhuai marmbrus Author: Herman van Hovell <[email protected]> Closes #9541 from hvanhovell/SPARK-9241-followup.
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.
The second PR for SPARK-9241, this adds support for multiple distinct columns to the new aggregation code path.
This PR solves the multiple DISTINCT column problem by rewriting these Aggregates into an Expand-Aggregate-Aggregate combination. See the JIRA ticket for some information on this. The advantages over the - competing - first PR are:
OpenHashSetallocating to much memory. However, this will multiply the number of input rows by the number of distinct clauses (plus one), and puts a lot more memory pressure on the aggregation code path itself.The location of this Rule is a bit funny, and should probably change when the old aggregation path is changed.
cc @yhuai - Could you also tell me where to add tests for this?