Skip to content

Conversation

@gatorsmile
Copy link
Member

What changes were proposed in this pull request?

This PR is to address the comment: #12146 (diff). It removes the function isViewSupported from SessionCatalog. After the removal, we still can capture the user errors if users try to drop a table using DROP VIEW.

How was this patch tested?

Modified the existing test cases

// When ifExists is false, no exception is issued when the table does not exist.
// Instead, log it as an error message.
val objectName = if (isView) "View" else "Table"
logError(s"$objectName '${tableName.quotedString}' does not exist")
Copy link
Member Author

Choose a reason for hiding this comment

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

Why we need to check tableExists at the beginning?

Here, the reason is to avoid issuing a confusing message. Without the change, users might get a confusing error message table 'abc' does not exist, although they issue a command DROP VIEW.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just let SessionCatalog to log something like Table or View ${name.quotedString} does not exist?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, we might need to change a couple of messages for it. Let me try it. Thanks!

@SparkQA
Copy link

SparkQA commented Apr 10, 2016

Test build #55470 has finished for PR 12284 at commit 0e11331.

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

@gatorsmile
Copy link
Member Author

cc @yhuai @andrewor14 Thanks!

@SparkQA
Copy link

SparkQA commented Apr 10, 2016

Test build #55489 has finished for PR 12284 at commit 8296bc6.

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

@SparkQA
Copy link

SparkQA commented Apr 11, 2016

Test build #55495 has finished for PR 12284 at commit 9271a03.

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

@yhuai
Copy link
Contributor

yhuai commented Apr 11, 2016

Thanks. Merging to master.

@asfgit asfgit closed this in 9f838bd Apr 11, 2016
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.

3 participants