Skip to content

Commit

Permalink
[SPARK-29498][SQL] CatalogTable to HiveTable should not change the ta…
Browse files Browse the repository at this point in the history
…ble's ownership

### What changes were proposed in this pull request?

`CatalogTable` to `HiveTable` will change the table's ownership. How to reproduce:
```scala
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable, CatalogTableType}
import org.apache.spark.sql.types.{LongType, StructType}

val identifier = TableIdentifier("spark_29498", None)
val owner = "SPARK-29498"
val newTable = CatalogTable(
  identifier,
  tableType = CatalogTableType.EXTERNAL,
  storage = CatalogStorageFormat(
    locationUri = None,
    inputFormat = None,
    outputFormat = None,
    serde = None,
    compressed = false,
    properties = Map.empty),
  owner = owner,
  schema = new StructType().add("i", LongType, false),
  provider = Some("hive"))

spark.sessionState.catalog.createTable(newTable, false)
// The owner is not SPARK-29498
println(spark.sessionState.catalog.getTableMetadata(identifier).owner)
```

This PR makes it set the `HiveTable`'s owner to `CatalogTable`'s owner if it's owner is not empty when converting `CatalogTable` to `HiveTable`.

### Why are the changes needed?
We should not change the ownership of the table when converting `CatalogTable` to `HiveTable`.

### Does this PR introduce any user-facing change?
No

### How was this patch tested?
unit test

Closes #26160 from wangyum/SPARK-29498.

Authored-by: Yuming Wang <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
  • Loading branch information
wangyum authored and cloud-fan committed Oct 21, 2019
1 parent 5b4d917 commit e99a9f7
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -573,9 +573,8 @@ private[hive] class HiveClientImpl(
// If users explicitly alter these Hive-specific properties through ALTER TABLE DDL, we respect
// these user-specified values.
verifyColumnDataType(table.dataSchema)
val owner = Option(table.owner).filter(_.nonEmpty).getOrElse(userName)
val hiveTable = toHiveTable(
table.copy(properties = table.ignoredProperties ++ table.properties), Some(owner))
table.copy(properties = table.ignoredProperties ++ table.properties), Some(userName))
// Do not use `table.qualifiedName` here because this may be a rename
val qualifiedTableName = s"$dbName.$tableName"
shim.alterTable(client, qualifiedTableName, hiveTable)
Expand Down Expand Up @@ -1039,7 +1038,7 @@ private[hive] object HiveClientImpl {
}
hiveTable.setFields(schema.asJava)
hiveTable.setPartCols(partCols.asJava)
userName.foreach(hiveTable.setOwner)
Option(table.owner).filter(_.nonEmpty).orElse(userName).foreach(hiveTable.setOwner)
hiveTable.setCreateTime(MILLISECONDS.toSeconds(table.createTime).toInt)
hiveTable.setLastAccessTime(MILLISECONDS.toSeconds(table.lastAccessTime).toInt)
table.storage.locationUri.map(CatalogUtils.URIToString).foreach { loc =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,19 @@ class HiveExternalCatalogSuite extends ExternalCatalogSuite {
catalog.createDatabase(newDb("dbWithNullDesc").copy(description = null), ignoreIfExists = false)
assert(catalog.getDatabase("dbWithNullDesc").description == "")
}

test("SPARK-29498 CatalogTable to HiveTable should not change the table's ownership") {
val catalog = newBasicCatalog()
val owner = "SPARK-29498"
val hiveTable = CatalogTable(
identifier = TableIdentifier("spark_29498", Some("db1")),
tableType = CatalogTableType.MANAGED,
storage = storageFormat,
owner = owner,
schema = new StructType().add("i", "int"),
provider = Some("hive"))

catalog.createTable(hiveTable, ignoreIfExists = false)
assert(catalog.getTable("db1", "spark_29498").owner === owner)
}
}

0 comments on commit e99a9f7

Please sign in to comment.