haizhou-zhao commented on code in PR #6045:
URL: https://github.com/apache/iceberg/pull/6045#discussion_r1005077283
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -437,6 +451,18 @@ private Table newHmsTable(TableMetadata metadata) {
null,
TableType.EXTERNAL_TABLE.toString());
+ // hive2 does not support setting table ownership type, therefore we leave
it as
+ // a table property. Once upgraded to hive3, then we can simply set
+ //
newTable.setOwnerType(PrincipalType.valueOf(metadata.get(HMS_TABLE_OWNER_TYPE)))
+ String ownerType = metadata.property(TableProperties.HMS_TABLE_OWNER_TYPE,
null);
+ if (ownerType != null &&
ownerType.equalsIgnoreCase(PrincipalType.GROUP.name())) {
+ newTable
+ .getParameters()
+ .put(TableProperties.HMS_TABLE_OWNER_TYPE,
PrincipalType.GROUP.name());
Review Comment:
nvm, ignore my previous comment. I think I mis-understood what you are
saying. Yes, just directly put `ownerType` works. I probably shouldn't use
`equalsIgnoreCase` in L458 in the first place. I think we should make clear to
users, if they want to tweak owner type, their key should be all-cap `"GROUP"`
in the property.
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -437,6 +451,18 @@ private Table newHmsTable(TableMetadata metadata) {
null,
TableType.EXTERNAL_TABLE.toString());
+ // hive2 does not support setting table ownership type, therefore we leave
it as
+ // a table property. Once upgraded to hive3, then we can simply set
+ //
newTable.setOwnerType(PrincipalType.valueOf(metadata.get(HMS_TABLE_OWNER_TYPE)))
+ String ownerType = metadata.property(TableProperties.HMS_TABLE_OWNER_TYPE,
null);
+ if (ownerType != null &&
ownerType.equalsIgnoreCase(PrincipalType.GROUP.name())) {
+ newTable
+ .getParameters()
+ .put(TableProperties.HMS_TABLE_OWNER_TYPE,
PrincipalType.GROUP.name());
+ } else { // default: individual ownership
Review Comment:
Good idea.
--
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]
