szehon-ho commented on a change in pull request #2925:
URL: https://github.com/apache/iceberg/pull/2925#discussion_r702452787
##########
File path: core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
##########
@@ -253,6 +253,189 @@ public void testValidationSuccess() {
statuses(Status.ADDED, Status.ADDED));
}
+ @Test
+ public void testReplaceConflictPartitionedTable() {
+ table.newFastAppend()
+ .appendFile(FILE_A)
+ .appendFile(FILE_B)
+ .commit();
+
+ TableMetadata base = readMetadata();
+ long baseId = base.currentSnapshot().snapshotId();
+
+ // Concurrent Replace
+ table.newReplacePartitions()
+ .addFile(FILE_A)
+ .commit();
+
+ ReplacePartitions replace = table.newReplacePartitions()
+ .validateFromSnapshot(baseId)
+ .validateNoConflictingAppends()
+ .addFile(FILE_A)
+ .addFile(FILE_B);
+
+ AssertHelpers.assertThrows("Should reject commit with file matching
partitions replaced",
+ ValidationException.class,
+ "Found conflicting files that can contain records matching true:
[/path/to/data-a.parquet]",
+ replace::commit);
+ }
+
+ @Test
+ public void testAppendConflictPartitionedTable() {
+ table.newFastAppend()
+ .appendFile(FILE_A)
+ .commit();
+
+ TableMetadata base = readMetadata();
+ long baseId = base.currentSnapshot().snapshotId();
+
+ // Concurrent Insert
+ table.newFastAppend()
+ .appendFile(FILE_B)
+ .commit();
+
+ ReplacePartitions replace = table.newReplacePartitions()
+ .validateFromSnapshot(baseId)
+ .validateNoConflictingAppends()
+ .addFile(FILE_A)
+ .addFile(FILE_B);
+
+ AssertHelpers.assertThrows("Should reject commit with file matching
partitions replaced",
+ ValidationException.class,
+ "Found conflicting files that can contain records matching true:
[/path/to/data-b.parquet]",
+ replace::commit);
+ }
+
+ @Test
+ public void testNoReplaceConflictPartitionedTable() {
+ table.newFastAppend()
+ .appendFile(FILE_A)
+ .commit();
+
+ TableMetadata base = readMetadata();
+ long baseId = base.currentSnapshot().snapshotId();
+
+ // Concurrent Insert
+ table.newReplacePartitions()
+ .addFile(FILE_E)
+ .commit();
+
+ table.newReplacePartitions()
+ .validateFromSnapshot(baseId)
+ .addFile(FILE_A)
+ .addFile(FILE_B)
+ .commit();
+
+ long replaceId = readMetadata().currentSnapshot().snapshotId();
+ Assert.assertEquals("Table should have 2 manifest",
+ 2, table.currentSnapshot().allManifests().size());
+ validateManifestEntries(table.currentSnapshot().allManifests().get(0),
+ ids(replaceId, replaceId),
+ files(FILE_A, FILE_B),
+ statuses(Status.ADDED, Status.ADDED));
+
+ validateManifestEntries(table.currentSnapshot().allManifests().get(1),
+ ids(replaceId),
+ files(FILE_E),
+ statuses(Status.DELETED));
Review comment:
Yea FILE_E is same partition as FILE_A. I added a comment a few lines
earlier explaining, will it suffice?
--
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]
