-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Delta Migration: Add version and timestamp tags for each Delta Lake transaction when add to Iceberg transaction #7450
Delta Migration: Add version and timestamp tags for each Delta Lake transaction when add to Iceberg transaction #7450
Conversation
Signed-off-by: Rushan Jiang <rushanj@andrew.cmu.edu>
Signed-off-by: Rushan Jiang <rushanj@andrew.cmu.edu>
delta-lake/src/main/java/org/apache/iceberg/delta/BaseSnapshotDeltaLakeTableAction.java
Outdated
Show resolved
Hide resolved
delta-lake/src/main/java/org/apache/iceberg/delta/BaseSnapshotDeltaLakeTableAction.java
Show resolved
Hide resolved
deltaVersionTimestamp | ||
.toInstant() | ||
.atZone(ZoneId.of(DELTA_TIME_STAMP_ZONE)) | ||
.format(DateTimeFormatter.ofPattern(DELTA_TIME_STAMP_FORMAT)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering how much it is worth doing this conversion, instead of just using the millisecond timestamp value. Any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought about this a bit more, I think I would +1 for just using raw millisecond timestamp. Dealing with timezone is complex and should be avoided if there is no significant benefit. Also it leaves a space character in the tag, that might cause issue.
Signed-off-by: Rushan Jiang <rushanj@andrew.cmu.edu>
Signed-off-by: Rushan Jiang <rushanj@andrew.cmu.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me!
Thanks for adding the feature Jonas! |
Fixes #6769
For each Iceberg transaction migrated, add
delta-version-xxx
anddelta-ts-xxxxx
tag to the snapshot.delta-version-xxx
represents the logical delta lake version.delta-ts-xxxxx
represents the commit time of the corresponding delta lake transaction