Skip to content

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

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
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

@liancheng liancheng commented Mar 22, 2015

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

AmplabJenkins commented Mar 22, 2015

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 Outdated
Copy link
Contributor

@rxin rxin Mar 22, 2015

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

AmplabJenkins commented Mar 23, 2015

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

liancheng commented Mar 23, 2015

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

AmplabJenkins commented Mar 23, 2015

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 <[email protected]>

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 <[email protected]>
@marmbrus
Copy link
Contributor

marmbrus commented Mar 24, 2015

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