diff --git a/core/src/main/java/org/apache/iceberg/BaseOverwriteFiles.java b/core/src/main/java/org/apache/iceberg/BaseOverwriteFiles.java index 06cca61fa7fa..3dd8cad11d90 100644 --- a/core/src/main/java/org/apache/iceberg/BaseOverwriteFiles.java +++ b/core/src/main/java/org/apache/iceberg/BaseOverwriteFiles.java @@ -35,7 +35,7 @@ public class BaseOverwriteFiles extends MergingSnapshotProducer private Long startingSnapshotId = null; private Expression conflictDetectionFilter = null; private boolean validateNewDataFiles = false; - private boolean validateNewDeleteFiles = false; + private boolean validateNewDeletes = false; protected BaseOverwriteFiles(String tableName, TableOperations ops) { super(tableName, ops); @@ -98,7 +98,7 @@ public OverwriteFiles validateNoConflictingData() { @Override public OverwriteFiles validateNoConflictingDeletes() { - this.validateNewDeleteFiles = true; + this.validateNewDeletes = true; failMissingDeletePaths(); return this; } @@ -134,10 +134,11 @@ protected void validate(TableMetadata base) { validateAddedDataFiles(base, startingSnapshotId, dataConflictDetectionFilter()); } - if (validateNewDeleteFiles) { + if (validateNewDeletes) { if (rowFilter() != Expressions.alwaysFalse()) { Expression filter = conflictDetectionFilter != null ? conflictDetectionFilter : rowFilter(); validateNoNewDeleteFiles(base, startingSnapshotId, filter); + validateDeletedDataFiles(base, startingSnapshotId, filter); } if (deletedDataFiles.size() > 0) { diff --git a/core/src/test/java/org/apache/iceberg/TestOverwriteWithValidation.java b/core/src/test/java/org/apache/iceberg/TestOverwriteWithValidation.java index c431c78a2b9c..1938ddddc649 100644 --- a/core/src/test/java/org/apache/iceberg/TestOverwriteWithValidation.java +++ b/core/src/test/java/org/apache/iceberg/TestOverwriteWithValidation.java @@ -747,6 +747,60 @@ public void testConcurrentConflictingPositionDeletesOverwriteByFilter() { overwrite::commit); } + @Test + public void testConcurrentConflictingDataFileDeleteOverwriteByFilter() { + Assert.assertNull("Should be empty table", table.currentSnapshot()); + + table.newAppend() + .appendFile(FILE_DAY_1) + .appendFile(FILE_DAY_2) + .commit(); + + Snapshot firstSnapshot = table.currentSnapshot(); + + OverwriteFiles overwrite = table.newOverwrite() + .overwriteByRowFilter(EXPRESSION_DAY_2) + .addFile(FILE_DAY_2_MODIFIED) + .validateFromSnapshot(firstSnapshot.snapshotId()) + .validateNoConflictingData() + .validateNoConflictingDeletes(); + + table.newOverwrite() + .deleteFile(FILE_DAY_2) + .commit(); + + AssertHelpers.assertThrows("Should reject commit", + ValidationException.class, "Found conflicting deleted files", + overwrite::commit); + } + + @Test + public void testConcurrentNonConflictingDataFileDeleteOverwriteByFilter() { + Assert.assertNull("Should be empty table", table.currentSnapshot()); + + table.newAppend() + .appendFile(FILE_DAY_1) + .appendFile(FILE_DAY_2) + .commit(); + + Snapshot firstSnapshot = table.currentSnapshot(); + + OverwriteFiles overwrite = table.newOverwrite() + .overwriteByRowFilter(EXPRESSION_DAY_2) + .addFile(FILE_DAY_2_MODIFIED) + .validateFromSnapshot(firstSnapshot.snapshotId()) + .validateNoConflictingData() + .validateNoConflictingDeletes(); + + table.newOverwrite() + .deleteFile(FILE_DAY_1) + .commit(); + + overwrite.commit(); + + validateTableFiles(table, FILE_DAY_2_MODIFIED); + } + @Test public void testConcurrentNonConflictingPositionDeletes() { Assume.assumeTrue(formatVersion == 2);