rebase and address

apache · Apr 27, 2023 · 768782d · 768782d
1 parent 7166d54
commit 768782d
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 21 deletions.
diff --git a/core/src/main/java/org/apache/iceberg/BaseIncrementalScan.java b/core/src/main/java/org/apache/iceberg/BaseIncrementalScan.java
@@ -37,12 +37,9 @@ protected abstract CloseableIterable<T> doPlanFiles(
   @Override
   public ThisT fromSnapshotInclusive(String ref) {
     SnapshotRef snapshotRef = table().refs().get(ref);
-    Preconditions.checkArgument(snapshotRef != null, "Cannot find ref %s", ref);
+    Preconditions.checkArgument(snapshotRef != null, "Cannot find ref: %s", ref);
     Preconditions.checkArgument(snapshotRef.isTag(), "Ref %s is not a tag", ref);
-
-    TableScanContext newContext = context().fromSnapshotIdInclusive(snapshotRef.snapshotId());
-
-    return newRefinedScan(table(), schema(), newContext);
+    return fromSnapshotInclusive(snapshotRef.snapshotId());
   }
 
   @Override
@@ -58,9 +55,8 @@ public ThisT fromSnapshotInclusive(long fromSnapshotId) {
   @Override
   public ThisT fromSnapshotExclusive(String ref) {
     SnapshotRef snapshotRef = table().refs().get(ref);
-    Preconditions.checkArgument(snapshotRef != null, "Cannot find ref %s", ref);
+    Preconditions.checkArgument(snapshotRef != null, "Cannot find ref: %s", ref);
     Preconditions.checkArgument(snapshotRef.isTag(), "Ref %s is not a tag", ref);
-
     return fromSnapshotExclusive(snapshotRef.snapshotId());
   }
 
@@ -85,7 +81,6 @@ public ThisT toSnapshot(String ref) {
     SnapshotRef snapshotRef = table().refs().get(ref);
     Preconditions.checkArgument(snapshotRef != null, "Cannot find ref: %s", ref);
     Preconditions.checkArgument(snapshotRef.isTag(), "Ref %s is not a tag", ref);
-
     return toSnapshot(snapshotRef.snapshotId());
   }
 
@@ -142,7 +137,8 @@ private long toSnapshotIdInclusive() {
         Preconditions.checkArgument(
             SnapshotUtil.isAncestorOf(
                 table(), currentSnapshot.snapshotId(), context().toSnapshotId()),
-            "End snapshot is not a valid snapshot on the current branch");
+            "End snapshot is not a valid snapshot on the current branch: %s",
+            context().branch());
       }
 
       return context().toSnapshotId();

diff --git a/core/src/main/java/org/apache/iceberg/TableScanContext.java b/core/src/main/java/org/apache/iceberg/TableScanContext.java
@@ -96,6 +96,9 @@ public MetricsReporter metricsReporter() {
     return LoggingMetricsReporter.instance();
   }
 
+  @Nullable
+  public abstract String branch();
+
   TableScanContext useSnapshotId(Long scanSnapshotId) {
     return ImmutableTableScanContext.builder().from(this).snapshotId(scanSnapshotId).build();
   }
@@ -172,6 +175,10 @@ TableScanContext reportWith(MetricsReporter reporter) {
         .build();
   }
 
+  TableScanContext useBranch(String ref) {
+    return ImmutableTableScanContext.builder().from(this).branch(ref).build();
+  }
+
   public static TableScanContext empty() {
     return ImmutableTableScanContext.builder().build();
   }

diff --git a/core/src/test/java/org/apache/iceberg/TestBaseIncrementalAppendScan.java b/core/src/test/java/org/apache/iceberg/TestBaseIncrementalAppendScan.java
@@ -69,11 +69,11 @@ public void testFromSnapshotInclusiveWithRef() {
     long snapshotCId = table.currentSnapshot().snapshotId();
 
     IncrementalAppendScan scan = newScan().fromSnapshotInclusive(tagSnapshotAName);
-    Assert.assertEquals(5, Iterables.size(scan.planFiles()));
+    Assertions.assertThat(scan.planFiles()).hasSize(5);
 
     IncrementalAppendScan scan3 =
         newScan().fromSnapshotInclusive(tagSnapshotAName).toSnapshot(tagSnapshotBName);
-    Assert.assertEquals(3, Iterables.size(scan3.planFiles()));
+    Assertions.assertThat(scan3.planFiles()).hasSize(3);
 
     Assertions.assertThatThrownBy(() -> newScan().fromSnapshotInclusive(branchName))
         .isInstanceOf(IllegalArgumentException.class)
@@ -102,11 +102,11 @@ public void testFromSnapshotExclusiveWithRef() {
     long snapshotCId = table.currentSnapshot().snapshotId();
 
     IncrementalAppendScan scan2 = newScan().fromSnapshotExclusive(tagSnapshotAName);
-    Assert.assertEquals(4, Iterables.size(scan2.planFiles()));
+    Assertions.assertThat(scan2.planFiles()).hasSize(4);
 
     IncrementalAppendScan scan3 =
         newScan().fromSnapshotExclusive(tagSnapshotAName).toSnapshot(tagSnapshotBName);
-    Assert.assertEquals(2, Iterables.size(scan3.planFiles()));
+    Assertions.assertThat(scan3.planFiles()).hasSize(2);
 
     Assertions.assertThatThrownBy(() -> newScan().fromSnapshotExclusive(branchName))
         .isInstanceOf(IllegalArgumentException.class)
@@ -141,29 +141,29 @@ public void testUseBranch() {
     long snapshotBranchCId = table.snapshot(branchName).snapshotId();
 
     IncrementalAppendScan scan = newScan().fromSnapshotInclusive(tagSnapshotAName);
-    Assert.assertEquals(5, Iterables.size(scan.planFiles()));
+    Assertions.assertThat(scan.planFiles()).hasSize(5);
 
     IncrementalAppendScan scan2 =
         newScan().fromSnapshotInclusive(tagSnapshotAName).useBranch(branchName);
-    Assert.assertEquals(3, Iterables.size(scan2.planFiles()));
+    Assertions.assertThat(scan2.planFiles()).hasSize(3);
 
     IncrementalAppendScan scan3 = newScan().toSnapshot(snapshotBranchBId).useBranch(branchName);
-    Assert.assertEquals(2, Iterables.size(scan3.planFiles()));
+    Assertions.assertThat(scan3.planFiles()).hasSize(2);
 
     IncrementalAppendScan scan4 = newScan().toSnapshot(snapshotBranchCId).useBranch(branchName);
-    Assert.assertEquals(3, Iterables.size(scan4.planFiles()));
+    Assertions.assertThat(scan4.planFiles()).hasSize(3);
 
     IncrementalAppendScan scan5 =
         newScan()
             .fromSnapshotExclusive(tagSnapshotAName)
             .toSnapshot(snapshotBranchBId)
             .useBranch(branchName);
-    Assert.assertEquals(1, Iterables.size(scan5.planFiles()));
+    Assertions.assertThat(scan5.planFiles()).hasSize(1);
 
     Assertions.assertThatThrownBy(
             () -> newScan().toSnapshot(snapshotMainBId).useBranch(branchName).planFiles())
         .isInstanceOf(IllegalArgumentException.class)
-        .hasMessage("End snapshot is not a valid snapshot on the current branch");
+        .hasMessageContaining("End snapshot is not a valid snapshot on the current branch");
 
     Assertions.assertThatThrownBy(
             () -> newScan().fromSnapshotInclusive(snapshotBranchBId).useBranch("notExistBranch"))
@@ -245,10 +245,10 @@ public void testToSnapshotWithRef() {
     table.manageSnapshots().createTag(tagSnapshotBranchBName, snapshotBranchBId).commit();
 
     IncrementalAppendScan scan = newScan().toSnapshot(tagSnapshotMainBName);
-    Assert.assertEquals(2, Iterables.size(scan.planFiles()));
+    Assertions.assertThat(scan.planFiles()).hasSize(2);
 
     IncrementalAppendScan scan2 = newScan().toSnapshot(tagSnapshotBranchBName);
-    Assert.assertEquals(3, Iterables.size(scan2.planFiles()));
+    Assertions.assertThat(scan2.planFiles()).hasSize(3);
 
     Assertions.assertThatThrownBy(() -> newScan().toSnapshot(branchName))
         .isInstanceOf(IllegalArgumentException.class)