stevenzwu commented on code in PR #4744:
URL: https://github.com/apache/iceberg/pull/4744#discussion_r878586330
##########
core/src/main/java/org/apache/iceberg/BaseIncrementalAppendScan.java:
##########
@@ -79,26 +72,50 @@ public IncrementalAppendScan toSnapshot(long toSnapshotId) {
@Override
public CloseableIterable<FileScanTask> planFiles() {
- // Return an empty iterable when table is empty
- if (table().currentSnapshot() == null) {
- return CloseableIterable.empty();
+ long toSnapshotIdInclusive;
+ if (context().toSnapshotId() != null) {
+ toSnapshotIdInclusive = context().toSnapshotId();
+ } else if (table().currentSnapshot() != null) {
+ toSnapshotIdInclusive = table().currentSnapshot().snapshotId();
+ } else {
+ throw new IllegalArgumentException("End snapshot is not set and table
has no current snapshot");
}
- long toSnapshotIdInclusive = context().toSnapshotId() != null ?
- context().toSnapshotId() : table().currentSnapshot().snapshotId();
+
if (context().fromSnapshotId() != null) {
Preconditions.checkArgument(
SnapshotUtil.isAncestorOf(table(), toSnapshotIdInclusive,
context().fromSnapshotId()),
- "Starting snapshot (exclusive) %d is not an ancestor of end snapshot
%d",
+ "Starting snapshot (%s) %s is not an ancestor of end snapshot %s",
+ context().fromSnapshotInclusive() ? "inclusive" : "exclusive",
context().fromSnapshotId(), toSnapshotIdInclusive);
}
+ Long fromSnapshotIdExclusive = context().fromSnapshotId();
+ if (context().fromSnapshotId() != null &&
context().fromSnapshotInclusive()) {
+ // for inclusive behavior fromSnapshotIdExclusive is set to the parent
snapshot id, which can be null.
+ // appendsBetween handles null fromSnapshotIdExclusive properly by
finding the ancestors of end snapshot.
+ fromSnapshotIdExclusive =
table().snapshot(context().fromSnapshotId()).parentId();
+ }
+
+ notifyIncrementalScanEvent(table(), context(), toSnapshotIdInclusive);
+
// appendsBetween handles null fromSnapshotId (exclusive) properly
- List<Snapshot> snapshots = appendsBetween(table(),
context().fromSnapshotId(), toSnapshotIdInclusive);
+ List<Snapshot> snapshots = appendsBetween(table(),
fromSnapshotIdExclusive, toSnapshotIdInclusive);
if (snapshots.isEmpty()) {
return CloseableIterable.empty();
}
+ return planFilesFromSnapshots(snapshots);
Review Comment:
this method was extracted to avoid checkstyle error on CyclomaticComplexity
```
[ant:checkstyle] [ERROR]
/Users/stevenwu/ws/iceberg/core/src/main/java/org/apache/iceberg/BaseIncrementalAppendScan.java:73:3:
Cyclomatic Complexity is 13 (max allowed is 12). [CyclomaticComplexity]
```
--
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]
