rdblue commented on code in PR #5984:
URL: https://github.com/apache/iceberg/pull/5984#discussion_r1001958189
##########
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:
I think the intent of this PR is to set the incremental scan's ending ref.
If that's the case, then I don't think it makes sense to set the end ref in a
`from` method.
The existing API behavior is to implicitly use the current table state as
the end ref. That is, if you don't call `toSnapshot`, it is assumed to be
`table.currentSnapshot`. That allows you to configure a scan by just calling
`from`:
```java
table.newIncrementalScan()
.fromSnapshotInclusive(lastProcessedId)
.planTasks()
```
The equivalent is this:
```java
table.newIncrementalScan()
.fromSnapshotExclusive(lastProcessedId)
.toSnapshot(table.currentSnapshot().snapshotId())
.planTasks()
```
Because `toSnapshot` is what is changing to use a branch, it is `toSnapshot`
that needs to be updated, not `fromSnapshot`.
The change should be to add a new `to` method that includes a branch name,
either `to(String)` or `toRef(String)` (maybe `toBranch`; I'm not sure if tags
would be used).
--
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
