pvary commented on code in PR #9011:
URL: https://github.com/apache/iceberg/pull/9011#discussion_r1403245669
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -207,11 +195,15 @@ protected void doCommit(TableMetadata base, TableMetadata
metadata) {
updateHiveTable = true;
LOG.debug("Committing existing table: {}", fullName);
} else {
- tbl = newHmsTable(metadata);
+ tbl =
+ newHmsTable(
+ metadata.property(HiveCatalog.HMS_TABLE_OWNER,
HiveHadoopUtil.currentUser()));
Review Comment:
You might be better positioned to suggest things from the `View` point of
view 😄, but if it were only me, then I would have created the following method
in `HiveOperationsBase`:
```
default Table newHmsTable(TableMetadata metadata) {
[..]
}
```
and kept the implementation the same (using the same metadata key to get the
owner), like before:
```
Preconditions.checkNotNull(metadata, "'metadata' parameter can't be
null");
final long currentTimeMillis = System.currentTimeMillis();
Table newTable =
new Table(
tableName,
database,
metadata.property(HiveCatalog.HMS_TABLE_OWNER,
HiveHadoopUtil.currentUser()),
(int) currentTimeMillis / 1000,
(int) currentTimeMillis / 1000,
Integer.MAX_VALUE,
null,
Collections.emptyList(),
Maps.newHashMap(),
null,
null,
TableType.EXTERNAL_TABLE.toString());
newTable
.getParameters()
.put("EXTERNAL", "TRUE"); // using the external table type also
requires this
return newTable;
```
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -207,11 +195,15 @@ protected void doCommit(TableMetadata base, TableMetadata
metadata) {
updateHiveTable = true;
LOG.debug("Committing existing table: {}", fullName);
} else {
- tbl = newHmsTable(metadata);
+ tbl =
+ newHmsTable(
+ metadata.property(HiveCatalog.HMS_TABLE_OWNER,
HiveHadoopUtil.currentUser()));
Review Comment:
You might be better positioned to suggest things from the `View` point of
view 😄, but if it were only me, then I would have created the following method
in `HiveOperationsBase`:
```
default Table newHmsTable(TableMetadata metadata) {
[..]
}
```
and kept the implementation the same (using the same metadata key to get the
owner), like before:
```
Preconditions.checkNotNull(metadata, "'metadata' parameter can't be
null");
final long currentTimeMillis = System.currentTimeMillis();
Table newTable =
new Table(
tableName,
database,
metadata.property(HiveCatalog.HMS_TABLE_OWNER,
HiveHadoopUtil.currentUser()),
(int) currentTimeMillis / 1000,
(int) currentTimeMillis / 1000,
Integer.MAX_VALUE,
null,
Collections.emptyList(),
Maps.newHashMap(),
null,
null,
TableType.EXTERNAL_TABLE.toString());
newTable
.getParameters()
.put("EXTERNAL", "TRUE"); // using the external table type also
requires this
return newTable;
```
--
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
