haizhou-zhao commented on code in PR #6045:
URL: https://github.com/apache/iceberg/pull/6045#discussion_r1022077957
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -539,12 +577,22 @@ Database convertToDatabase(Namespace namespace,
Map<String, String> meta) {
database.setDescription(value);
} else if (key.equals("location")) {
database.setLocationUri(value);
+ } else if (key.equals(HMS_DB_OWNER)) {
+ database.setOwnerName(value);
+ } else if (key.equals(HMS_DB_OWNER_TYPE) && value != null) {
+ database.setOwnerType(PrincipalType.valueOf(value));
} else {
if (value != null) {
parameter.put(key, value);
}
}
});
+
+ if (database.getOwnerName() == null) {
+ database.setOwnerName(System.getProperty("user.name"));
Review Comment:
@danielcweeks I actually had conversation with both @gaborkaszab and
@szehon-ho on this topic. We want to put "adding ownership support for
namespaces" and "use UGI.getCurrentUser as default value" into two separate
PRs. My very first version of this PR was actually a combination of both, which
people felt was just too much content into a single PR for reviewing purpose.
Context is: the current iceberg-hive-metastore code is using Java/OS username
as default value.
Yes, I will follow up this PR with another one to switch to UGI.currentUser
(yes, I agree with you that it would be a more correct default value in the
Hive world comparing to Java/OS username). If you have a strong opinion on
these two changes should be in one single PR, then, for sure, please let us
know. Thx!
--
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]
