hililiwei commented on code in PR #5984: URL: https://github.com/apache/iceberg/pull/5984#discussion_r1009275075
########## api/src/main/java/org/apache/iceberg/IncrementalScan.java: ########## @@ -21,6 +21,23 @@ /** API for configuring an incremental scan. */ public interface IncrementalScan<ThisT, T extends ScanTask, G extends ScanTaskGroup<T>> extends Scan<ThisT, T, G> { + + /** + * Instructs this scan to look for changes starting from a particular snapshot (inclusive). + * + * <p>If the start snapshot is not configured, it is defaulted to the oldest ancestor of the end + * snapshot (inclusive). + * + * @param fromSnapshotId the start snapshot ID (inclusive) + * @param referenceName the ref used + * @return this for method chaining + * @throws IllegalArgumentException if the start snapshot is not an ancestor of the end snapshot + */ + default ThisT fromSnapshotInclusive(long fromSnapshotId, String referenceName) { Review Comment: `fromSnapshotInclusive(String fromSnapshotRef)` is fine for me, but one thing I want to clarify is that if we pass in a tag, then it represents the snapshot of the tag binding, but what does it mean if it's a branch? Use the current latest snapshot of the branch and switch the branch to `fromSnapshotRef`? Or it not allowed to use branch? -- 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