Skip to content

[SPARK-26929][SQL]fix table owner use user instead of principal when create table through spark-sql or beeline #23952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hddong wants to merge 12 commits into from
Closed

[SPARK-26929][SQL]fix table owner use user instead of principal when create table through spark-sql or beeline #23952

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2609c9c
[SPARK-26929][SQL]fix table owner use user instead of principal when …
hddong Mar 4, 2019
05e54fe
Update HiveClientImpl.scala
hddong Jul 25, 2019
bdcc9fa
Update HiveClientImpl.scala
hddong Jul 25, 2019
c4e3468
Merge branch 'master' into SPARK-26929-fix-table-owner
hddong Jul 25, 2019
e8c0571
Update HiveClientImpl.scala
hddong Jul 25, 2019
c27d4af
Update HiveClientImpl.scala
hddong Jul 25, 2019
f1eb8cf
Update HiveClientImpl.scala
hddong Jul 26, 2019
0c7ddb1
Update HiveClientImpl.scala
hddong Sep 10, 2019
6011598
Update HiveClientImpl.scala
hddong Sep 10, 2019
76192b2
Update HiveClientImpl.scala
hddong Sep 10, 2019
a8d6b1d
Update HiveClientImpl.scala
hddong Sep 11, 2019
2f37513
Update HiveClientImpl.scala
hddong Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@

package org.apache.spark.sql.hive.client

import java.io.{File, PrintStream}
import java.io.{File, IOException, PrintStream}
import java.lang.{Iterable => JIterable}
import java.util.{Locale, Map => JMap}
import java.util.concurrent.TimeUnit._

import javax.security.auth.login.LoginException
import scala.collection.JavaConverters._
import scala.collection.mutable
import scala.collection.mutable.ArrayBuffer
Expand Down Expand Up @@ -55,6 +56,7 @@ import org.apache.spark.sql.hive.HiveExternalCatalog.{DATASOURCE_SCHEMA, DATASOU
import org.apache.spark.sql.hive.client.HiveClientImpl._
import org.apache.spark.sql.types._
import org.apache.spark.util.{CircularBuffer, Utils}
import org.apache.hadoop.hive.shims.{Utils => HiveUtils}

/**
* A class that wraps the HiveClient and converts its responses to externally visible classes.
Expand Down Expand Up @@ -191,7 +193,12 @@ private[hive] class HiveClientImpl(
/** Returns the configuration for the current session. */
def conf: HiveConf = state.getConf

private val userName = conf.getUser
private val userName: String = try {
val ugi = HiveUtils.getUGI
ugi.getShortUserName
Copy link
Contributor

@cloud-fan cloud-fan Mar 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we prove this is the right fix? What if there is no UGI?

Copy link
Contributor Author

@hddong hddong Mar 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cloud-fan , It will return system user information if there is no ugi. Actually, the source code of conf.getUser is:

public String getUser() throws IOException {
        try {
             UserGroupInformation ugi = Utils.getUGI(); 
             return ugi.getUserName();
         } catch (LoginException var2) { 
             throw new IOException(var2); 
         } 
     }

this change just use ugi.getShortUserName instead of ugi.getUserName()

Copy link
Contributor

@HeartSaVioR HeartSaVioR Aug 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like Utils is accessible from here, so if we just want to leverage Hive code and call getShortUserName instead, we can just do it with one-liner.

private val userName = org.apache.hadoop.hive.shims.Utils.getUGI.getShortUserName

Here I intended to not catch LoginException as we don't do anything but throw it.

Copy link
Contributor Author

@hddong hddong Aug 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

org.apache.hadoop.hive.shims.Utils.getUGI.getShortUserName

Yep, but shims.Utils is not compatible with hive0.*, and cannot pass all tests. This file is added after hive1.*, so, copy the source code here.

Copy link
Contributor

@HeartSaVioR HeartSaVioR Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK. Didn't recognize it doesn't match with some versions. Makes sense to me.

Copy link
Contributor

@HeartSaVioR HeartSaVioR Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f1eb8cf was for this, sorry I had to track down commits as well.

} catch {
case e: LoginException => throw new IOException(e)
}

override def getConf(key: String, defaultValue: String): String = {
conf.get(key, defaultValue)
Expand Down