rdblue commented on a change in pull request #2925:
URL: https://github.com/apache/iceberg/pull/2925#discussion_r698069158
##########
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:
It isn't clear to me why FILE_E was deleted. Is that in the same
partition as FILE_A or FILE_B? If so, then we need to do something to make the
test more readable so it is clear why this is correct.
--
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]
