Skip to content

Conversation

@dilipbiswal
Copy link
Contributor

Find out the missing attributes by recursively looking
at the sort order expression and rest of the code
takes care of projecting them out.

Added description from @cloud-fan

I wanna explain a bit more about this bug.

When we resolve sort ordering, we will use a special method, which only resolves UnresolvedAttributes and UnresolvedExtractValue. However, for something like Floor('a), even the 'a is resolved, the floor expression may still being unresolved as data type mismatch(for example, 'a is string type and Floor need double type), thus can't pass this filter, and we can't push down this missing attribute 'a

@dilipbiswal
Copy link
Contributor Author

@cloud-fan
Can you please take a look. Thanks ..

Copy link
Contributor

Choose a reason for hiding this comment

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

the test case can be simplified to:

    val a = testRelation2.output(0)
    val c = testRelation2.output(2)

    val plan = testRelation2.select('c).orderBy(Floor('a).asc)
    val expected = testRelation2.select(c, a).orderBy(Floor(a.cast(DoubleType)).asc).select(c)

    checkAnalysis(plan, expected)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a LOT. I will make the change.

@cloud-fan
Copy link
Contributor

I wanna explain a bit more about this bug.

When we resolve sort ordering, we will use a special method, which only resolves UnresolvedAttributes and UnresolvedExtractValue. However, for something like Floor('a), even the 'a is resolved, the floor expression may still being unresolved as data type mismatch(for example, 'a is string type and Floor need double type), thus can't pass this filter, and we can't push down this missing attribute 'a.

@cloud-fan
Copy link
Contributor

ok to test.

Copy link
Contributor

Choose a reason for hiding this comment

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

how about
val requiredAttributes = AttributeSet(newOrdering).filter(_.resolved)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Much simpler :-). Will make the change and test..

@dilipbiswal
Copy link
Contributor Author

@cloud-fan
Hi Wenchen, i have fixed the code based on your comments.

@SparkQA
Copy link

SparkQA commented Oct 15, 2015

Test build #43776 has finished for PR 9123 at commit ac78af1.

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

@SparkQA
Copy link

SparkQA commented Oct 15, 2015

Test build #43778 has finished for PR 9123 at commit 6f97fb7.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

remove these 2 extra blank lines please.

Copy link
Contributor

Choose a reason for hiding this comment

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

use 'c and 'a instead of c and a. The plan will get analyzed in the method checkAnalysis, so we should give unresolved attributes, to simulate normal query plans. (FYI, in spark SQL DSL, the symbol 'a will be automatically turned into UnresolvedAttribute)

@cloud-fan
Copy link
Contributor

hi @dilipbiswal , can you update the PR title to make it a complete sentence(i.e. no ellipsis)? and can you also put my explaination of this bug in PR description? so that if someone look into the commit log in the future, they can easily understand what's going on here.

@dilipbiswal dilipbiswal changed the title [SPARK-10534][SQL] ORDER BY clause allows only columns that are present in S… [SPARK-10534][SQL] ORDER BY clause allows only columns that are present in the select projection list.. Oct 15, 2015
@dilipbiswal dilipbiswal changed the title [SPARK-10534][SQL] ORDER BY clause allows only columns that are present in the select projection list.. [SPARK-10534][SQL] ORDER BY clause allows only columns that are present in the select projection list Oct 15, 2015
@SparkQA
Copy link

SparkQA commented Oct 15, 2015

Test build #43798 has finished for PR 9123 at commit 4f162ed.

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

@cloud-fan
Copy link
Contributor

LGTM pending test.

@cloud-fan
Copy link
Contributor

retest this please.

@SparkQA
Copy link

SparkQA commented Oct 15, 2015

Test build #43796 has finished for PR 9123 at commit 9ed26d1.

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

@cloud-fan
Copy link
Contributor

cc @marmbrus

@SparkQA
Copy link

SparkQA commented Oct 15, 2015

Test build #43802 has finished for PR 9123 at commit 4f162ed.

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

@cloud-fan
Copy link
Contributor

ping @yhuai

@yhuai
Copy link
Contributor

yhuai commented Oct 21, 2015

Thank you! Merging to master and 1.5 branch.

asfgit pushed a commit that referenced this pull request Oct 21, 2015
…ent in the select projection list

Find out the missing attributes by recursively looking
at the sort order expression and rest of the code
takes care of projecting them out.

Added description from cloud-fan

I wanna explain a bit more about this bug.

When we resolve sort ordering, we will use a special method, which only resolves UnresolvedAttributes and UnresolvedExtractValue. However, for something like Floor('a), even the 'a is resolved, the floor expression may still being unresolved as data type mismatch(for example, 'a is string type and Floor need double type), thus can't pass this filter, and we can't push down this missing attribute 'a

Author: Dilip Biswal <dbiswal@us.ibm.com>

Closes #9123 from dilipbiswal/SPARK-10534.

(cherry picked from commit 49ea0e9)
Signed-off-by: Yin Huai <yhuai@databricks.com>
@asfgit asfgit closed this in 49ea0e9 Oct 21, 2015
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