Skip to content

[SPARK-15187] [SQL] Disallow Dropping Default Database #12962

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

[SPARK-15187] [SQL] Disallow Dropping Default Database #12962

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
20ebf8a
ban dropping default db
gatorsmile May 6, 2016
67c2ebc
address comments.
gatorsmile May 7, 2016
223ed36
try 1
gatorsmile May 9, 2016
87099f1
address comments
gatorsmile May 9, 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
3 changes: 3 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ class SessionCatalog(
}

def dropDatabase(db: String, ignoreIfNotExists: Boolean, cascade: Boolean): Unit = {
if (db == "default") {

@cloud-fan cloud-fan May 7, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how about case sensitivity?

@gatorsmile gatorsmile May 7, 2016 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You are asking a very great question! Actually, I am not very sure how it works.

First, the current code base is not considering case sensitivity in database names. That part is missing. I think we should create another function like what the function formatTableName is doing for all the cases. For example, formatDatabaseName.

Second, I saw we have a function formatTableName for formatting table names based on the configuration of CASE_SENSITIVE. However, the underlying Hive metastore is not case sensitive. See the document: https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL

My question is how to achieve the case sensitivity when using Hive metastore? also cc @yhuai

@gatorsmile gatorsmile May 7, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added test cases to show the differences between Hive metastore and InMemoryCatalog

@gatorsmile gatorsmile May 7, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now, I think we should issue an exception if users set CASE_SENSITIVE to true if users use Hive Metastore.

@gatorsmile gatorsmile May 7, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

So far, the best way I found is to override caseSensitiveAnalysis to false in conf of HiveSessionState.

  override lazy val conf: SQLConf = new SQLConf {
    override def caseSensitiveAnalysis: Boolean = false
  }

@gatorsmile gatorsmile May 9, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let me submit a PR to fix it. : )

throw new AnalysisException(s"Can not drop default database")
}
externalCatalog.dropDatabase(db, ignoreIfNotExists, cascade)
}

Expand Down
7 changes: 7 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -939,4 +939,11 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
Row("Usage: a ^ b - Bitwise exclusive OR.") :: Nil
)
}

test("drop default database") {
val message = intercept[AnalysisException] {
sql("DROP DATABASE default")
}.getMessage
assert(message.contains("Can not drop default database"))
}
}