You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2016/09/15 06:43:21 UTC
spark git commit: [SPARK-17440][SPARK-17441] Fixed Multiple Bugs in
ALTER TABLE
Repository: spark
Updated Branches:
refs/heads/master bb3229436 -> 6a6adb167
[SPARK-17440][SPARK-17441] Fixed Multiple Bugs in ALTER TABLE
### What changes were proposed in this pull request?
For the following `ALTER TABLE` DDL, we should issue an exception when the target table is a `VIEW`:
```SQL
ALTER TABLE viewName SET LOCATION '/path/to/your/lovely/heart'
ALTER TABLE viewName SET SERDE 'whatever'
ALTER TABLE viewName SET SERDEPROPERTIES ('x' = 'y')
ALTER TABLE viewName PARTITION (a=1, b=2) SET SERDEPROPERTIES ('x' = 'y')
ALTER TABLE viewName ADD IF NOT EXISTS PARTITION (a='4', b='8')
ALTER TABLE viewName DROP IF EXISTS PARTITION (a='2')
ALTER TABLE viewName RECOVER PARTITIONS
ALTER TABLE viewName PARTITION (a='1', b='q') RENAME TO PARTITION (a='100', b='p')
```
In addition, `ALTER TABLE RENAME PARTITION` is unable to handle data source tables, just like the other `ALTER PARTITION` commands. We should issue an exception instead.
### How was this patch tested?
Added a few test cases.
Author: gatorsmile <ga...@gmail.com>
Closes #15004 from gatorsmile/altertable.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/6a6adb16
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/6a6adb16
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/6a6adb16
Branch: refs/heads/master
Commit: 6a6adb1673775df63a62270879eac70f5f8d7d75
Parents: bb32294
Author: gatorsmile <ga...@gmail.com>
Authored: Thu Sep 15 14:43:10 2016 +0800
Committer: Wenchen Fan <we...@databricks.com>
Committed: Thu Sep 15 14:43:10 2016 +0800
----------------------------------------------------------------------
.../spark/sql/execution/command/ddl.scala | 45 +++++++++----
.../spark/sql/execution/command/tables.scala | 4 +-
.../spark/sql/execution/command/DDLSuite.scala | 63 ++++++++++++++----
.../spark/sql/hive/execution/HiveDDLSuite.scala | 67 +++++++++++---------
4 files changed, 120 insertions(+), 59 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/6a6adb16/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
----------------------------------------------------------------------
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
index dcda2f8..c0ccdca 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
@@ -230,8 +230,8 @@ case class AlterTableSetPropertiesCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
- DDLUtils.verifyAlterTableType(catalog, tableName, isView)
val table = catalog.getTableMetadata(tableName)
+ DDLUtils.verifyAlterTableType(catalog, table, isView)
// This overrides old properties
val newTable = table.copy(properties = table.properties ++ properties)
catalog.alterTable(newTable)
@@ -258,8 +258,8 @@ case class AlterTableUnsetPropertiesCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
- DDLUtils.verifyAlterTableType(catalog, tableName, isView)
val table = catalog.getTableMetadata(tableName)
+ DDLUtils.verifyAlterTableType(catalog, table, isView)
if (!ifExists) {
propKeys.foreach { k =>
if (!table.properties.contains(k)) {
@@ -299,6 +299,7 @@ case class AlterTableSerDePropertiesCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
val table = catalog.getTableMetadata(tableName)
+ DDLUtils.verifyAlterTableType(catalog, table, isView = false)
// For datasource tables, disallow setting serde or specifying partition
if (partSpec.isDefined && DDLUtils.isDatasourceTable(table)) {
throw new AnalysisException("Operation not allowed: ALTER TABLE SET " +
@@ -348,6 +349,7 @@ case class AlterTableAddPartitionCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
val table = catalog.getTableMetadata(tableName)
+ DDLUtils.verifyAlterTableType(catalog, table, isView = false)
if (DDLUtils.isDatasourceTable(table)) {
throw new AnalysisException(
"ALTER TABLE ADD PARTITION is not allowed for tables defined using the datasource API")
@@ -377,7 +379,14 @@ case class AlterTableRenamePartitionCommand(
extends RunnableCommand {
override def run(sparkSession: SparkSession): Seq[Row] = {
- sparkSession.sessionState.catalog.renamePartitions(
+ val catalog = sparkSession.sessionState.catalog
+ val table = catalog.getTableMetadata(tableName)
+ if (DDLUtils.isDatasourceTable(table)) {
+ throw new AnalysisException(
+ "ALTER TABLE RENAME PARTITION is not allowed for tables defined using the datasource API")
+ }
+ DDLUtils.verifyAlterTableType(catalog, table, isView = false)
+ catalog.renamePartitions(
tableName, Seq(oldPartition), Seq(newPartition))
Seq.empty[Row]
}
@@ -408,6 +417,7 @@ case class AlterTableDropPartitionCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
val table = catalog.getTableMetadata(tableName)
+ DDLUtils.verifyAlterTableType(catalog, table, isView = false)
if (DDLUtils.isDatasourceTable(table)) {
throw new AnalysisException(
"ALTER TABLE DROP PARTITIONS is not allowed for tables defined using the datasource API")
@@ -469,6 +479,7 @@ case class AlterTableRecoverPartitionsCommand(
s"Operation not allowed: $cmd on temporary tables: $tableName")
}
val table = catalog.getTableMetadata(tableName)
+ DDLUtils.verifyAlterTableType(catalog, table, isView = false)
if (DDLUtils.isDatasourceTable(table)) {
throw new AnalysisException(
s"Operation not allowed: $cmd on datasource tables: $tableName")
@@ -644,6 +655,7 @@ case class AlterTableSetLocationCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
val table = catalog.getTableMetadata(tableName)
+ DDLUtils.verifyAlterTableType(catalog, table, isView = false)
partitionSpec match {
case Some(spec) =>
// Partition spec is specified, so we set the location only for this partition
@@ -682,19 +694,26 @@ object DDLUtils {
/**
* If the command ALTER VIEW is to alter a table or ALTER TABLE is to alter a view,
* issue an exception [[AnalysisException]].
+ *
+ * Note: temporary views can be altered by both ALTER VIEW and ALTER TABLE commands,
+ * since temporary views can be also created by CREATE TEMPORARY TABLE. In the future,
+ * when we decided to drop the support, we should disallow users to alter temporary views
+ * by ALTER TABLE.
*/
def verifyAlterTableType(
catalog: SessionCatalog,
- tableIdentifier: TableIdentifier,
+ tableMetadata: CatalogTable,
isView: Boolean): Unit = {
- catalog.getTableMetadataOption(tableIdentifier).map(_.tableType match {
- case CatalogTableType.VIEW if !isView =>
- throw new AnalysisException(
- "Cannot alter a view with ALTER TABLE. Please use ALTER VIEW instead")
- case o if o != CatalogTableType.VIEW && isView =>
- throw new AnalysisException(
- s"Cannot alter a table with ALTER VIEW. Please use ALTER TABLE instead")
- case _ =>
- })
+ if (!catalog.isTemporaryTable(tableMetadata.identifier)) {
+ tableMetadata.tableType match {
+ case CatalogTableType.VIEW if !isView =>
+ throw new AnalysisException(
+ "Cannot alter a view with ALTER TABLE. Please use ALTER VIEW instead")
+ case o if o != CatalogTableType.VIEW && isView =>
+ throw new AnalysisException(
+ s"Cannot alter a table with ALTER VIEW. Please use ALTER TABLE instead")
+ case _ =>
+ }
+ }
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/6a6adb16/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
----------------------------------------------------------------------
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
index 9fbcd48..60e6b5d 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
@@ -158,7 +158,8 @@ case class AlterTableRenameCommand(
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
- DDLUtils.verifyAlterTableType(catalog, oldName, isView)
+ val table = catalog.getTableMetadata(oldName)
+ DDLUtils.verifyAlterTableType(catalog, table, isView)
// If this is a temp view, just rename the view.
// Otherwise, if this is a real table, we also need to uncache and invalidate the table.
val isTemporary = catalog.isTemporaryTable(oldName)
@@ -177,7 +178,6 @@ case class AlterTableRenameCommand(
}
}
// For datasource tables, we also need to update the "path" serde property
- val table = catalog.getTableMetadata(oldName)
if (DDLUtils.isDatasourceTable(table) && table.tableType == CatalogTableType.MANAGED) {
val newPath = catalog.defaultTablePath(newTblName)
val newTable = table.withNewStorage(
http://git-wip-us.apache.org/repos/asf/spark/blob/6a6adb16/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 95672e0..4a17180 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
@@ -696,6 +696,18 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
assert(spark.table("teachers").collect().toSeq == df.collect().toSeq)
}
+ test("rename temporary table") {
+ withTempView("tab1", "tab2") {
+ spark.range(10).createOrReplaceTempView("tab1")
+ sql("ALTER TABLE tab1 RENAME TO tab2")
+ checkAnswer(spark.table("tab2"), spark.range(10).toDF())
+ intercept[NoSuchTableException] { spark.table("tab1") }
+ sql("ALTER VIEW tab2 RENAME TO tab1")
+ checkAnswer(spark.table("tab1"), spark.range(10).toDF())
+ intercept[NoSuchTableException] { spark.table("tab2") }
+ }
+ }
+
test("rename temporary table - destination table already exists") {
withTempView("tab1", "tab2") {
sql(
@@ -880,25 +892,16 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
test("alter table: rename partition") {
val catalog = spark.sessionState.catalog
val tableIdent = TableIdentifier("tab1", Some("dbx"))
- val part1 = Map("a" -> "1", "b" -> "q")
- val part2 = Map("a" -> "2", "b" -> "c")
- val part3 = Map("a" -> "3", "b" -> "p")
- createDatabase(catalog, "dbx")
- createTable(catalog, tableIdent)
- createTablePartition(catalog, part1, tableIdent)
- createTablePartition(catalog, part2, tableIdent)
- createTablePartition(catalog, part3, tableIdent)
- assert(catalog.listPartitions(tableIdent).map(_.spec).toSet ==
- Set(part1, part2, part3))
+ createPartitionedTable(tableIdent, isDatasourceTable = false)
sql("ALTER TABLE dbx.tab1 PARTITION (a='1', b='q') RENAME TO PARTITION (a='100', b='p')")
- sql("ALTER TABLE dbx.tab1 PARTITION (a='2', b='c') RENAME TO PARTITION (a='200', b='c')")
+ sql("ALTER TABLE dbx.tab1 PARTITION (a='2', b='c') RENAME TO PARTITION (a='20', b='c')")
assert(catalog.listPartitions(tableIdent).map(_.spec).toSet ==
- Set(Map("a" -> "100", "b" -> "p"), Map("a" -> "200", "b" -> "c"), part3))
+ Set(Map("a" -> "100", "b" -> "p"), Map("a" -> "20", "b" -> "c"), Map("a" -> "3", "b" -> "p")))
// rename without explicitly specifying database
catalog.setCurrentDatabase("dbx")
sql("ALTER TABLE tab1 PARTITION (a='100', b='p') RENAME TO PARTITION (a='10', b='p')")
assert(catalog.listPartitions(tableIdent).map(_.spec).toSet ==
- Set(Map("a" -> "10", "b" -> "p"), Map("a" -> "200", "b" -> "c"), part3))
+ Set(Map("a" -> "10", "b" -> "p"), Map("a" -> "20", "b" -> "c"), Map("a" -> "3", "b" -> "p")))
// table to alter does not exist
intercept[NoSuchTableException] {
sql("ALTER TABLE does_not_exist PARTITION (c='3') RENAME TO PARTITION (c='333')")
@@ -909,6 +912,38 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
}
}
+ test("alter table: rename partition (datasource table)") {
+ createPartitionedTable(TableIdentifier("tab1", Some("dbx")), isDatasourceTable = true)
+ val e = intercept[AnalysisException] {
+ sql("ALTER TABLE dbx.tab1 PARTITION (a='1', b='q') RENAME TO PARTITION (a='100', b='p')")
+ }.getMessage
+ assert(e.contains(
+ "ALTER TABLE RENAME PARTITION is not allowed for tables defined using the datasource API"))
+ // table to alter does not exist
+ intercept[NoSuchTableException] {
+ sql("ALTER TABLE does_not_exist PARTITION (c='3') RENAME TO PARTITION (c='333')")
+ }
+ }
+
+ private def createPartitionedTable(
+ tableIdent: TableIdentifier,
+ isDatasourceTable: Boolean): Unit = {
+ val catalog = spark.sessionState.catalog
+ val part1 = Map("a" -> "1", "b" -> "q")
+ val part2 = Map("a" -> "2", "b" -> "c")
+ val part3 = Map("a" -> "3", "b" -> "p")
+ createDatabase(catalog, "dbx")
+ createTable(catalog, tableIdent)
+ createTablePartition(catalog, part1, tableIdent)
+ createTablePartition(catalog, part2, tableIdent)
+ createTablePartition(catalog, part3, tableIdent)
+ assert(catalog.listPartitions(tableIdent).map(_.spec).toSet ==
+ Set(part1, part2, part3))
+ if (isDatasourceTable) {
+ convertToDatasourceTable(catalog, tableIdent)
+ }
+ }
+
test("show tables") {
withTempView("show1a", "show2b") {
sql(
@@ -1255,7 +1290,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
}
// table to alter does not exist
intercept[AnalysisException] {
- sql("ALTER TABLE does_not_exist SET SERDEPROPERTIES ('x' = 'y')")
+ sql("ALTER TABLE does_not_exist PARTITION (a=1, b=2) SET SERDEPROPERTIES ('x' = 'y')")
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/6a6adb16/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 3cba5b2..aa35a33 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
@@ -305,6 +305,16 @@ class HiveDDLSuite
}
}
+ private def assertErrorForAlterTableOnView(sqlText: String): Unit = {
+ val message = intercept[AnalysisException](sql(sqlText)).getMessage
+ assert(message.contains("Cannot alter a view with ALTER TABLE. Please use ALTER VIEW instead"))
+ }
+
+ private def assertErrorForAlterViewOnTable(sqlText: String): Unit = {
+ val message = intercept[AnalysisException](sql(sqlText)).getMessage
+ assert(message.contains("Cannot alter a table with ALTER VIEW. Please use ALTER TABLE instead"))
+ }
+
test("alter views and alter table - misuse") {
val tabName = "tab1"
withTable(tabName) {
@@ -317,45 +327,42 @@ class HiveDDLSuite
assert(catalog.tableExists(TableIdentifier(tabName)))
assert(catalog.tableExists(TableIdentifier(oldViewName)))
+ assert(!catalog.tableExists(TableIdentifier(newViewName)))
- var message = intercept[AnalysisException] {
- sql(s"ALTER VIEW $tabName RENAME TO $newViewName")
- }.getMessage
- assert(message.contains(
- "Cannot alter a table with ALTER VIEW. Please use ALTER TABLE instead"))
+ assertErrorForAlterViewOnTable(s"ALTER VIEW $tabName RENAME TO $newViewName")
- message = intercept[AnalysisException] {
- sql(s"ALTER VIEW $tabName SET TBLPROPERTIES ('p' = 'an')")
- }.getMessage
- assert(message.contains(
- "Cannot alter a table with ALTER VIEW. Please use ALTER TABLE instead"))
+ assertErrorForAlterTableOnView(s"ALTER TABLE $oldViewName RENAME TO $newViewName")
- message = intercept[AnalysisException] {
- sql(s"ALTER VIEW $tabName UNSET TBLPROPERTIES ('p')")
- }.getMessage
- assert(message.contains(
- "Cannot alter a table with ALTER VIEW. Please use ALTER TABLE instead"))
+ assertErrorForAlterViewOnTable(s"ALTER VIEW $tabName SET TBLPROPERTIES ('p' = 'an')")
- message = intercept[AnalysisException] {
- sql(s"ALTER TABLE $oldViewName RENAME TO $newViewName")
- }.getMessage
- assert(message.contains(
- "Cannot alter a view with ALTER TABLE. Please use ALTER VIEW instead"))
+ assertErrorForAlterTableOnView(s"ALTER TABLE $oldViewName SET TBLPROPERTIES ('p' = 'an')")
- message = intercept[AnalysisException] {
- sql(s"ALTER TABLE $oldViewName SET TBLPROPERTIES ('p' = 'an')")
- }.getMessage
- assert(message.contains(
- "Cannot alter a view with ALTER TABLE. Please use ALTER VIEW instead"))
+ assertErrorForAlterViewOnTable(s"ALTER VIEW $tabName UNSET TBLPROPERTIES ('p')")
- message = intercept[AnalysisException] {
- sql(s"ALTER TABLE $oldViewName UNSET TBLPROPERTIES ('p')")
- }.getMessage
- assert(message.contains(
- "Cannot alter a view with ALTER TABLE. Please use ALTER VIEW instead"))
+ assertErrorForAlterTableOnView(s"ALTER TABLE $oldViewName UNSET TBLPROPERTIES ('p')")
+
+ assertErrorForAlterTableOnView(s"ALTER TABLE $oldViewName SET LOCATION '/path/to/home'")
+
+ assertErrorForAlterTableOnView(s"ALTER TABLE $oldViewName SET SERDE 'whatever'")
+
+ assertErrorForAlterTableOnView(s"ALTER TABLE $oldViewName SET SERDEPROPERTIES ('x' = 'y')")
+
+ assertErrorForAlterTableOnView(
+ s"ALTER TABLE $oldViewName PARTITION (a=1, b=2) SET SERDEPROPERTIES ('x' = 'y')")
+
+ assertErrorForAlterTableOnView(
+ s"ALTER TABLE $oldViewName ADD IF NOT EXISTS PARTITION (a='4', b='8')")
+
+ assertErrorForAlterTableOnView(s"ALTER TABLE $oldViewName DROP IF EXISTS PARTITION (a='2')")
+
+ assertErrorForAlterTableOnView(s"ALTER TABLE $oldViewName RECOVER PARTITIONS")
+
+ assertErrorForAlterTableOnView(
+ s"ALTER TABLE $oldViewName PARTITION (a='1') RENAME TO PARTITION (a='100')")
assert(catalog.tableExists(TableIdentifier(tabName)))
assert(catalog.tableExists(TableIdentifier(oldViewName)))
+ assert(!catalog.tableExists(TableIdentifier(newViewName)))
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org