Uh oh!
There was an error while loading. Please reload this page.
Rename last updated timestamp column in PartitionsTable - #8003
Conversation
| root.eqDeleteRecordCount, | ||
| root.eqDeleteFileCount, | ||
| root.lastUpdatedMs, | ||
| Optional.ofNullable(root.lastUpdatedAt).map(t -> t * 1000).orElse(null), |
There was a problem hiding this comment.
Am i missing something here? Is it just moving the multiply by 1000, from update() to here?
There was a problem hiding this comment.
yes Szehon, I am not changing how we read from partition metadata table, But the data is stored at microsecond precision instead (if needed to expose via other means)
dramaticlly
commented
Jul 6, 2023
discussed with @szehon-ho offline, I think we can leave the scope of this PR to only rename the column from |
szehon-ho
left a comment
There was a problem hiding this comment.
New name looks good to me, to be more consistent with other metadata tables.
szehon-ho
commented
Jul 6, 2023
Merged, thanks @dramaticlly for fix and @ajantha-bhat for pointing out the issue. We can do follow up if there are more issues. |
ajantha-bhat
commented
Jul 7, 2023
Thanks for quickly changing the name and making it less confusing for the readers. |
Good callout from @ajantha-bhat on iceberg spec where
timestamptzsupposed to store at microseconds precision but what we did here is essentially storelastUpdatedMsin PartitionsTable with microsecond * 1000 (since we cannot be more precise than what's already tracked on snapshot side)This patch try to
last_updated_mstolast_updated_atto conform with what we had in SnapshotsTablehttps://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/SnapshotsTable.java#L99
CC @szehon-ho@rdblue