Skip to content

[SPARK-17589] [TEST] [2.0] Fix test case create external table in MetastoreDataSourcesSuite #15145

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
gatorsmile wants to merge 2 commits into from
Closed

[SPARK-17589] [TEST] [2.0] Fix test case create external table in MetastoreDataSourcesSuite #15145

gatorsmile wants to merge 2 commits into from

Conversation

@gatorsmile
Copy link
Member

@gatorsmile gatorsmile commented Sep 19, 2016

What changes were proposed in this pull request?

This PR is to fix a test failure on the branch 2.0 builds:
https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-branch-2.0-test-maven-hadoop-2.7/711/

Error Message

"Table `default`.`createdJsonTable` already exists.;" did not contain "Table default.createdJsonTable already exists." We should complain that createdJsonTable already exists

How was this patch tested?

N/A

@gatorsmile
Copy link
Member Author

gatorsmile commented Sep 19, 2016

cc @cloud-fan @srowen

cloud-fan
cloud-fan reviewed Sep 19, 2016
View reviewed changes
sql/hive/src/test/scala/org/apache/spark/sql/hive/MetastoreDataSourcesSuite.scala Outdated
intercept[AnalysisException] {
sparkSession.catalog.createExternalTable("createdJsonTable", jsonFilePath.toString)
}.getMessage.contains("Table default.createdJsonTable already exists."),
}.getMessage.contains("Table `default`.`createdJsonTable` already exists."),
Copy link
Contributor

@cloud-fan cloud-fan Sep 19, 2016

Choose a reason for hiding this comment

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

I think we may generate wrong error message, can you take a look at CreateDatasourceCommand?

Copy link
Contributor

@cloud-fan cloud-fan Sep 19, 2016

Choose a reason for hiding this comment

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

we should use unquotedString there

Copy link
Member Author

@gatorsmile gatorsmile Sep 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I have a question about unquotedString. When should we issue an unquotedString? Unless we explicitly specify it, we always use quotedString for TableIdentifier. For FunctionIdentifier, we use an unquotedString.

Now, very few places are issuing unquotedString for table identifiers. Should we correct them to make all the messages consistent?

Copy link
Contributor

@cloud-fan cloud-fan Sep 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

The master branch also use unquotedString here, let's fix it first, and we can discuss about the choice between quotedString and unquotedString later

Copy link
Member Author

@gatorsmile gatorsmile Sep 19, 2016

Choose a reason for hiding this comment

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

Sure, will change it soon. Thanks!

@SparkQA
Copy link

SparkQA commented Sep 19, 2016

Test build #65583 has finished for PR 15145 at commit a7713de.

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

@yhuai
Copy link
Contributor

yhuai commented Sep 19, 2016

LGTM. Pending jenkins.

@SparkQA
Copy link

SparkQA commented Sep 19, 2016

Test build #65602 has finished for PR 15145 at commit a73599e.

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

@yhuai
Copy link
Contributor

yhuai commented Sep 19, 2016

Thanks @gatorsmile. I am merging this fix to branch 2.0.

asfgit pushed a commit that referenced this pull request Sep 19, 2016
@gatorsmile @yhuai
[SPARK-17589][TEST][2.0] Fix test case create external table in Met…
c4660d6 
…astoreDataSourcesSuite

### What changes were proposed in this pull request?
This PR is to fix a test failure on the branch 2.0 builds:
https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-branch-2.0-test-maven-hadoop-2.7/711/
```
Error Message

"Table `default`.`createdJsonTable` already exists.;" did not contain "Table default.createdJsonTable already exists." We should complain that createdJsonTable already exists
```

### How was this patch tested?
N/A

Author: gatorsmile <gatorsmile@gmail.com>

Closes #15145 from gatorsmile/fixTestCase.
@yhuai
Copy link
Contributor

yhuai commented Sep 19, 2016

Merged.

@gatorsmile
Copy link
Member Author

gatorsmile commented Sep 19, 2016

Thanks! Let me close it

@gatorsmile gatorsmile closed this Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cloud-fan cloud-fan cloud-fan left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gatorsmile @SparkQA @yhuai @cloud-fan