Baunsgaard commented on code in PR #16620:
URL: https://github.com/apache/iceberg/pull/16620#discussion_r3361863337
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveOperationsBase.java:
##########
@@ -213,8 +213,8 @@ default Table newHmsTable(String hmsTableOwner) {
table(),
database(),
hmsTableOwner,
- (int) currentTimeMillis / 1000,
- (int) currentTimeMillis / 1000,
Review Comment:
Since you are anyway fixing it, it is a bit odd to do the calculation twice,
How about editing the variable above and call it:
```
final int currentTime = (int) (System.currentTimeMillis() / 1000L);
```
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -172,6 +172,22 @@ protected HiveCatalog catalog() {
return catalog;
}
+ @Test
+ public void newTableSetsCurrentHmsLastAccessTime() throws TException {
+ TableIdentifier tableIdent = TableIdentifier.of(DB_NAME,
"create_time_tbl");
+
+ int beforeSeconds = (int) (System.currentTimeMillis() / 1000);
+ catalog.createTable(tableIdent, getTestSchema());
+ int afterSeconds = (int) (System.currentTimeMillis() / 1000);
+
+ // HMS overwrites createTime server-side on create, but keeps the
lastAccessTime that Iceberg
+ // sends. Computing it as '(int) currentTimeMillis / 1000' casts the long
before dividing and
+ // overflows to a 1970-era value; assert it instead reflects the current
epoch second.
Review Comment:
I would suggest removing this comment.
--
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]
