fix(hive-sync): set HMS table createTime in seconds instead of milliseconds - #19335
Conversation
…econds Hive stores Table.createTime as seconds since the epoch in an i32 field, but HMSDDLExecutor.createTable passed raw System.currentTimeMillis(): the value is 1000x too large and, being ~1.7e12, overflows the int cast into a garbage (often negative or far-past) value. Divide by 1000 before the cast, matching the existing HoodieHiveCatalog usage. Adds unit test TestHMSDDLExecutorCreateTable that captures the Table passed to createTable (mocked IMetaStoreClient) and asserts createTime is epoch seconds within the call window.
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the contribution! This PR fixes the HMS-mode Hive sync path so the table createTime is recorded in epoch seconds rather than raw milliseconds, matching Hive's i32 contract and the existing HoodieHiveCatalog convention. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. One minor comment on the test — the explanation of the bug overflow behaviour is already covered by the production code comment and could be trimmed.
cc @yihua
| verify(client).createTable(captor.capture()); | ||
| int createTime = captor.getValue().getCreateTime(); | ||
|
|
||
| // createTime must be epoch seconds within the call window. Passing raw milliseconds instead |
There was a problem hiding this comment.
🤖 nit: the second sentence here ('Passing raw milliseconds instead would overflow...') duplicates the comment already added in HMSDDLExecutor.java -- could you trim it to just the first sentence describing what the assertion window represents?
…the createTime test comment
|
@voonhous could you please review this easy fix |
|
Danny thank you for review! |
4 participants
Describe the issue this Pull Request addresses
When syncing a table in HMS mode,
HMSDDLExecutor.createTablesets the HivecreateTimewithnewTb.setCreateTime((int) System.currentTimeMillis()). Hive storescreateTimeas seconds since the epoch in ani32field, but this passes raw milliseconds: the value is 1000x too large and, being ~1.7e12, overflows theintcast into a garbage (often negative or far-past) value. The rest of the codebase already does this correctly, for exampleHoodieHiveCataloguses(int) (System.currentTimeMillis() / 1000).Summary and Changelog
Divides the millisecond timestamp by 1000 before the
intcast so the Hive metastore records the table's creation time as epoch seconds, matching Hive's contract and the existingHoodieHiveCatalogusage.HMSDDLExecutor.createTablenow callssetCreateTime((int) (System.currentTimeMillis() / 1000)).TestHMSDDLExecutorCreateTablethat captures theTablepassed tocreateTable(with a mockedIMetaStoreClient) and assertscreateTimeis epoch seconds within the call window; this assertion fails against the previous milliseconds value.Impact
Tables created via HMS-mode Hive sync now record a correct
createTime(visible inDESCRIBE FORMATTEDand used by tools that read table age). No public API or configuration change.Risk Level
low
One-line change plus a unit test; the behavior is verified against the corrected value and matches the existing
HoodieHiveCatalogconvention.Documentation Update
none
Contributor's checklist