Skip to content

Conversation

@hvanhovell
Copy link
Contributor

What changes were proposed in this pull request?

We currently have quite a few analysis exception subclasses, the problem with these is that they are not well supported throughout Spark (in pySpark for example) and that the added value is limited. This PR removes the no such object and the already exists exceptions, and replaces them by a factory methods that create analysis exceptions with the required error messages.

How was this patch tested?

Modified existing tests.

case _: NoSuchTableException =>
u.failAnalysis(s"Table or view not found: ${tableIdentWithDb.unquotedString}")
// If the database is defined and that database is not found, throw an AnalysisException.
if (!tableIdentWithDb.database.exists(catalog.databaseExists)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we should move this into catalog.lookupRelation and attach the location here.

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

try {
  catalog.lookupRelation(tableIdentWithDb)
} catch {
  case a: AnalysisException => u.failAnalysis(a.message)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, could work. This change is also causing the tests to fail, so I am revisiting it.

@hvanhovell
Copy link
Contributor Author

cc @cloud-fan @ueshin @rxin

@SparkQA
Copy link

SparkQA commented Apr 21, 2017

Test build #76031 has finished for PR 17716 at commit 6dc3204.

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

@SparkQA
Copy link

SparkQA commented Apr 21, 2017

Test build #76039 has finished for PR 17716 at commit 79ee4a6.

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


private def assertNoSuchTable(query: String): Unit = {
intercept[NoSuchTableException] {
intercept[AnalysisException] {
Copy link
Member

Choose a reason for hiding this comment

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

AnalysisException is a pretty general Exception in Spark SQL. The future refactoring might introduce bugs without trigger any test case failure.

Could we check the error messages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

@HyukjinKwon
Copy link
Member

@hvanhovell, how is it going?

@jiangxb1987
Copy link
Contributor

ping @hvanhovell Are you still working on this?

@asfgit asfgit closed this in b32bd00 Jun 27, 2017
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.

6 participants