haizhou-zhao commented on code in PR #6324:
URL: https://github.com/apache/iceberg/pull/6324#discussion_r1053645840
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -567,8 +570,13 @@ Database convertToDatabase(Namespace namespace,
Map<String, String> meta) {
});
if (database.getOwnerName() == null) {
- database.setOwnerName(System.getProperty("user.name"));
- database.setOwnerType(PrincipalType.USER);
+ try {
+
database.setOwnerName(UserGroupInformation.getCurrentUser().getUserName());
+ database.setOwnerType(PrincipalType.USER);
+ } catch (IOException e) {
+ throw new UncheckedIOException(
+ String.format("Fail to obtain default (UGI) user for database %s",
database), e);
Review Comment:
Agree on comments for error message. On the other point, I assume you are
suggesting if we cannot obtain UGI identity, then we should use OS username
like before. I'd like to put more discussion on it.
This PR is created because OS username (System.getProperty("user.name")) is
not a good representation of who the owner of this process is when interacting
with Hive Metastore (@danielcweeks @szehon-ho @gaborkaszab correct me if
needed). Hive Metastore and its accompanying execution engines (HQL or Spark)
usually recognize the Kerberos principal as identity, hence Hadoop's UGI is
chosen over OS username.
When UGI fail to get the current identity, that means the user do not have
the correct keytab or other credentials to authenticate with Kerberos server.
If we fall back to OS username, then the outcome is we are swallowing a user
issue, and setting the owner to some user unrecognizable by Hive ecosystem. If
user didn't set up their Kerberos credentials properly, then I think it's the
user's issue, and the cleanest way is to throw the error back at user, instead
of swallowing the error and try to do something for user.
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -422,11 +425,23 @@ private Table newHmsTable(TableMetadata metadata) {
Preconditions.checkNotNull(metadata, "'metadata' parameter can't be null");
final long currentTimeMillis = System.currentTimeMillis();
+ String owner = metadata.properties().get(HiveCatalog.HMS_TABLE_OWNER);
+ if (owner == null) {
+ try {
Review Comment:
Ack
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
