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-13087][SQL] Fix group by function for sort based aggregation #11011

Closed
wants to merge 1 commit into from

Conversation

marmbrus
Copy link
Contributor

@marmbrus marmbrus commented Feb 2, 2016

It is not valid to call toAttribute on a NamedExpression unless we know for sure that the child produced that NamedExpression. The current code worked fine when the grouping expressions were simple, but when they were a derived value this blew up at execution time.

@marmbrus
Copy link
Contributor Author

marmbrus commented Feb 2, 2016

/cc @cloud-fan

@marmbrus
Copy link
Contributor Author

marmbrus commented Feb 2, 2016

I'm investigating if this is a problem in master and will open another PR if needed.

@marmbrus
Copy link
Contributor Author

marmbrus commented Feb 2, 2016

Opened #11013 for master

@SparkQA
Copy link

SparkQA commented Feb 2, 2016

Test build #50519 has finished for PR 11011 at commit e605a09.

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

@cloud-fan
Copy link
Contributor

good catch! I think it's a mistake when we write planAggregateWithoutPartial, there is no intermediate layer and we can't assume the child will produce that NamedExpression.

LGTM

@yhuai
Copy link
Contributor

yhuai commented Feb 2, 2016

Merging to branch 1.6.

asfgit pushed a commit that referenced this pull request Feb 2, 2016
It is not valid to call `toAttribute` on a `NamedExpression` unless we know for sure that the child produced that `NamedExpression`.  The current code worked fine when the grouping expressions were simple, but when they were a derived value this blew up at execution time.

Author: Michael Armbrust <michael@databricks.com>

Closes #11011 from marmbrus/groupByFunction.
@yhuai
Copy link
Contributor

yhuai commented Feb 2, 2016

Merged!

@marmbrus marmbrus closed this Feb 2, 2016
@marmbrus marmbrus deleted the groupByFunction branch March 8, 2016 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants