xiaoxuandev commented on code in PR #17764:
URL: https://github.com/apache/iceberg/pull/17764#discussion_r3930119475
##########
core/src/test/java/org/apache/iceberg/TestRewriteFiles.java:
##########
@@ -813,4 +813,221 @@ public void removingDataFileAlsoRemovesDV() {
files(fileADeletes(), fileBDeletes()),
statuses(ManifestEntry.Status.DELETED, ManifestEntry.Status.EXISTING));
}
+
+ @TestTemplate
+ public void
testRewriteOfFileWithSuccessiveDVReplacementsRejectedAsConflict() {
+ assumeThat(formatVersion).isGreaterThanOrEqualTo(3);
+
+ commit(table, table.newAppend().appendFile(FILE_A), branch);
+ Snapshot s0 = latestSnapshot(table, branch);
+
+ DeleteFile dv1 = newDV(FILE_A);
+ commit(table, table.newRowDelta().addDeletes(dv1), branch);
+ Snapshot s1 = latestSnapshot(table, branch);
+
+ DeleteFile dv2 = newDV(FILE_A);
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .removeDeletes(dv1)
+ .addDeletes(dv2)
+ .validateFromSnapshot(s1.snapshotId()),
+ branch);
+
+ assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRewrite()
+ .validateFromSnapshot(s0.snapshotId())
+ .rewriteFiles(
+ ImmutableSet.of(FILE_A),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_B),
+ ImmutableSet.of()),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageContaining("Cannot commit, found new delete for replaced
data file")
+ .hasMessageNotContaining("Can't index multiple DVs");
+ }
+
+ @TestTemplate
+ public void testRewriteOfUnrelatedFileSucceedsWithSuccessiveDVReplacements()
{
+ assumeThat(formatVersion).isGreaterThanOrEqualTo(3);
+
+ commit(table, table.newAppend().appendFile(FILE_A).appendFile(FILE_B),
branch);
+ Snapshot s0 = latestSnapshot(table, branch);
+
+ DeleteFile dv1 = newDV(FILE_A);
+ commit(table, table.newRowDelta().addDeletes(dv1), branch);
+ Snapshot s1 = latestSnapshot(table, branch);
+
+ DeleteFile dv2 = newDV(FILE_A);
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .removeDeletes(dv1)
+ .addDeletes(dv2)
+ .validateFromSnapshot(s1.snapshotId()),
+ branch);
+
+ commit(
+ table,
+ table
+ .newRewrite()
+ .validateFromSnapshot(s0.snapshotId())
+ .rewriteFiles(
+ ImmutableSet.of(FILE_B),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_C),
+ ImmutableSet.of()),
+ branch);
+
+ assertThat(latestSnapshot(table,
branch).operation()).isEqualTo(DataOperations.REPLACE);
+ }
+
+ @TestTemplate
+ public void
testRewriteOfUnrelatedFileSucceedsWhenDVRemovedByReplaceSnapshot() {
+ assumeThat(formatVersion).isGreaterThanOrEqualTo(3);
+
+ commit(table, table.newAppend().appendFile(FILE_A).appendFile(FILE_B),
branch);
+ Snapshot s0 = latestSnapshot(table, branch);
+
+ DeleteFile dv1 = newDV(FILE_A);
+ commit(table, table.newRowDelta().addDeletes(dv1), branch);
+
+ // VALIDATE_ADDED_DELETE_FILES_OPERATIONS excludes "replace", so the
window never opens the
+ // manifest that records the removal of DV1 and DV1 keeps a live entry in
it
+ DeleteFile dv1prime = newDV(FILE_A);
+ commit(
+ table,
+ table
+ .newRewrite()
+ .rewriteFiles(
+ ImmutableSet.of(),
+ ImmutableSet.of(dv1),
+ ImmutableSet.of(),
+ ImmutableSet.of(dv1prime)),
+ branch);
+ Snapshot s2 = latestSnapshot(table, branch);
+ assertThat(s2.operation()).isEqualTo(DataOperations.REPLACE);
+
+ DeleteFile dv2 = newDV(FILE_A);
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .removeDeletes(dv1prime)
+ .addDeletes(dv2)
+ .validateFromSnapshot(s2.snapshotId()),
+ branch);
+
+ commit(
+ table,
+ table
+ .newRewrite()
+ .validateFromSnapshot(s0.snapshotId())
+ .rewriteFiles(
+ ImmutableSet.of(FILE_B),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_C),
+ ImmutableSet.of()),
+ branch);
+
+ assertThat(latestSnapshot(table,
branch).operation()).isEqualTo(DataOperations.REPLACE);
+ }
+
+ @TestTemplate
+ public void
testRewriteWithOldSequenceNumberRejectsSuccessiveDVReplacements() {
+ assumeThat(formatVersion).isGreaterThanOrEqualTo(3);
+
+ commit(table, table.newAppend().appendFile(FILE_A), branch);
+ Snapshot s0 = latestSnapshot(table, branch);
+ long oldSequenceNumber = s0.sequenceNumber();
+
+ DeleteFile dv1 = newDV(FILE_A);
+ commit(table, table.newRowDelta().addDeletes(dv1), branch);
+ Snapshot s1 = latestSnapshot(table, branch);
+
+ DeleteFile dv2 = newDV(FILE_A);
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .removeDeletes(dv1)
+ .addDeletes(dv2)
+ .validateFromSnapshot(s1.snapshotId()),
+ branch);
+
+ assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRewrite()
+ .validateFromSnapshot(s0.snapshotId())
+ .rewriteFiles(
+ ImmutableSet.of(FILE_A), ImmutableSet.of(FILE_D),
oldSequenceNumber),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageContaining("Cannot commit, found new position delete for
replaced data file")
+ .hasMessageNotContaining("Can't index multiple DVs");
+ }
+
+ @TestTemplate
+ public void testRewriteOfUnrelatedFileSucceedsWhenDVIsLiveInTwoManifests() {
+ assumeThat(formatVersion).isGreaterThanOrEqualTo(3);
+
+ commit(
+ table,
table.newAppend().appendFile(FILE_A).appendFile(FILE_B).appendFile(FILE_C),
branch);
+ Snapshot s0 = latestSnapshot(table, branch);
+
+ // both DVs land in one delete manifest
+ commit(table,
table.newRowDelta().addDeletes(newDV(FILE_A)).addDeletes(newDV(FILE_B)),
branch);
+
+ // dropping FILE_B's DV as dangling rewrites that manifest, and the copy
still carries the DV
+ // for FILE_A, leaving it live in both
+ commit(table, table.newDelete().deleteFile(FILE_B), branch);
+
+ commit(
+ table,
+ table
+ .newRewrite()
+ .validateFromSnapshot(s0.snapshotId())
+ .rewriteFiles(
+ ImmutableSet.of(FILE_C),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_D),
+ ImmutableSet.of()),
+ branch);
+
+ assertThat(latestSnapshot(table,
branch).operation()).isEqualTo(DataOperations.REPLACE);
Review Comment:
Updated, thanks!
--
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]
