Skip to content

Commit c862835

Browse files
HeartSaVioRcloud-fan
authored andcommitted
[SPARK-28996][SQL][TESTS] Add tests regarding username of HiveClient
### What changes were proposed in this pull request? This patch proposes to add new tests to test the username of HiveClient to prevent changing the semantic unintentionally. The owner of Hive table has been changed back-and-forth, principal -> username -> principal, and looks like the change is not intentional. (Please refer [SPARK-28996](https://issues.apache.org/jira/browse/SPARK-28996) for more details.) This patch intends to prevent this. This patch also renames previous HiveClientSuite(s) to HivePartitionFilteringSuite(s) as it was commented as TODO, as well as previous tests are too narrowed to test only partition filtering. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Newly added UTs. Closes #25696 from HeartSaVioR/SPARK-28996. Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
1 parent 4d27a25 commit c862835

File tree

6 files changed

+97
-5
lines changed

6 files changed

+97
-5
lines changed

sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClient.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,4 +292,6 @@ private[hive] trait HiveClient {
292292
/** Used for testing only. Removes all metadata from this instance of Hive. */
293293
def reset(): Unit
294294

295+
/** Returns the user name which is used as owner for Hive table. */
296+
def userName: String
295297
}

sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ private[hive] class HiveClientImpl(
222222
hiveConf
223223
}
224224

225-
private val userName = UserGroupInformation.getCurrentUser.getShortUserName
225+
override val userName = UserGroupInformation.getCurrentUser.getShortUserName
226226

227227
override def getConf(key: String, defaultValue: String): String = {
228228
conf.get(key, defaultValue)
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.hive.client
19+
20+
import java.security.PrivilegedExceptionAction
21+
22+
import org.apache.hadoop.conf.Configuration
23+
import org.apache.hadoop.security.UserGroupInformation
24+
import org.scalatest.{BeforeAndAfterAll, PrivateMethodTester}
25+
26+
import org.apache.spark.util.Utils
27+
28+
class HiveClientUserNameSuite(version: String) extends HiveVersionSuite(version) {
29+
30+
test("username of HiveClient - no UGI") {
31+
// Assuming we're not faking System username
32+
assert(getUserNameFromHiveClient === System.getProperty("user.name"))
33+
}
34+
35+
test("username of HiveClient - UGI") {
36+
val ugi = UserGroupInformation.createUserForTesting(
37+
"fakeprincipal@EXAMPLE.COM", Array.empty)
38+
ugi.doAs(new PrivilegedExceptionAction[Unit]() {
39+
override def run(): Unit = {
40+
assert(getUserNameFromHiveClient === ugi.getShortUserName)
41+
}
42+
})
43+
}
44+
45+
test("username of HiveClient - Proxy user") {
46+
val ugi = UserGroupInformation.createUserForTesting(
47+
"fakeprincipal@EXAMPLE.COM", Array.empty)
48+
val proxyUgi = UserGroupInformation.createProxyUserForTesting(
49+
"proxyprincipal@EXAMPLE.COM", ugi, Array.empty)
50+
proxyUgi.doAs(new PrivilegedExceptionAction[Unit]() {
51+
override def run(): Unit = {
52+
assert(getUserNameFromHiveClient === proxyUgi.getShortUserName)
53+
}
54+
})
55+
}
56+
57+
private def getUserNameFromHiveClient: String = {
58+
val hadoopConf = new Configuration()
59+
hadoopConf.set("hive.metastore.warehouse.dir", Utils.createTempDir().toURI().toString())
60+
val client = buildClient(hadoopConf)
61+
client.userName
62+
}
63+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.hive.client
19+
20+
import scala.collection.immutable.IndexedSeq
21+
22+
import org.scalatest.Suite
23+
24+
class HiveClientUserNameSuites extends Suite with HiveClientVersions {
25+
override def nestedSuites: IndexedSeq[Suite] = {
26+
versions.map(new HiveClientUserNameSuite(_))
27+
}
28+
}

sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuite.scala renamed to sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HivePartitionFilteringSuite.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ import org.apache.spark.sql.catalyst.expressions._
3131
import org.apache.spark.sql.types.{BooleanType, IntegerType, LongType, StructType}
3232
import org.apache.spark.util.Utils
3333

34-
// TODO: Refactor this to `HivePartitionFilteringSuite`
35-
class HiveClientSuite(version: String)
34+
class HivePartitionFilteringSuite(version: String)
3635
extends HiveVersionSuite(version) with BeforeAndAfterAll {
3736

3837
private val tryDirectSqlKey = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL.varname

sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuites.scala renamed to sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HivePartitionFilteringSuites.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ import scala.collection.immutable.IndexedSeq
2121

2222
import org.scalatest.Suite
2323

24-
class HiveClientSuites extends Suite with HiveClientVersions {
24+
class HivePartitionFilteringSuites extends Suite with HiveClientVersions {
2525
override def nestedSuites: IndexedSeq[Suite] = {
2626
// Hive 0.12 does not provide the partition filtering API we call
27-
versions.filterNot(_ == "0.12").map(new HiveClientSuite(_))
27+
versions.filterNot(_ == "0.12").map(new HivePartitionFilteringSuite(_))
2828
}
2929
}

0 commit comments

Comments
 (0)