Skip to content

Conversation

@cloud-fan
Copy link
Contributor

this is based on #9844, with some bug fix and clean up.

The problems is that, normal operator should be resolved based on its child, but Sort operator can also be resolved based on its grandchild. So we have 3 rules that can resolve Sort: ResolveReferences, ResolveSortReferences(if grandchild is Project) and ResolveAggregateFunctions(if grandchild is Aggregate).
For example, select c1 as a , c2 as b from tab group by c1, c2 order by a, c2, we need to resolve a and c2 for Sort. Firstly a will be resolved in ResolveReferences based on its child, and when we reach ResolveAggregateFunctions, we will try to resolve both a and c2 based on its grandchild, but failed because a is not a legal aggregate expression.

whoever merge this PR, please give the credit to @dilipbiswal

@SparkQA
Copy link

SparkQA commented Nov 25, 2015

Test build #46670 has finished for PR 9961 at commit 8ad6897.

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

@cloud-fan
Copy link
Contributor Author

cc @marmbrus @yhuai

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent

@SparkQA
Copy link

SparkQA commented Nov 26, 2015

Test build #46723 has finished for PR 9961 at commit 9868b7e.

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

@marmbrus
Copy link
Contributor

Thanks, I'm going to merge this to master and 1.6.

asfgit pushed a commit that referenced this pull request Nov 26, 2015
…of aliases and real columns

this is based on #9844, with some bug fix and clean up.

The problems is that, normal operator should be resolved based on its child, but `Sort` operator can also be resolved based on its grandchild. So we have 3 rules that can resolve `Sort`: `ResolveReferences`, `ResolveSortReferences`(if grandchild is `Project`) and `ResolveAggregateFunctions`(if grandchild is `Aggregate`).
For example, `select c1 as a , c2 as b from tab group by c1, c2 order by a, c2`, we need to resolve `a` and `c2` for `Sort`. Firstly `a` will be resolved in `ResolveReferences` based on its child, and when we reach `ResolveAggregateFunctions`, we will try to resolve both `a` and `c2` based on its grandchild, but failed because `a` is not a legal aggregate expression.

whoever merge this PR, please give the credit to dilipbiswal

Author: Dilip Biswal <[email protected]>
Author: Wenchen Fan <[email protected]>

Closes #9961 from cloud-fan/sort.

(cherry picked from commit bc16a67)
Signed-off-by: Michael Armbrust <[email protected]>
@asfgit asfgit closed this in bc16a67 Nov 26, 2015
@dilipbiswal
Copy link
Contributor

@marmbrus @cloud-fan Thank you !!

@cloud-fan cloud-fan deleted the sort branch November 27, 2015 03:29
markhamstra pushed a commit to markhamstra/spark that referenced this pull request Dec 9, 2015
…of aliases and real columns

this is based on apache#9844, with some bug fix and clean up.

The problems is that, normal operator should be resolved based on its child, but `Sort` operator can also be resolved based on its grandchild. So we have 3 rules that can resolve `Sort`: `ResolveReferences`, `ResolveSortReferences`(if grandchild is `Project`) and `ResolveAggregateFunctions`(if grandchild is `Aggregate`).
For example, `select c1 as a , c2 as b from tab group by c1, c2 order by a, c2`, we need to resolve `a` and `c2` for `Sort`. Firstly `a` will be resolved in `ResolveReferences` based on its child, and when we reach `ResolveAggregateFunctions`, we will try to resolve both `a` and `c2` based on its grandchild, but failed because `a` is not a legal aggregate expression.

whoever merge this PR, please give the credit to dilipbiswal

Author: Dilip Biswal <[email protected]>
Author: Wenchen Fan <[email protected]>

Closes apache#9961 from cloud-fan/sort.
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.

5 participants