gaborkaszab commented on code in PR #6621:
URL: https://github.com/apache/iceberg/pull/6621#discussion_r1097453375
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -494,6 +494,17 @@ private void setHmsTableParameters(
// remove any props from HMS that are no longer present in Iceberg table
props
obsoleteProps.forEach(parameters::remove);
+ // altering owner
+ if (metadata.properties().get(HiveCatalog.HMS_TABLE_OWNER) != null) {
+ tbl.setOwner(metadata.properties().get(HiveCatalog.HMS_TABLE_OWNER));
+ }
+
+ // dropping owner: instead of leaving the owner blank/null, the owner will
be
+ // default to whoever is making the current drop operation
+ if (obsoleteProps.contains(HiveCatalog.HMS_TABLE_OWNER)) {
Review Comment:
@haizhou-zhao, sorry for the delay with my answer.
I might miss something here but I don't think that table ownership should be
present in the Iceberg Metadata files nor should be a table property, it's
rather an attribute of the table in HMS that could be provided during table
creation. If you check [this
line](https://github.com/apache/iceberg/blob/333227fbd13821365cec1bdbfcb9314a239bea0f/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L374)
I deliberately remove the HMS_TABLE_OWNER from the table properties because
this information should only present for tables in HiveCatalog in HMS. The
reason TableMetadata.properties() is used for setting the table ownership is
that there was no other way to pass this piece of information to HMS without
changing the Iceberg API. But HMS_TABLE_OWNER is eventually not added as an
actual table param.
Following this logic I don't think that removing table ownership makes sense
as a table should always have an owner. I'd vote for rejecting such requests
rather than setting some default value. If the current user wants to grab the
ownership of the table then it can change the ownership rather than removing it.
So in general how I imagine this is to allow changing ownership via setting
a table property (but at the end this shouldn't end up as an actual table
property) and I'd not allow the user to remove table ownership.
--
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]
