[SPARK-4322][SQL] Enables struct fields as sub expressions of grouping fields

[liancheng]

While resolving struct fields, the resulted GetField expression is wrapped with an Alias to make it a named expression. Assume a is a struct instance with a field b, then "a.b" will be resolved as Alias(GetField(a, "b"), "b"). Thus, for this following SQL query:

SELECT a.b + 1 FROM t GROUP BY a.b + 1

the grouping expression is

Add(GetField(a, "b"), Literal(1, IntegerType))

while the aggregation expression is

Add(Alias(GetField(a, "b"), "b"), Literal(1, IntegerType))

This mismatch makes the above SQL query fail during both analysis and execution phases. This PR fixes this issue by removing the alias when substituting aggregation expressions.

[SparkQA]

Test build #23314 has started for PR 3248 at commit 7f46532.

• This patch merges cleanly.

[SparkQA]

Test build #23314 has finished for PR 3248 at commit 7f46532.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

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

[SparkQA]

Test build #23365 has started for PR 3248 at commit dd20a79.

• This patch merges cleanly.

[liancheng]

Only aliases around GetField should be trimmed. The reason of the last build failure is that some necessary aliases are trimmed by mistake.

[SparkQA]

Test build #23366 has started for PR 3248 at commit 23a46ea.

• This patch merges cleanly.

[SparkQA]

Test build #23365 has finished for PR 3248 at commit dd20a79.

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

[AmplabJenkins]

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

[SparkQA]

Test build #23366 has finished for PR 3248 at commit 23a46ea.

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

[AmplabJenkins]

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

[marmbrus]

An earlier version of catalyst actually did something similar to having GetField be a named expression. However, this complicated a lot of things. In particular, it made recursive schemas impossible and it is a little tricky to have non-leaf nodes that have to have expression ids.

That said, I'm not opposed to this idea in general, but I think it might be a fair amount of work to realize. We should possible evaluate a few options as the current situation also seems less than ideal.

I'd propose we merge this now to fix the bug and investigate after 1.2.

[marmbrus]

Thanks for working on this! I went ahead and merged to master and 1.2, but lets revisit the idea of what to do about GetField calls once the release is out the door.