[SPARK-14128][SQL] Alter table DDL followup
## What changes were proposed in this pull request? This is just a followup to #12121, which implemented the alter table DDLs using the `SessionCatalog`. Specially, this corrects the behavior of setting the location of a datasource table. For datasource tables, we need to set the `locationUri` in addition to the `path` entry in the serde properties. Additionally, changing the location of a datasource table partition is not allowed. ## How was this patch tested? `DDLSuite` Author: Andrew Or <andrew@databricks.com> Closes #12186 from andrewor14/alter-table-ddl-followup.
This commit is contained in:
parent
f6456fa80b
commit
adbfdb878d
|
@ -383,8 +383,9 @@ case class AlterTableSetLocation(
|
||||||
val part = catalog.getPartition(tableName, spec)
|
val part = catalog.getPartition(tableName, spec)
|
||||||
val newPart =
|
val newPart =
|
||||||
if (DDLUtils.isDatasourceTable(table)) {
|
if (DDLUtils.isDatasourceTable(table)) {
|
||||||
part.copy(storage = part.storage.copy(
|
throw new AnalysisException(
|
||||||
serdeProperties = part.storage.serdeProperties ++ Map("path" -> location)))
|
"alter table set location for partition is not allowed for tables defined " +
|
||||||
|
"using the datasource API")
|
||||||
} else {
|
} else {
|
||||||
part.copy(storage = part.storage.copy(locationUri = Some(location)))
|
part.copy(storage = part.storage.copy(locationUri = Some(location)))
|
||||||
}
|
}
|
||||||
|
@ -394,6 +395,7 @@ case class AlterTableSetLocation(
|
||||||
val newTable =
|
val newTable =
|
||||||
if (DDLUtils.isDatasourceTable(table)) {
|
if (DDLUtils.isDatasourceTable(table)) {
|
||||||
table.withNewStorage(
|
table.withNewStorage(
|
||||||
|
locationUri = Some(location),
|
||||||
serdeProperties = table.storage.serdeProperties ++ Map("path" -> location))
|
serdeProperties = table.storage.serdeProperties ++ Map("path" -> location))
|
||||||
} else {
|
} else {
|
||||||
table.withNewStorage(locationUri = Some(location))
|
table.withNewStorage(locationUri = Some(location))
|
||||||
|
|
|
@ -417,23 +417,37 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
|
||||||
.map { s => catalog.getPartition(tableIdent, s).storage }
|
.map { s => catalog.getPartition(tableIdent, s).storage }
|
||||||
.getOrElse { catalog.getTable(tableIdent).storage }
|
.getOrElse { catalog.getTable(tableIdent).storage }
|
||||||
if (isDatasourceTable) {
|
if (isDatasourceTable) {
|
||||||
assert(storageFormat.serdeProperties.get("path") === Some(expected))
|
if (spec.isDefined) {
|
||||||
|
assert(storageFormat.serdeProperties.isEmpty)
|
||||||
|
assert(storageFormat.locationUri.isEmpty)
|
||||||
|
} else {
|
||||||
|
assert(storageFormat.serdeProperties.get("path") === Some(expected))
|
||||||
|
assert(storageFormat.locationUri === Some(expected))
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
assert(storageFormat.locationUri === Some(expected))
|
assert(storageFormat.locationUri === Some(expected))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Optionally expect AnalysisException
|
||||||
|
def maybeWrapException[T](expectException: Boolean)(body: => T): Unit = {
|
||||||
|
if (expectException) intercept[AnalysisException] { body } else body
|
||||||
|
}
|
||||||
// set table location
|
// set table location
|
||||||
sql("ALTER TABLE dbx.tab1 SET LOCATION '/path/to/your/lovely/heart'")
|
sql("ALTER TABLE dbx.tab1 SET LOCATION '/path/to/your/lovely/heart'")
|
||||||
verifyLocation("/path/to/your/lovely/heart")
|
verifyLocation("/path/to/your/lovely/heart")
|
||||||
// set table partition location
|
// set table partition location
|
||||||
sql("ALTER TABLE dbx.tab1 PARTITION (a='1') SET LOCATION '/path/to/part/ways'")
|
maybeWrapException(isDatasourceTable) {
|
||||||
|
sql("ALTER TABLE dbx.tab1 PARTITION (a='1') SET LOCATION '/path/to/part/ways'")
|
||||||
|
}
|
||||||
verifyLocation("/path/to/part/ways", Some(partSpec))
|
verifyLocation("/path/to/part/ways", Some(partSpec))
|
||||||
// set table location without explicitly specifying database
|
// set table location without explicitly specifying database
|
||||||
catalog.setCurrentDatabase("dbx")
|
catalog.setCurrentDatabase("dbx")
|
||||||
sql("ALTER TABLE tab1 SET LOCATION '/swanky/steak/place'")
|
sql("ALTER TABLE tab1 SET LOCATION '/swanky/steak/place'")
|
||||||
verifyLocation("/swanky/steak/place")
|
verifyLocation("/swanky/steak/place")
|
||||||
// set table partition location without explicitly specifying database
|
// set table partition location without explicitly specifying database
|
||||||
sql("ALTER TABLE tab1 PARTITION (a='1') SET LOCATION 'vienna'")
|
maybeWrapException(isDatasourceTable) {
|
||||||
|
sql("ALTER TABLE tab1 PARTITION (a='1') SET LOCATION 'vienna'")
|
||||||
|
}
|
||||||
verifyLocation("vienna", Some(partSpec))
|
verifyLocation("vienna", Some(partSpec))
|
||||||
// table to alter does not exist
|
// table to alter does not exist
|
||||||
intercept[AnalysisException] {
|
intercept[AnalysisException] {
|
||||||
|
|
Loading…
Reference in a new issue