You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by li...@apache.org on 2018/04/10 16:33:14 UTC
spark git commit: [SPARK-19724][SQL][FOLLOW-UP] Check location of
managed table when ignoreIfExists is true
Repository: spark
Updated Branches:
refs/heads/master 3323b156f -> e17965891
[SPARK-19724][SQL][FOLLOW-UP] Check location of managed table when ignoreIfExists is true
## What changes were proposed in this pull request?
In the PR #20886, I mistakenly check the table location only when `ignoreIfExists` is false, which was following the original deprecated PR.
That was wrong. When `ignoreIfExists` is true and the target table doesn't exist, we should also check the table location. In other word, **`ignoreIfExists` has nothing to do with table location validation**.
This is a follow-up PR to fix the mistake.
## How was this patch tested?
Add one unit test.
Author: Gengliang Wang <ge...@databricks.com>
Closes #21001 from gengliangwang/SPARK-19724-followup.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/e1796589
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/e1796589
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/e1796589
Branch: refs/heads/master
Commit: e179658914963de472120a81621396706584c949
Parents: 3323b15
Author: Gengliang Wang <ge...@databricks.com>
Authored: Tue Apr 10 09:33:09 2018 -0700
Committer: gatorsmile <ga...@gmail.com>
Committed: Tue Apr 10 09:33:09 2018 -0700
----------------------------------------------------------------------
.../spark/sql/catalyst/catalog/SessionCatalog.scala | 11 +++++++++--
.../sql/execution/command/createDataSourceTables.scala | 2 +-
.../apache/spark/sql/execution/command/DDLSuite.scala | 9 +++++++++
.../apache/spark/sql/hive/execution/HiveDDLSuite.scala | 2 +-
4 files changed, 20 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/e1796589/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
index 52ed89e..c390337 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
@@ -286,7 +286,10 @@ class SessionCatalog(
* Create a metastore table in the database specified in `tableDefinition`.
* If no such database is specified, create it in the current database.
*/
- def createTable(tableDefinition: CatalogTable, ignoreIfExists: Boolean): Unit = {
+ def createTable(
+ tableDefinition: CatalogTable,
+ ignoreIfExists: Boolean,
+ validateLocation: Boolean = true): Unit = {
val db = formatDatabaseName(tableDefinition.identifier.database.getOrElse(getCurrentDatabase))
val table = formatTableName(tableDefinition.identifier.table)
val tableIdentifier = TableIdentifier(table, Some(db))
@@ -305,7 +308,11 @@ class SessionCatalog(
}
requireDbExists(db)
- if (!ignoreIfExists) {
+ if (tableExists(newTableDefinition.identifier)) {
+ if (!ignoreIfExists) {
+ throw new TableAlreadyExistsException(db = db, table = table)
+ }
+ } else if (validateLocation) {
validateTableLocation(newTableDefinition)
}
externalCatalog.createTable(newTableDefinition, ignoreIfExists)
http://git-wip-us.apache.org/repos/asf/spark/blob/e1796589/sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala
----------------------------------------------------------------------
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala
index f7c3e9b..f6ef433 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala
@@ -182,7 +182,7 @@ case class CreateDataSourceTableAsSelectCommand(
// provider (for example, see org.apache.spark.sql.parquet.DefaultSource).
schema = result.schema)
// Table location is already validated. No need to check it again during table creation.
- sessionState.catalog.createTable(newTable, ignoreIfExists = true)
+ sessionState.catalog.createTable(newTable, ignoreIfExists = false, validateLocation = false)
result match {
case fs: HadoopFsRelation if table.partitionColumnNames.nonEmpty &&
http://git-wip-us.apache.org/repos/asf/spark/blob/e1796589/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
----------------------------------------------------------------------
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
index 4304d0b6..cbd7f9d 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
@@ -425,6 +425,15 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
sql(s"CREATE TABLE tab1 (col1 int, col2 string) USING ${dataSource}")
}.getMessage
assert(ex.contains(exMsgWithDefaultDB))
+
+ // Always check location of managed table, with or without (IF NOT EXISTS)
+ withTable("tab2") {
+ sql(s"CREATE TABLE tab2 (col1 int, col2 string) USING ${dataSource}")
+ ex = intercept[AnalysisException] {
+ sql(s"CREATE TABLE IF NOT EXISTS tab1 LIKE tab2")
+ }.getMessage
+ assert(ex.contains(exMsgWithDefaultDB))
+ }
} finally {
waitForTasksToFinish()
Utils.deleteRecursively(tableLoc)
http://git-wip-us.apache.org/repos/asf/spark/blob/e1796589/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
----------------------------------------------------------------------
diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
index db76ec9..c85db78 100644
--- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
@@ -1461,7 +1461,7 @@ class HiveDDLSuite
assert(e2.getMessage.contains(forbiddenPrefix + "foo"))
val e3 = intercept[AnalysisException] {
- sql(s"CREATE TABLE tbl (a INT) TBLPROPERTIES ('${forbiddenPrefix}foo'='anything')")
+ sql(s"CREATE TABLE tbl2 (a INT) TBLPROPERTIES ('${forbiddenPrefix}foo'='anything')")
}
assert(e3.getMessage.contains(forbiddenPrefix + "foo"))
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org