szehon-ho commented on code in PR #6621:
URL: https://github.com/apache/iceberg/pull/6621#discussion_r1081718764
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -328,6 +329,140 @@ public void testCreateTableCustomSortOrder() throws
Exception {
}
}
+ @Test
+ public void testAlterTableOwner() throws IOException, TException {
+ alterTableAndVerifyOwner(
+ DB_NAME,
+ "tbl_alter_owner_1",
+ ImmutableMap.of(HiveCatalog.HMS_TABLE_OWNER, "some_owner"),
+ ImmutableMap.of(HiveCatalog.HMS_TABLE_OWNER, "some_other_owner"),
+ "some_owner",
+ "some_other_owner");
+ alterTableAndVerifyOwner(
+ DB_NAME,
+ "tbl_alter_owner_2",
+ ImmutableMap.of(),
+ ImmutableMap.of(HiveCatalog.HMS_TABLE_OWNER, "another_owner"),
+ UserGroupInformation.getCurrentUser().getUserName(),
+ "another_owner");
+ alterTableAndVerifyOwner(
+ DB_NAME,
+ "tbl_alter_owner_noop_1",
+ ImmutableMap.of(HiveCatalog.HMS_TABLE_OWNER, "some_owner"),
+ ImmutableMap.of(),
+ "some_owner",
+ "some_owner");
+ alterTableAndVerifyOwner(
+ DB_NAME,
+ "tbl_alter_owner_noop_2",
+ ImmutableMap.of(HiveCatalog.HMS_TABLE_OWNER, "some_owner"),
+ ImmutableMap.of("unrelated_prop", "val"),
+ "some_owner",
+ "some_owner");
+ alterTableAndVerifyOwner(
+ DB_NAME,
+ "tbl_alter_owner_noop_3",
+ ImmutableMap.of(),
+ ImmutableMap.of(),
+ UserGroupInformation.getCurrentUser().getUserName(),
+ UserGroupInformation.getCurrentUser().getUserName());
+ }
+
+ private void alterTableAndVerifyOwner(
Review Comment:
Nit: I think in most tests, we put the private at end of file? So the
```@Test``` are all in the top and easier to find.
##########
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:
Also too don't know of the use-case, but makes sense to me to set it to null
here.
--
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]
