From f408912eb237c22b4126f64a34132ed4f000b285 Mon Sep 17 00:00:00 2001 From: Kent Yao Date: Wed, 9 Oct 2019 18:15:52 +0800 Subject: [PATCH 1/5] [SPARK-29405][SQL] Alter table / Insert statements should not change a table's ownership --- .../org/apache/spark/sql/hive/client/HiveClientImpl.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala index 296f9ff5dd0bb..d706bdae92b34 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala @@ -27,6 +27,7 @@ import scala.collection.JavaConverters._ import scala.collection.mutable import scala.collection.mutable.ArrayBuffer +import org.apache.commons.lang3.StringUtils import org.apache.hadoop.fs.Path import org.apache.hadoop.hive.common.StatsSetupConst import org.apache.hadoop.hive.conf.HiveConf @@ -573,8 +574,9 @@ 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 = if (StringUtils.isEmpty(table.owner)) userName else table.owner val hiveTable = toHiveTable( - table.copy(properties = table.ignoredProperties ++ table.properties), Some(userName)) + table.copy(properties = table.ignoredProperties ++ table.properties), Some(owner)) // Do not use `table.qualifiedName` here because this may be a rename val qualifiedTableName = s"$dbName.$tableName" shim.alterTable(client, qualifiedTableName, hiveTable) From a6721ca93857acea1bfbc34046b967a39f8a5fa3 Mon Sep 17 00:00:00 2001 From: Kent Yao Date: Sat, 12 Oct 2019 16:11:40 +0800 Subject: [PATCH 2/5] fix ut --- .../scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala index d706bdae92b34..3e3460c822a8a 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala @@ -574,7 +574,7 @@ 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 = if (StringUtils.isEmpty(table.owner)) userName else table.owner + val owner = Option(table.owner).filter(_.nonEmpty).getOrElse(userName) val hiveTable = toHiveTable( table.copy(properties = table.ignoredProperties ++ table.properties), Some(owner)) // Do not use `table.qualifiedName` here because this may be a rename From a990a3138d8a3079d6d01e9fec30f26cd15dbe76 Mon Sep 17 00:00:00 2001 From: Kent Yao Date: Thu, 17 Oct 2019 15:23:36 +0800 Subject: [PATCH 3/5] add an unit test --- .../spark/sql/hive/client/VersionsSuite.scala | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala index 1c82c7e86faab..d6930e974c050 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala @@ -291,6 +291,19 @@ class VersionsSuite extends SparkFunSuite with Logging { assert(client.getTable("default", "src").properties.contains("changed")) } + test(s"$version: alterTable - should respect the original catalog table's owner name") { + val ownerName = "SPARK-29405" + val originalTable = client.getTable("default", "src") + // mocking the owner is what we declared + val newTable = originalTable.copy(owner = ownerName) + client.alterTable(newTable) + assert(client.getTable("default", "src") === ownerName) + // mocking the owner is empty + val newTable2 = originalTable.copy(owner = "") + client.alterTable(newTable2) + assert(client.getTable("default", "src") === client.userName) + } + test(s"$version: alterTable(dbName: String, tableName: String, table: CatalogTable)") { val newTable = client.getTable("default", "src").copy(properties = Map("changedAgain" -> "")) client.alterTable("default", "src", newTable) From 0743ddd61bcef35e6e55598cbb6d3cb9476f76bd Mon Sep 17 00:00:00 2001 From: Kent Yao Date: Thu, 17 Oct 2019 17:03:28 +0800 Subject: [PATCH 4/5] fix ut --- .../org/apache/spark/sql/hive/client/VersionsSuite.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala index d6930e974c050..6941048cf6f3b 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala @@ -297,11 +297,11 @@ class VersionsSuite extends SparkFunSuite with Logging { // mocking the owner is what we declared val newTable = originalTable.copy(owner = ownerName) client.alterTable(newTable) - assert(client.getTable("default", "src") === ownerName) + assert(client.getTable("default", "src").owner === ownerName) // mocking the owner is empty val newTable2 = originalTable.copy(owner = "") client.alterTable(newTable2) - assert(client.getTable("default", "src") === client.userName) + assert(client.getTable("default", "src").owner === client.userName) } test(s"$version: alterTable(dbName: String, tableName: String, table: CatalogTable)") { From 10cb13bc846f5ad207e3a4f417483ded1b9179ef Mon Sep 17 00:00:00 2001 From: Kent Yao Date: Thu, 17 Oct 2019 17:09:17 +0800 Subject: [PATCH 5/5] rm unsed import --- .../scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala index 3e3460c822a8a..783bc5b562ff4 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala @@ -27,7 +27,6 @@ import scala.collection.JavaConverters._ import scala.collection.mutable import scala.collection.mutable.ArrayBuffer -import org.apache.commons.lang3.StringUtils import org.apache.hadoop.fs.Path import org.apache.hadoop.hive.common.StatsSetupConst import org.apache.hadoop.hive.conf.HiveConf