From 9507b67885cb2f1e9014e3175f35c505874e7ccf Mon Sep 17 00:00:00 2001 From: Andrew Or Date: Wed, 11 May 2016 14:16:01 -0700 Subject: [PATCH 1/2] Specifying LOCATION implies it's an EXTERNAL table --- .../spark/sql/execution/SparkSqlParser.scala | 12 +++++++----- .../sql/execution/command/DDLCommandSuite.scala | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala index 87e6f9094daaf..bf6f1c5fc2213 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala @@ -745,11 +745,6 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { if (ctx.bucketSpec != null) { throw operationNotAllowed("CREATE TABLE ... CLUSTERED BY", ctx) } - val tableType = if (external) { - CatalogTableType.EXTERNAL - } else { - CatalogTableType.MANAGED - } val comment = Option(ctx.STRING).map(string) val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns) val cols = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns) @@ -787,6 +782,13 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { serde = rowStorage.serde.orElse(fileStorage.serde).orElse(defaultStorage.serde), compressed = false, serdeProperties = rowStorage.serdeProperties ++ fileStorage.serdeProperties) + // If location is defined, we'll assume this is an external table. + // Otherwise, we may accidentally delete existing data. + val tableType = if (external || location.isDefined) { + CatalogTableType.EXTERNAL + } else { + CatalogTableType.MANAGED + } // TODO support the sql text - have a proper location for this! val tableDesc = CatalogTable( diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala index a728ac3c8a42b..ea1af93136961 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala @@ -18,7 +18,8 @@ package org.apache.spark.sql.execution.command import org.apache.spark.sql.catalyst.TableIdentifier -import org.apache.spark.sql.catalyst.catalog.{FunctionResource, FunctionResourceType} +import org.apache.spark.sql.catalyst.catalog.{CatalogTableType, FunctionResource} +import org.apache.spark.sql.catalyst.catalog.FunctionResourceType import org.apache.spark.sql.catalyst.parser.ParseException import org.apache.spark.sql.catalyst.plans.PlanTest import org.apache.spark.sql.catalyst.plans.logical.Project @@ -211,6 +212,18 @@ class DDLCommandSuite extends PlanTest { comparePlans(parsed4, expected4) } + test("create table - location implies external") { + val query = "CREATE TABLE my_tab LOCATION '/something/anything'" + parser.parsePlan(query) match { + case ct: CreateTable => + assert(ct.table.tableType == CatalogTableType.EXTERNAL) + assert(ct.table.storage.locationUri == Some("/something/anything")) + case other => + fail(s"Expected to parse ${classOf[CreateTable].getClass.getName} from query," + + s"got ${other.getClass.getName}: $query") + } + } + // ALTER TABLE table_name RENAME TO new_table_name; // ALTER VIEW view_name RENAME TO new_view_name; test("alter table/view: rename table/view") { From a342224f168c8320d14ebf4c62b4b699100d3336 Mon Sep 17 00:00:00 2001 From: Andrew Or Date: Wed, 11 May 2016 15:26:27 -0700 Subject: [PATCH 2/2] Fix tests --- .../apache/spark/sql/hive/execution/HiveDDLSuite.scala | 8 +++----- .../apache/spark/sql/hive/execution/SQLQuerySuite.scala | 5 +---- 2 files changed, 4 insertions(+), 9 deletions(-) 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 8b60802b9123b..ae61322844c3b 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 @@ -72,7 +72,7 @@ class HiveDDLSuite } } - test("drop managed tables in default database") { + test("drop external tables in default database") { withTempDir { tmpDir => val tabName = "tab1" withTable(tabName) { @@ -88,13 +88,11 @@ class HiveDDLSuite val hiveTable = hiveContext.sessionState.catalog .getTableMetadata(TableIdentifier(tabName, Some("default"))) - // It is a managed table, although it uses external in SQL - assert(hiveTable.tableType == CatalogTableType.MANAGED) + assert(hiveTable.tableType == CatalogTableType.EXTERNAL) assert(tmpDir.listFiles.nonEmpty) sql(s"DROP TABLE $tabName") - // The data are deleted since the table type is not EXTERNAL - assert(tmpDir.listFiles == null) + assert(tmpDir.listFiles.nonEmpty) } } } diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala index 6ce5051cbd3ad..ac9a3930fd21b 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala @@ -1534,10 +1534,7 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton { assert(fs.listStatus(new Path(path, "part=1")).nonEmpty) sql("drop table test_table") - assert( - !fs.exists(path), - "Once a managed table has been dropped, " + - "dirs of this table should also have been deleted.") + assert(fs.exists(path), "This is an external table, so the data should not have been dropped") } test("SPARK-14981: DESC not supported for sorting columns") {