[SPARK-5285][SQL] Removed GroupExpression in catalyst

[scwf]

Removed GroupExpression since we no need to have this wrapper

[SparkQA]

Test build #25676 has started for PR 4075 at commit 59d61bf.

• This patch merges cleanly.

[SparkQA]

Test build #25676 has finished for PR 4075 at commit 59d61bf.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class SparkListenerJobEnd(

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25676/
Test PASSed.

[scwf]

/cc @marmbrus

[marmbrus]

I don't think transformations recurse into Seq[Expression] do they? That was the point of the container here.

Seems there might be a whole in our unit tests that they still pass with the removal of this. /cc @chenghao-intel

[scwf]

yeah getit，i will check to add some unittests here

[chenghao-intel]

The GroupExpression is not used as transformation in Analyzer, but in Optimizer, that's why it still can pass the unit test. I should document this.

[scwf]

@chenghao-intel， do we need to add unit test fot this?

[marmbrus]

I think there still could be a unit test to make sure that things in GroupExpressions get optimized.

[scwf]

@marmbrus after my investigate i think it is very rare case we will optimize GroupExpressions.
Should our optimization cover sql such as SELECT a, b, count(*) FROM T1 GROUP BY a, b, 1+1 GROUPING SETS (1+1, a, (a, b), b, ())? if not maybe we do not need to optimize it, then the change of this PR is safe.

[marmbrus]

I don't really see the benefit of removing it. Transform should be able to walk all expression, even if there are no optimizations that apply today.

[scwf]

Yes, i agree. Then should i add the unit test i give above?

[chenghao-intel]

Well, actually it will be widely used in optimization.
e.g. SELECT MAX(a), MAX(b) FROM t1 GROUP BY a, b GROUPING SETS (a, b)
It will produces 2 GroupExpressions:
MAX(Literal(null)), MAX(b) and MAX(a), MAX(Literal(null)) and eventually it turns to
null, MAX(b) and MAX(a), null after being optimized.