aokolnychyi commented on a change in pull request #3581:
URL: https://github.com/apache/iceberg/pull/3581#discussion_r753207559
##########
File path: core/src/test/java/org/apache/iceberg/TestDeleteFiles.java
##########
@@ -72,4 +76,79 @@ public void testMultipleDeletes() {
files(FILE_B, FILE_C),
statuses(Status.DELETED, Status.EXISTING));
}
+
+ @Test
+ public void testAlreadyDeletedFilesAreIgnoredDuringDeletesByRowFilter() {
+ PartitionSpec spec = table.spec();
+
+ DataFile firstDataFile = DataFiles.builder(spec)
+ .withPath("/path/to/data-2.parquet")
+ .withFileSizeInBytes(10)
+ .withPartitionPath("data_bucket=0")
+ .withMetrics(new Metrics(5L,
+ null, // no column sizes
+ ImmutableMap.of(1, 5L, 2, 5L), // value count
+ ImmutableMap.of(1, 0L, 2, 0L), // null count
+ null, // no nan value counts
+ ImmutableMap.of(1, longToBuffer(0L)), // lower bounds
+ ImmutableMap.of(1, longToBuffer(10L)) // upper bounds
+ ))
+ .build();
+
+ DataFile secondDataFile = DataFiles.builder(spec)
+ .withPath("/path/to/data-1.parquet")
+ .withFileSizeInBytes(10)
+ .withPartitionPath("data_bucket=0")
+ .withMetrics(new Metrics(5L,
+ null, // no column sizes
+ ImmutableMap.of(1, 5L, 2, 5L), // value count
+ ImmutableMap.of(1, 0L, 2, 0L), // null count
+ null, // no nan value counts
+ ImmutableMap.of(1, longToBuffer(0L)), // lower bounds
+ ImmutableMap.of(1, longToBuffer(4L)) // upper bounds
+ ))
+ .build();
+
+ // add both data files
+ table.newFastAppend()
+ .appendFile(firstDataFile)
+ .appendFile(secondDataFile)
+ .commit();
+
+ Snapshot initialSnapshot = table.currentSnapshot();
+ Assert.assertEquals("Should have 1 manifest", 1,
initialSnapshot.allManifests().size());
+ validateManifestEntries(initialSnapshot.allManifests().get(0),
+ ids(initialSnapshot.snapshotId(), initialSnapshot.snapshotId()),
+ files(firstDataFile, secondDataFile),
+ statuses(Status.ADDED, Status.ADDED));
+
+ // delete the first data file
+ table.newDelete()
+ .deleteFile(firstDataFile)
+ .commit();
+
+ Snapshot deleteSnapshot = table.currentSnapshot();
+ Assert.assertEquals("Should have 1 manifest", 1,
deleteSnapshot.allManifests().size());
+ validateManifestEntries(deleteSnapshot.allManifests().get(0),
+ ids(deleteSnapshot.snapshotId(), initialSnapshot.snapshotId()),
+ files(firstDataFile, secondDataFile),
+ statuses(Status.DELETED, Status.EXISTING));
+
+ // delete the second data file using a row filter
+ // the commit should succeed as there is only one live data file
+ table.newDelete()
Review comment:
This was failing as we were checking the already deleted file.
--
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]
