Skip to content
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

[SPARK-37557][SQL] Replace object hash with sort aggregate if child is already sorted #34824

Closed
wants to merge 1 commit into from

Conversation

c21
Copy link
Contributor

@c21 c21 commented Dec 7, 2021

What changes were proposed in this pull request?

This is a follow up of #34702 (comment) , where we can replace object hash aggregate with sort aggregate as well. This PR is to handle object hash aggregate.

Why are the changes needed?

Increase coverage of rule by handling object hash aggregate as well.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Modified unit test in ReplaceHashWithSortAggSuite.scala to cover object hash aggregate (by using aggregate expression COLLECT_LIST).

@github-actions github-actions bot added the SQL label Dec 7, 2021
@c21
Copy link
Contributor Author

c21 commented Dec 7, 2021

@cloud-fan could you help take a look when you have time? Thanks.

@SparkQA
Copy link

SparkQA commented Dec 7, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50443/

@SparkQA
Copy link

SparkQA commented Dec 7, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50443/

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 41a940f Dec 7, 2021
@c21
Copy link
Contributor Author

c21 commented Dec 7, 2021

Thank you @cloud-fan for review!

@c21 c21 deleted the agg-rule-followup branch December 7, 2021 04:48
@SparkQA
Copy link

SparkQA commented Dec 7, 2021

Test build #145968 has finished for PR 34824 at commit 0046770.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants