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


##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -103,7 +103,22 @@ public static Iterable<Snapshot> currentAncestors(Table 
table) {
    * @return a set of snapshot IDs of the known ancestor snapshots, including 
the current ID
    */
   public static List<Long> currentAncestorIds(Table table) {
-    return ancestorIds(table.currentSnapshot(), table::snapshot);
+    return currentAncestorIds(table, null);
+  }
+
+  /**
+   * Return the snapshot IDs for the ancestors of the current table state at a 
given branch.
+   *
+   * <p>Ancestor IDs are ordered by commit time, descending. The first ID is 
the current snapshot at
+   * a given branch, followed by its parent, and so on.
+   *
+   * @param table a {@link Table}
+   * @param branch branch name of the table (nullable)
+   * @return a set of snapshot IDs of the known ancestor snapshots, including 
the current ID
+   */
+  public static List<Long> currentAncestorIds(Table table, String branch) {

Review Comment:
   Do we need the new API? Just want to avoid having to maintain another util 
API. It seems like we only use it in the new tests just to get the end snapshot 
for the incremental scan. In practice I think we would have the snapshot ID or 
it would just be a tagged snapshot which we could lookup the ID for.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to