kbendick commented on a change in pull request #3039:
URL: https://github.com/apache/iceberg/pull/3039#discussion_r697832912
##########
File path: core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java
##########
@@ -65,12 +65,16 @@ public static boolean ancestorOf(Table table, long
snapshotId, long ancestorSnap
}
/**
- * Traverses the history of the table's current snapshot and finds the
oldest Snapshot.
- * @return null if there is no current snapshot in the table, else the
oldest Snapshot.
+ * Traverses the history of the table's current snapshot and finds the
oldest Snapshot after the timestamp.
+ * @return null if there is no current snapshot in the table, else the
oldest Snapshot after the timestamp.
*/
- public static Snapshot oldestSnapshot(Table table) {
+ public static Snapshot oldestSnapshot(Table table, long timestamp) {
Snapshot current = table.currentSnapshot();
- while (current != null && current.parentId() != null) {
+ if (current == null || current.timestampMillis() < timestamp) {
+ return null;
+ }
+
+ while (current.parentId() != null &&
table.snapshot(current.parentId()).timestampMillis() > timestamp) {
Review comment:
I think if we use `as-of-timestamp`, this question becomes unnecessary
(the Boolean logic needs to change a bit possibly).
I think `as-of-timestamp` makes more sense to expose first as it’s already
available in the batch source (so it would be consistent), and users can then
easily specify a time for their table without having to do some time give or
take 1 millisecond - where the table could have changed (theoretically though
probably not likely). `as-of-timestamp` avoids that concern.
If there’s a use case for an option in addition to `as-of-timestamp`, please
do say so. But in that case, I’d argue we should implement both.
--
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]
