amogh-jahagirdar commented on code in PR #13556:
URL: https://github.com/apache/iceberg/pull/13556#discussion_r2238105630


##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -303,6 +309,33 @@ public static List<DataFile> newFiles(
     return newFiles;
   }
 
+  public static CloseableIterable<DataFile> newFilesBetween(
+      Long baseSnapshotId, long latestSnapshotId, Function<Long, Snapshot> 
lookup, FileIO io) {
+
+    List<Snapshot> snapshots = Lists.newArrayList();
+    Snapshot lastSnapshot = null;
+    for (Snapshot currentSnapshot : ancestorsOf(latestSnapshotId, lookup)) {
+      lastSnapshot = currentSnapshot;
+      if (Objects.equals(currentSnapshot.snapshotId(), baseSnapshotId)) {
+        break;
+      }
+      snapshots.add(currentSnapshot);

Review Comment:
   Style nit: Can we add a newline after the if block here



##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -303,6 +309,33 @@ public static List<DataFile> newFiles(
     return newFiles;
   }
 
+  public static CloseableIterable<DataFile> newFilesBetween(
+      Long baseSnapshotId, long latestSnapshotId, Function<Long, Snapshot> 
lookup, FileIO io) {

Review Comment:
   Nit: I know this is just using what the previous API did but I feel like 
better names would be `startSnapshotId` and `endSnapshotId` similar naming to 
some of the incremental structures in Iceberg.  



-- 
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]

Reply via email to