Skip to content

[SPARK-18097][SQL] Add exception catch to handle corrupted metadata scenario while dropping the table #16083

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

Closed
jayadevanmurali wants to merge 5 commits into from
Closed

[SPARK-18097][SQL] Add exception catch to handle corrupted metadata scenario while dropping the table #16083

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
290b86b
Merge pull request #1 from apache/master
jayadevanmurali Jan 31, 2016
9a7f87f
Merge remote-tracking branch 'upstream/master'
jayadevanmurali May 10, 2016
d8ed586
Merge remote-tracking branch 'upstream/master'
jayadevanmurali Oct 24, 2016
cffbec2
Merge remote-tracking branch 'upstream/master'
jayadevanmurali Nov 26, 2016
14bc0b9
updated ddl.scala
jayadevanmurali Nov 30, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,18 @@ case class DropTableCommand(
if (!catalog.isTemporaryTable(tableName) && catalog.tableExists(tableName)) {
// If the command DROP VIEW is to drop a table or DROP TABLE is to drop a view
// issue an exception.
catalog.getTableMetadata(tableName).tableType match {
case CatalogTableType.VIEW if !isView =>
throw new AnalysisException(
"Cannot drop a view with DROP TABLE. Please use DROP VIEW instead")
case o if o != CatalogTableType.VIEW && isView =>
throw new AnalysisException(
s"Cannot drop a table with DROP VIEW. Please use DROP TABLE instead")
case _ =>
try {
catalog.getTableMetadata(tableName).tableType match {
case CatalogTableType.VIEW if !isView =>
throw new AnalysisException(
"Cannot drop a view with DROP TABLE. Please use DROP VIEW instead")
case o if o != CatalogTableType.VIEW && isView =>
throw new AnalysisException(
s"Cannot drop a table with DROP VIEW. Please use DROP TABLE instead")
case _ =>
}
} catch {
case e: QueryExecutionException => log.warn(e.toString, e)
Copy link
Member

@gatorsmile gatorsmile Nov 30, 2016

Choose a reason for hiding this comment

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

How did you test this?

Copy link
Contributor Author

@jayadevanmurali jayadevanmurali Dec 1, 2016

Choose a reason for hiding this comment

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

Hi @gatorsmile, I tested this change against all existing test cases. Also @davies mentioned in the jira itself, we cannot purposefully corrupt the meta data. So I tested and verified, this change will not break any existing test cases. And we are expecting QueryExecutionException when the meta data is get corrupted in some way.

Copy link
Member

@gatorsmile gatorsmile Dec 1, 2016

Choose a reason for hiding this comment

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

The existing test cases do not cover this case. I am afraid this might not work as expected. We need to test it.

Copy link
Contributor

@thomastechs thomastechs Dec 1, 2016

Choose a reason for hiding this comment

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

@gatorsmile and @davies ; In that case, any suggestions to mock the metadata corrupt scenario?

Copy link
Member

@gatorsmile gatorsmile Dec 2, 2016

Choose a reason for hiding this comment

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

I do not know why we can hit QueryExecutionException in getTableMetadata.

Copy link
Member

@gatorsmile gatorsmile Dec 2, 2016

Choose a reason for hiding this comment

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

Maybe you can manually modify the metadata stored in the database and check whether it fails as expected.

}
}
try {
Expand Down