haizhou-zhao commented on code in PR #6621:
URL: https://github.com/apache/iceberg/pull/6621#discussion_r1097737870
##########
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:
@gaborkaszab First of all, at this moment, the actual logic of creating
table differs from what you think, because all properties, including ownership,
gets into iceberg metadata file on creation:
[ref](https://github.com/apache/iceberg/blob/333227fbd13821365cec1bdbfcb9314a239bea0f/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java#L191)
Secondly, I have a different thought on whether ownership should be in
iceberg metadata file or not. Iceberg supports many forms of metastores and
today Iceberg users are allowed to migrate their tables between metastores,
such as between Hive Metastore (HMS), Nessie, JDBC, Dynamo, etc.
[ref](https://github.com/apache/iceberg/blob/32cfb62fdb9dca215250e4ca8285754e9393767d/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java#L69)
So I think the question becomes, whether Iceberg users only care about
ownership when they are using HMS, or they they also care when they use other
forms of metastore. I tend to believe that ownership is an Iceberg table
attributes, not specific to any forms of metastore. Otherwise, a user who wants
to migrate a table from Hive to Nessie, for example, would find the ownership
info lost over the process of migration (and they have to manually set the
ownership again in Nessie).
Lastly, I favor allowing user to drop ownership, but I'm good with following
your thought there to not allow. I can't imagine a use case for dropping
ownership (yes, I agree with you why not just alter). Yet, I see no harm in
allowing users to drop ownership, and I see ownership as a normal property just
like others - if we are allowed to drop other iceberg or metastore properties,
then making a logical exception for the ownership property so that the "unset"
(set null/clear) operation is forbidden might be some inconsistency that users
need to remember and be aware of when they use the code.
--
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]
