[SPARK-15187] [SQL] Disallow Dropping Default Database #12962
Conversation
|
Test build #58015 has finished for PR 12962 at commit
|
| } | ||
|
|
||
| def dropDatabase(db: String, ignoreIfNotExists: Boolean, cascade: Boolean): Unit = { | ||
| if (db == "default") { |
There was a problem hiding this comment.
how about case sensitivity?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Added test cases to show the differences between Hive metastore and InMemoryCatalog
- InMemoryCatalog: https://github.com/gatorsmile/spark/blob/dropDefaultDB/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala#L943
- Hive Metastore:
https://github.com/gatorsmile/spark/blob/dropDefaultDB/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala#L500
There was a problem hiding this comment.
Now, I think we should issue an exception if users set CASE_SENSITIVE to true if users use Hive Metastore.
There was a problem hiding this comment.
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
}There was a problem hiding this comment.
Let me submit a PR to fix it. : )
|
Test build #58072 has finished for PR 12962 at commit
|
|
@cloud-fan #12993 resolves the issue you mentioned above. Will change this PR too for resolving the issues of database names by calling Thanks! |
|
retest this please |
|
LGTM |
|
To fix the test failure, we need to call |
|
Test build #58150 has finished for PR 12962 at commit
|
|
Test build #58179 has finished for PR 12962 at commit
|
|
@andrewor14 @cloud-fan The PR is ready for review. Thanks! |
#### What changes were proposed in this pull request? In Hive Metastore, dropping default database is not allowed. However, in `InMemoryCatalog`, this is allowed. This PR is to disallow users to drop default database. #### How was this patch tested? Previously, we already have a test case in HiveDDLSuite. Now, we also add the same one in DDLSuite Author: gatorsmile <[email protected]> Closes #12962 from gatorsmile/dropDefaultDB. (cherry picked from commit f453791) Signed-off-by: Wenchen Fan <[email protected]>
|
LGTM , merging to master and 2.0, thanks! |
4 participants
What changes were proposed in this pull request?
In Hive Metastore, dropping default database is not allowed. However, in
InMemoryCatalog, this is allowed.This PR is to disallow users to drop default database.
How was this patch tested?
Previously, we already have a test case in HiveDDLSuite. Now, we also add the same one in DDLSuite