nastra commented on code in PR #5984:
URL: https://github.com/apache/iceberg/pull/5984#discussion_r1277600618
##########
core/src/test/java/org/apache/iceberg/TestBaseIncrementalAppendScan.java:
##########
@@ -51,6 +51,165 @@ public void testFromSnapshotInclusive() {
Assert.assertEquals(3, Iterables.size(scanWithToSnapshot.planFiles()));
}
+ @Test
+ public void fromSnapshotInclusiveWithNonExistingRef() {
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotInclusive("nonExistingRef"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot find ref: nonExistingRef");
+ }
+
+ @Test
+ public void fromSnapshotInclusiveWithTag() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagSnapshotBName = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagSnapshotBName, snapshotBId).commit();
+ table.newFastAppend().appendFile(FILE_C).appendFile(FILE_C).commit();
+
+ /*
+ files:FILE_A files:FILE_B FILE_B files:FILE_C
FILE_C
+ ---- snapshotAId(tag:t1) ---- snapshotMainB(tag:t2) ---- currentSnapshot
+ */
+ IncrementalAppendScan scan =
newScan().fromSnapshotInclusive(tagSnapshotAName);
+ Assertions.assertThat(scan.planFiles()).hasSize(5);
+
+ IncrementalAppendScan scanWithToSnapshot =
+
newScan().fromSnapshotInclusive(tagSnapshotAName).toSnapshot(tagSnapshotBName);
+ Assertions.assertThat(scanWithToSnapshot.planFiles()).hasSize(3);
+ }
+
+ @Test
+ public void fromSnapshotInclusiveWithBranchShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotInclusive(branchName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a tag", branchName));
+
+ Assertions.assertThatThrownBy(
+ () ->
newScan().fromSnapshotInclusive(snapshotAId).toSnapshot(branchName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a tag", branchName));
+ }
+
+ @Test
+ public void testUseBranch() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagName2 = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotMainBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagName2, snapshotMainBId).commit();
+
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchBId = table.snapshot(branchName).snapshotId();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchCId = table.snapshot(branchName).snapshotId();
+
+ /*
+
+ files:FILE_A files:FILE_B FILE_B files:FILE_B FILE_B
+ ---- snapshotA(tag:t1) ---- snapshotMainB(tag:t2) ---- currentSnapshot
+ \
+ \
+ \files:FILE_C
+ snapshotBranchB
+ \
+ \
+ \files:FILE_C
+ snapshotBranchC(branch:b1)
+ */
+ IncrementalAppendScan scan =
newScan().fromSnapshotInclusive(tagSnapshotAName);
+ Assertions.assertThat(scan.planFiles()).hasSize(5);
+
+ IncrementalAppendScan scan2 =
+
newScan().fromSnapshotInclusive(tagSnapshotAName).useBranch(branchName);
+ Assertions.assertThat(scan2.planFiles()).hasSize(3);
+
+ IncrementalAppendScan scan3 =
newScan().toSnapshot(snapshotBranchBId).useBranch(branchName);
+ Assertions.assertThat(scan3.planFiles()).hasSize(2);
+
+ IncrementalAppendScan scan4 =
newScan().toSnapshot(snapshotBranchCId).useBranch(branchName);
+ Assertions.assertThat(scan4.planFiles()).hasSize(3);
+
+ IncrementalAppendScan scan5 =
+ newScan()
+ .fromSnapshotExclusive(tagSnapshotAName)
+ .toSnapshot(snapshotBranchBId)
+ .useBranch(branchName);
+ Assertions.assertThat(scan5.planFiles()).hasSize(1);
+ }
+
+ @Test
+ public void testUseBranchWithTagShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ Assertions.assertThatThrownBy(
+ () ->
newScan().fromSnapshotInclusive(snapshotAId).useBranch(tagSnapshotAName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a branch", tagSnapshotAName));
+ }
+
+ @Test
+ public void testUseBranchWithInvalidEndSnapshotShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagName2 = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotMainBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagName2, snapshotMainBId).commit();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+
+ /*
+
+ files:FILE_A files:FILE_B FILE_B
+ ---- snapshotA(tag:t1) ---- snapshotMainB(tag:t2)
+ \
+ \
+ \files:FILE_C
+ snapshotBranchB(branch:b1)
+ */
+ Assertions.assertThatThrownBy(
+ () ->
newScan().toSnapshot(snapshotMainBId).useBranch(branchName).planFiles())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("End snapshot is not a valid snapshot on the
current branch");
+ }
+
+ @Test
+ public void testUseBranchWithNonExistingRef() {
+ Assertions.assertThatThrownBy(() -> newScan().useBranch("notExistBranch"))
Review Comment:
```suggestion
Assertions.assertThatThrownBy(() ->
newScan().useBranch("nonExistingRef"))
```
##########
core/src/test/java/org/apache/iceberg/TestBaseIncrementalAppendScan.java:
##########
@@ -101,6 +305,62 @@ public void testToSnapshot() {
Assert.assertEquals(2, Iterables.size(scan.planFiles()));
}
+ @Test
+ public void testToSnapshotWithTag() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+ table.newFastAppend().appendFile(FILE_B).commit();
+ long snapshotBId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ table.manageSnapshots().createBranch(branchName, snapshotBId).commit();
+
+ String tagSnapshotMainBName = "t1";
+ table.manageSnapshots().createTag(tagSnapshotMainBName,
snapshotBId).commit();
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+
+ String tagSnapshotBranchBName = "t2";
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchBId = table.snapshot(branchName).snapshotId();
+ table.manageSnapshots().createTag(tagSnapshotBranchBName,
snapshotBranchBId).commit();
+
+ /*
+
+ files:FILE_A files:FILE_B files:FILE_B FILE_B
+ ----snapshotA ------ snapshotMainB(tag:t1) -------- currentSnapshot
+ \
+ \
+ \files:FILE_C
+ snapshotBranchB(branch:b1, tag:t2)
+ */
+ IncrementalAppendScan scan = newScan().toSnapshot(tagSnapshotMainBName);
+ Assertions.assertThat(scan.planFiles()).hasSize(2);
+
+ IncrementalAppendScan scan2 = newScan().toSnapshot(tagSnapshotBranchBName);
+ Assertions.assertThat(scan2.planFiles()).hasSize(3);
+ }
+
+ @Test
+ public void testToSnapshotWithNonExistingRef() {
+ Assertions.assertThatThrownBy(() -> newScan().toSnapshot("notExistTag"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Cannot find ref");
Review Comment:
```suggestion
.hasMessage("Cannot find ref: nonExistingRef")
```
##########
core/src/test/java/org/apache/iceberg/TestBaseIncrementalAppendScan.java:
##########
@@ -51,6 +51,131 @@ public void testFromSnapshotInclusive() {
Assert.assertEquals(3, Iterables.size(scanWithToSnapshot.planFiles()));
}
+ @Test
+ public void testFromSnapshotInclusiveWithRef() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagSnapshotBName = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagSnapshotBName, snapshotBId).commit();
+ table.newFastAppend().appendFile(FILE_C).appendFile(FILE_C).commit();
+ long snapshotCId = table.currentSnapshot().snapshotId();
+
+ IncrementalAppendScan scan =
newScan().fromSnapshotInclusive(tagSnapshotAName);
+ Assertions.assertThat(scan.planFiles()).hasSize(5);
+
+ IncrementalAppendScan scan3 =
+
newScan().fromSnapshotInclusive(tagSnapshotAName).toSnapshot(tagSnapshotBName);
+ Assertions.assertThat(scan3.planFiles()).hasSize(3);
+
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotInclusive(branchName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a tag", branchName));
+
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotInclusive("notExistTag"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Cannot find ref");
+ }
+
+ @Test
+ public void testFromSnapshotExclusiveWithRef() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagSnapshotBName = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagSnapshotBName, snapshotBId).commit();
+ table.newFastAppend().appendFile(FILE_C).appendFile(FILE_C).commit();
+ long snapshotCId = table.currentSnapshot().snapshotId();
+
+ IncrementalAppendScan scan2 =
newScan().fromSnapshotExclusive(tagSnapshotAName);
+ Assertions.assertThat(scan2.planFiles()).hasSize(4);
+
+ IncrementalAppendScan scan3 =
+
newScan().fromSnapshotExclusive(tagSnapshotAName).toSnapshot(tagSnapshotBName);
+ Assertions.assertThat(scan3.planFiles()).hasSize(2);
+
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotExclusive(branchName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a tag", branchName));
+
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotExclusive("notExistTag"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Cannot find ref");
+ }
+
+ @Test
+ public void testUseBranch() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagName2 = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotMainBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagName2, snapshotMainBId).commit();
+
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchBId = table.snapshot(branchName).snapshotId();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchCId = table.snapshot(branchName).snapshotId();
+
+ IncrementalAppendScan scan =
newScan().fromSnapshotInclusive(tagSnapshotAName);
+ Assertions.assertThat(scan.planFiles()).hasSize(5);
+
+ IncrementalAppendScan scan2 =
+
newScan().fromSnapshotInclusive(tagSnapshotAName).useBranch(branchName);
+ Assertions.assertThat(scan2.planFiles()).hasSize(3);
+
+ IncrementalAppendScan scan3 =
newScan().toSnapshot(snapshotBranchBId).useBranch(branchName);
+ Assertions.assertThat(scan3.planFiles()).hasSize(2);
+
+ IncrementalAppendScan scan4 =
newScan().toSnapshot(snapshotBranchCId).useBranch(branchName);
+ Assertions.assertThat(scan4.planFiles()).hasSize(3);
+
+ IncrementalAppendScan scan5 =
+ newScan()
+ .fromSnapshotExclusive(tagSnapshotAName)
+ .toSnapshot(snapshotBranchBId)
+ .useBranch(branchName);
+ Assertions.assertThat(scan5.planFiles()).hasSize(1);
+
+ Assertions.assertThatThrownBy(
Review Comment:
thanks, that's helps in understanding what exactly is being tested :rocket:
##########
core/src/test/java/org/apache/iceberg/TestBaseIncrementalAppendScan.java:
##########
@@ -101,6 +305,62 @@ public void testToSnapshot() {
Assert.assertEquals(2, Iterables.size(scan.planFiles()));
}
+ @Test
+ public void testToSnapshotWithTag() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+ table.newFastAppend().appendFile(FILE_B).commit();
+ long snapshotBId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ table.manageSnapshots().createBranch(branchName, snapshotBId).commit();
+
+ String tagSnapshotMainBName = "t1";
+ table.manageSnapshots().createTag(tagSnapshotMainBName,
snapshotBId).commit();
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+
+ String tagSnapshotBranchBName = "t2";
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchBId = table.snapshot(branchName).snapshotId();
+ table.manageSnapshots().createTag(tagSnapshotBranchBName,
snapshotBranchBId).commit();
+
+ /*
+
+ files:FILE_A files:FILE_B files:FILE_B FILE_B
+ ----snapshotA ------ snapshotMainB(tag:t1) -------- currentSnapshot
+ \
+ \
+ \files:FILE_C
+ snapshotBranchB(branch:b1, tag:t2)
+ */
+ IncrementalAppendScan scan = newScan().toSnapshot(tagSnapshotMainBName);
+ Assertions.assertThat(scan.planFiles()).hasSize(2);
+
+ IncrementalAppendScan scan2 = newScan().toSnapshot(tagSnapshotBranchBName);
+ Assertions.assertThat(scan2.planFiles()).hasSize(3);
+ }
+
+ @Test
+ public void testToSnapshotWithNonExistingRef() {
+ Assertions.assertThatThrownBy(() -> newScan().toSnapshot("notExistTag"))
Review Comment:
```suggestion
Assertions.assertThatThrownBy(() ->
newScan().toSnapshot("nonExistingRef"))
```
##########
core/src/test/java/org/apache/iceberg/TestBaseIncrementalAppendScan.java:
##########
@@ -51,6 +51,165 @@ public void testFromSnapshotInclusive() {
Assert.assertEquals(3, Iterables.size(scanWithToSnapshot.planFiles()));
}
+ @Test
+ public void fromSnapshotInclusiveWithNonExistingRef() {
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotInclusive("nonExistingRef"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot find ref: nonExistingRef");
+ }
+
+ @Test
+ public void fromSnapshotInclusiveWithTag() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagSnapshotBName = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagSnapshotBName, snapshotBId).commit();
+ table.newFastAppend().appendFile(FILE_C).appendFile(FILE_C).commit();
+
+ /*
+ files:FILE_A files:FILE_B FILE_B files:FILE_C
FILE_C
+ ---- snapshotAId(tag:t1) ---- snapshotMainB(tag:t2) ---- currentSnapshot
+ */
+ IncrementalAppendScan scan =
newScan().fromSnapshotInclusive(tagSnapshotAName);
+ Assertions.assertThat(scan.planFiles()).hasSize(5);
+
+ IncrementalAppendScan scanWithToSnapshot =
+
newScan().fromSnapshotInclusive(tagSnapshotAName).toSnapshot(tagSnapshotBName);
+ Assertions.assertThat(scanWithToSnapshot.planFiles()).hasSize(3);
+ }
+
+ @Test
+ public void fromSnapshotInclusiveWithBranchShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotInclusive(branchName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a tag", branchName));
+
+ Assertions.assertThatThrownBy(
+ () ->
newScan().fromSnapshotInclusive(snapshotAId).toSnapshot(branchName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a tag", branchName));
+ }
+
+ @Test
+ public void testUseBranch() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagName2 = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotMainBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagName2, snapshotMainBId).commit();
+
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchBId = table.snapshot(branchName).snapshotId();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchCId = table.snapshot(branchName).snapshotId();
+
+ /*
+
+ files:FILE_A files:FILE_B FILE_B files:FILE_B FILE_B
+ ---- snapshotA(tag:t1) ---- snapshotMainB(tag:t2) ---- currentSnapshot
+ \
+ \
+ \files:FILE_C
+ snapshotBranchB
+ \
+ \
+ \files:FILE_C
+ snapshotBranchC(branch:b1)
+ */
+ IncrementalAppendScan scan =
newScan().fromSnapshotInclusive(tagSnapshotAName);
+ Assertions.assertThat(scan.planFiles()).hasSize(5);
+
+ IncrementalAppendScan scan2 =
+
newScan().fromSnapshotInclusive(tagSnapshotAName).useBranch(branchName);
+ Assertions.assertThat(scan2.planFiles()).hasSize(3);
+
+ IncrementalAppendScan scan3 =
newScan().toSnapshot(snapshotBranchBId).useBranch(branchName);
+ Assertions.assertThat(scan3.planFiles()).hasSize(2);
+
+ IncrementalAppendScan scan4 =
newScan().toSnapshot(snapshotBranchCId).useBranch(branchName);
+ Assertions.assertThat(scan4.planFiles()).hasSize(3);
+
+ IncrementalAppendScan scan5 =
+ newScan()
+ .fromSnapshotExclusive(tagSnapshotAName)
+ .toSnapshot(snapshotBranchBId)
+ .useBranch(branchName);
+ Assertions.assertThat(scan5.planFiles()).hasSize(1);
+ }
+
+ @Test
+ public void testUseBranchWithTagShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ Assertions.assertThatThrownBy(
+ () ->
newScan().fromSnapshotInclusive(snapshotAId).useBranch(tagSnapshotAName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a branch", tagSnapshotAName));
+ }
+
+ @Test
+ public void testUseBranchWithInvalidEndSnapshotShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagName2 = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotMainBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagName2, snapshotMainBId).commit();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+
+ /*
+
+ files:FILE_A files:FILE_B FILE_B
+ ---- snapshotA(tag:t1) ---- snapshotMainB(tag:t2)
+ \
+ \
+ \files:FILE_C
+ snapshotBranchB(branch:b1)
+ */
+ Assertions.assertThatThrownBy(
+ () ->
newScan().toSnapshot(snapshotMainBId).useBranch(branchName).planFiles())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("End snapshot is not a valid snapshot on the
current branch");
+ }
+
+ @Test
+ public void testUseBranchWithNonExistingRef() {
+ Assertions.assertThatThrownBy(() -> newScan().useBranch("notExistBranch"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Cannot find ref");
Review Comment:
```suggestion
.hasMessage("Cannot find ref: nonExistingRef");
```
##########
core/src/test/java/org/apache/iceberg/TestBaseIncrementalAppendScan.java:
##########
@@ -51,6 +51,165 @@ public void testFromSnapshotInclusive() {
Assert.assertEquals(3, Iterables.size(scanWithToSnapshot.planFiles()));
}
+ @Test
+ public void fromSnapshotInclusiveWithNonExistingRef() {
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotInclusive("nonExistingRef"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot find ref: nonExistingRef");
+ }
+
+ @Test
+ public void fromSnapshotInclusiveWithTag() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagSnapshotBName = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagSnapshotBName, snapshotBId).commit();
+ table.newFastAppend().appendFile(FILE_C).appendFile(FILE_C).commit();
+
+ /*
+ files:FILE_A files:FILE_B FILE_B files:FILE_C
FILE_C
+ ---- snapshotAId(tag:t1) ---- snapshotMainB(tag:t2) ---- currentSnapshot
+ */
+ IncrementalAppendScan scan =
newScan().fromSnapshotInclusive(tagSnapshotAName);
+ Assertions.assertThat(scan.planFiles()).hasSize(5);
+
+ IncrementalAppendScan scanWithToSnapshot =
+
newScan().fromSnapshotInclusive(tagSnapshotAName).toSnapshot(tagSnapshotBName);
+ Assertions.assertThat(scanWithToSnapshot.planFiles()).hasSize(3);
+ }
+
+ @Test
+ public void fromSnapshotInclusiveWithBranchShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ Assertions.assertThatThrownBy(() ->
newScan().fromSnapshotInclusive(branchName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a tag", branchName));
+
+ Assertions.assertThatThrownBy(
+ () ->
newScan().fromSnapshotInclusive(snapshotAId).toSnapshot(branchName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a tag", branchName));
+ }
+
+ @Test
+ public void testUseBranch() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagName2 = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotMainBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagName2, snapshotMainBId).commit();
+
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchBId = table.snapshot(branchName).snapshotId();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+ long snapshotBranchCId = table.snapshot(branchName).snapshotId();
+
+ /*
+
+ files:FILE_A files:FILE_B FILE_B files:FILE_B FILE_B
+ ---- snapshotA(tag:t1) ---- snapshotMainB(tag:t2) ---- currentSnapshot
+ \
+ \
+ \files:FILE_C
+ snapshotBranchB
+ \
+ \
+ \files:FILE_C
+ snapshotBranchC(branch:b1)
+ */
+ IncrementalAppendScan scan =
newScan().fromSnapshotInclusive(tagSnapshotAName);
+ Assertions.assertThat(scan.planFiles()).hasSize(5);
+
+ IncrementalAppendScan scan2 =
+
newScan().fromSnapshotInclusive(tagSnapshotAName).useBranch(branchName);
+ Assertions.assertThat(scan2.planFiles()).hasSize(3);
+
+ IncrementalAppendScan scan3 =
newScan().toSnapshot(snapshotBranchBId).useBranch(branchName);
+ Assertions.assertThat(scan3.planFiles()).hasSize(2);
+
+ IncrementalAppendScan scan4 =
newScan().toSnapshot(snapshotBranchCId).useBranch(branchName);
+ Assertions.assertThat(scan4.planFiles()).hasSize(3);
+
+ IncrementalAppendScan scan5 =
+ newScan()
+ .fromSnapshotExclusive(tagSnapshotAName)
+ .toSnapshot(snapshotBranchBId)
+ .useBranch(branchName);
+ Assertions.assertThat(scan5.planFiles()).hasSize(1);
+ }
+
+ @Test
+ public void testUseBranchWithTagShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ Assertions.assertThatThrownBy(
+ () ->
newScan().fromSnapshotInclusive(snapshotAId).useBranch(tagSnapshotAName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(String.format("Ref %s is not a branch", tagSnapshotAName));
+ }
+
+ @Test
+ public void testUseBranchWithInvalidEndSnapshotShouldFail() {
+ table.newFastAppend().appendFile(FILE_A).commit();
+ long snapshotAId = table.currentSnapshot().snapshotId();
+
+ String branchName = "b1";
+ String tagSnapshotAName = "t1";
+ table.manageSnapshots().createBranch(branchName, snapshotAId).commit();
+ table.manageSnapshots().createTag(tagSnapshotAName, snapshotAId).commit();
+
+ String tagName2 = "t2";
+ table.newFastAppend().appendFile(FILE_B).appendFile(FILE_B).commit();
+ long snapshotMainBId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createTag(tagName2, snapshotMainBId).commit();
+
+ table.newFastAppend().appendFile(FILE_C).toBranch(branchName).commit();
+
+ /*
+
+ files:FILE_A files:FILE_B FILE_B
+ ---- snapshotA(tag:t1) ---- snapshotMainB(tag:t2)
+ \
+ \
+ \files:FILE_C
+ snapshotBranchB(branch:b1)
+ */
+ Assertions.assertThatThrownBy(
+ () ->
newScan().toSnapshot(snapshotMainBId).useBranch(branchName).planFiles())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("End snapshot is not a valid snapshot on the
current branch");
+ }
Review Comment:
I think this should also test
```
Assertions.assertThatThrownBy(
() ->
newScan().fromSnapshotInclusive(snapshotMainBId).useBranch(branchName).planFiles())
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining(
String.format(
"Starting snapshot (inclusive) %s is not an ancestor of end
snapshot 3",
snapshotMainBId));
```
either in this test method or in a separate one
--
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]
