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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to