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.

Sign up for GitHub

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

Jump to bottom

[SPARK-6452] [SQL] Checks for missing attributes and unresolved operator for all types of operator #5129

Closed
wants to merge 2 commits into from
Closed

[SPARK-6452] [SQL] Checks for missing attributes and unresolved operator for all types of operator #5129

wants to merge 2 commits into from

Conversation

liancheng
Copy link
Contributor

In CheckAnalysis, Filter and Aggregate are checked in separate case clauses, thus never hit those clauses for unresolved operators and missing input attributes.

This PR also removes the prettyString call when generating error message for missing input attributes. Because result of prettyString doesn't contain expression ID, and may give confusing messages like

resolved attributes a missing from a

cc @rxin

Review on Reviewable

@SparkQA
Copy link

SparkQA commented Mar 22, 2015

Test build #28965 has started for PR 5129 at commit 46facb9.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Mar 22, 2015

Test build #28965 timed out for PR 5129 at commit 46facb9 after a configured wait of 120m.

@AmplabJenkins
Copy link

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

rxin
rxin reviewed Mar 22, 2015
View reviewed changes
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala
@@ -199,4 +199,21 @@ class AnalysisSuite extends FunSuite with BeforeAndAfter {
assert(pl(3).dataType == DecimalType.Unlimited)
assert(pl(4).dataType == DoubleType)
}

test("SPARK-6452: CheckAnalysis should throw when Aggregate contains missing attributes") {
Copy link
Contributor

Choose a reason for hiding this comment

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

should throw AnalysisException ? otherwise it is an incomplete sentence

@liancheng liancheng mentioned this pull request Mar 23, 2015
Closed
@SparkQA
Copy link

SparkQA commented Mar 23, 2015

Test build #28983 has started for PR 5129 at commit 0c49010.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Mar 23, 2015

Test build #28983 has finished for PR 5129 at commit 0c49010.

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

@AmplabJenkins
Copy link

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

@SparkQA
Copy link

SparkQA commented Mar 23, 2015

Test build #28991 has started for PR 5129 at commit 52cdc69.

  • This patch merges cleanly.

@liancheng
Copy link
Contributor Author

Rebased after #5132 being merged. All comments are addressed. Thanks everyone for the review!

@SparkQA
Copy link

SparkQA commented Mar 23, 2015

Test build #28991 has finished for PR 5129 at commit 52cdc69.

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

@AmplabJenkins
Copy link

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

asfgit pushed a commit that referenced this pull request Mar 24, 2015
@liancheng @marmbrus
[SPARK-6452] [SQL] Checks for missing attributes and unresolved opera…
6f10142 
…tor for all types of operator

In `CheckAnalysis`, `Filter` and `Aggregate` are checked in separate case clauses, thus never hit those clauses for unresolved operators and missing input attributes.

This PR also removes the `prettyString` call when generating error message for missing input attributes. Because result of `prettyString` doesn't contain expression ID, and may give confusing messages like

> resolved attributes a missing from a

cc rxin

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/apache/spark/5129)
<!-- Reviewable:end -->

Author: Cheng Lian <lian@databricks.com>

Closes #5129 from liancheng/spark-6452 and squashes the following commits:

52cdc69 [Cheng Lian] Addresses comments
029f9bd [Cheng Lian] Checks for missing attributes and unresolved operator for all types of operator

(cherry picked from commit 1afcf77)
Signed-off-by: Michael Armbrust <michael@databricks.com>
@marmbrus
Copy link
Contributor

Thanks! Merged to master and 1.3

@asfgit asfgit closed this in 1afcf77 Mar 24, 2015
@liancheng liancheng deleted the spark-6452 branch March 24, 2015 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@liancheng @SparkQA @AmplabJenkins @marmbrus @rxin @chenghao-intel @cloud-fan @scwf