rdblue commented on a change in pull request #3600:
URL: https://github.com/apache/iceberg/pull/3600#discussion_r759674931
##########
File path: core/src/test/java/org/apache/iceberg/TestDeleteFiles.java
##########
@@ -148,6 +151,154 @@ public void
testAlreadyDeletedFilesAreIgnoredDuringDeletesByRowFilter() {
statuses(Status.DELETED));
}
+ @Test
+ public void testDeleteSomeFilesByRowFilterWithoutPartitionPredicates() {
+ PartitionSpec spec = table.spec();
+
+ DataFile firstDataFile = 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(2L)) // upper bounds
+ ))
+ .build();
+
+ DataFile secondDataFile = 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(8L)), // lower bounds
+ ImmutableMap.of(1, longToBuffer(10L)) // 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 second one using a metrics filter (no partition filter)
+ table.newDelete()
+ .deleteFromRowFilter(Expressions.greaterThan("id", 5))
+ .commit();
+
+ Snapshot deleteSnapshot = table.currentSnapshot();
+ Assert.assertEquals("Should have 1 manifest", 1,
deleteSnapshot.allManifests().size());
+ validateManifestEntries(deleteSnapshot.allManifests().get(0),
+ ids(initialSnapshot.snapshotId(), deleteSnapshot.snapshotId()),
+ files(firstDataFile, secondDataFile),
+ statuses(Status.EXISTING, Status.DELETED));
+ }
+
+ @Test
+ public void testDeleteSomeFilesByRowFilterWithCombinedPredicates() {
+ PartitionSpec spec = table.spec();
+
+ DataFile firstDataFile = DataFiles.builder(spec)
Review comment:
If these are identical to the ones in the other test, can we refactor
and move them into static variables? Maybe with names like
`DATA_BUCKET_0_IDS_0_2`
--
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]