diff --git a/core/src/main/java/org/apache/iceberg/AllManifestsTable.java b/core/src/main/java/org/apache/iceberg/AllManifestsTable.java index 2435de62f0f9..288b6d6baa48 100644 --- a/core/src/main/java/org/apache/iceberg/AllManifestsTable.java +++ b/core/src/main/java/org/apache/iceberg/AllManifestsTable.java @@ -307,7 +307,7 @@ private static class SnapshotEvaluator { private final Expression boundExpr; private SnapshotEvaluator(Expression expr, Types.StructType structType, boolean caseSensitive) { - this.boundExpr = Binder.bind(structType, expr, caseSensitive); + this.boundExpr = Binder.bind(structType, Expressions.rewriteNot(expr), caseSensitive); } private boolean eval(Snapshot snapshot) { @@ -335,11 +335,6 @@ public Boolean alwaysFalse() { return ROWS_CANNOT_MATCH; } - @Override - public Boolean not(Boolean result) { - return !result; - } - @Override public Boolean and(Boolean leftResult, Boolean rightResult) { return leftResult && rightResult; diff --git a/core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java b/core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java index d5a0402a7aa4..7375f7638946 100644 --- a/core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java +++ b/core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java @@ -467,6 +467,129 @@ public void testAllManifestsTableHonorsIgnoreResiduals() throws IOException { validateTaskScanResiduals(scan2, true); } + @TestTemplate + public void testAllManifestsTableNegatedPredicateOnNonSnapshotColumn() { + // Snapshots 1,2,3,4 + preparePartitionedTableData(); + + Table allManifestsTable = new AllManifestsTable(table); + TableScan scan = + allManifestsTable + .newScan() + .filter(Expressions.not(Expressions.equal("content", ManifestContent.DATA.id()))); + + assertThat(scannedPaths(scan)) + .as("A negated predicate on content must not prune snapshots") + .isEqualTo(expectedManifestListPaths(table.snapshots(), 1L, 2L, 3L, 4L)); + } + + @TestTemplate + public void testAllManifestsTableNotEqualPredicateOnNonSnapshotColumn() { + // Snapshots 1,2,3,4 + preparePartitionedTableData(); + + Table allManifestsTable = new AllManifestsTable(table); + TableScan scan = + allManifestsTable + .newScan() + .filter(Expressions.notEqual("content", ManifestContent.DATA.id())); + + assertThat(scannedPaths(scan)) + .as("A not-equal predicate on content must not prune snapshots") + .isEqualTo(expectedManifestListPaths(table.snapshots(), 1L, 2L, 3L, 4L)); + } + + @TestTemplate + public void testAllManifestsTableNegatedOrPredicate() { + // Snapshots 1,2,3,4 + preparePartitionedTableData(); + + long firstSnapshotId = 1L; + + Table allManifestsTable = new AllManifestsTable(table); + TableScan scan = + allManifestsTable + .newScan() + .filter( + Expressions.not( + Expressions.or( + Expressions.equal("reference_snapshot_id", firstSnapshotId), + Expressions.equal("content", ManifestContent.DELETES.id())))); + + /* + * NOT(reference_snapshot_id = firstSnapshotId OR content = DELETES) + * + * is rewritten as: + * + * reference_snapshot_id != firstSnapshotId + * AND content != DELETES + * + * The first snapshot can be pruned using reference_snapshot_id. + * The content predicate cannot prune the other snapshots because content + * is unknown while planning snapshot tasks. + */ + assertThat(scannedPaths(scan)) + .as("Only the explicitly excluded snapshot should be pruned") + .isEqualTo(expectedManifestListPaths(table.snapshots(), 2L, 3L, 4L)); + } + + @TestTemplate + public void testAllManifestsTableNegatedAndPredicate() { + // Snapshots 1,2,3,4 + preparePartitionedTableData(); + + long firstSnapshotId = 1L; + + Table allManifestsTable = new AllManifestsTable(table); + TableScan scan = + allManifestsTable + .newScan() + .filter( + Expressions.not( + Expressions.and( + Expressions.equal("reference_snapshot_id", firstSnapshotId), + Expressions.equal("content", ManifestContent.DELETES.id())))); + + /* + * NOT(reference_snapshot_id = firstSnapshotId AND content = DELETES) + * + * is rewritten as: + * + * reference_snapshot_id != firstSnapshotId + * OR content != DELETES + * + * Even for firstSnapshotId, content != DELETES may match. Therefore, + * no snapshot can be safely pruned. + */ + assertThat(scannedPaths(scan)) + .as("A possibly matching content predicate must keep all snapshots") + .isEqualTo(expectedManifestListPaths(table.snapshots(), 1L, 2L, 3L, 4L)); + } + + @TestTemplate + public void testAllManifestsTableNegatedSnapshotOnlyPredicate() { + // Snapshots 1,2,3,4 + preparePartitionedTableData(); + + Table allManifestsTable = new AllManifestsTable(table); + TableScan scan = + allManifestsTable + .newScan() + .filter( + Expressions.not( + Expressions.or( + Expressions.equal("reference_snapshot_id", 1L), + Expressions.equal("reference_snapshot_id", 2L)))); + + /* + * This confirms that rewriteNot does not merely disable pruning. + * Both excluded snapshots must still be pruned. + */ + assertThat(scannedPaths(scan)) + .as("Negated snapshot-only predicates should still prune snapshots") + .isEqualTo(expectedManifestListPaths(table.snapshots(), 3L, 4L)); + } + @TestTemplate public void testPartitionsTableScanNoFilter() { preparePartitionedTable(); diff --git a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java index 9ca29635f060..1bc93012a596 100644 --- a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java +++ b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java @@ -65,6 +65,7 @@ import org.apache.spark.sql.Encoders; import org.apache.spark.sql.Row; import org.apache.spark.sql.RowFactory; +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.catalyst.util.DateTimeUtils; import org.apache.spark.sql.types.StructType; import org.junit.jupiter.api.AfterEach; @@ -206,6 +207,78 @@ public void testUnpartitionedTable() throws Exception { TestHelpers.nonDerivedSchema(actualFilesDs), expectedFiles.get(1), actualFiles.get(1)); } + @TestTemplate + public void testAllManifestsTableWithNegatedContentFilters() throws NoSuchTableException { + sql( + "CREATE TABLE %s (id bigint, data string) USING iceberg TBLPROPERTIES" + + "('format-version'='%s', 'write.delete.mode'='merge-on-read')", + tableName, formatVersion); + + List records = + Lists.newArrayList( + new SimpleRecord(1, "a"), new SimpleRecord(2, "b"), new SimpleRecord(3, "c")); + + spark + .createDataset(records, Encoders.bean(SimpleRecord.class)) + .coalesce(1) + .writeTo(tableName) + .append(); + + // Create a delete manifest. + sql("DELETE FROM %s WHERE id = 1", tableName); + + List expectedDeleteManifests = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content = 1 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertThat(expectedDeleteManifests) + .as("Test setup should create at least one delete manifest") + .isNotEmpty(); + + List notEqualResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content != 0 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "content != 0 should return all delete manifests", + expectedDeleteManifests, + notEqualResults); + + List alternativeNotEqualResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content <> 0 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "content <> 0 should return all delete manifests", + expectedDeleteManifests, + alternativeNotEqualResults); + + List explicitNotResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE NOT(content = 0) " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "NOT(content = 0) should return all delete manifests", + expectedDeleteManifests, + explicitNotResults); + } + @TestTemplate public void testPositionDeletesTable() throws Exception { sql( diff --git a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java index 9ca29635f060..1bc93012a596 100644 --- a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java +++ b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java @@ -65,6 +65,7 @@ import org.apache.spark.sql.Encoders; import org.apache.spark.sql.Row; import org.apache.spark.sql.RowFactory; +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.catalyst.util.DateTimeUtils; import org.apache.spark.sql.types.StructType; import org.junit.jupiter.api.AfterEach; @@ -206,6 +207,78 @@ public void testUnpartitionedTable() throws Exception { TestHelpers.nonDerivedSchema(actualFilesDs), expectedFiles.get(1), actualFiles.get(1)); } + @TestTemplate + public void testAllManifestsTableWithNegatedContentFilters() throws NoSuchTableException { + sql( + "CREATE TABLE %s (id bigint, data string) USING iceberg TBLPROPERTIES" + + "('format-version'='%s', 'write.delete.mode'='merge-on-read')", + tableName, formatVersion); + + List records = + Lists.newArrayList( + new SimpleRecord(1, "a"), new SimpleRecord(2, "b"), new SimpleRecord(3, "c")); + + spark + .createDataset(records, Encoders.bean(SimpleRecord.class)) + .coalesce(1) + .writeTo(tableName) + .append(); + + // Create a delete manifest. + sql("DELETE FROM %s WHERE id = 1", tableName); + + List expectedDeleteManifests = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content = 1 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertThat(expectedDeleteManifests) + .as("Test setup should create at least one delete manifest") + .isNotEmpty(); + + List notEqualResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content != 0 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "content != 0 should return all delete manifests", + expectedDeleteManifests, + notEqualResults); + + List alternativeNotEqualResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content <> 0 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "content <> 0 should return all delete manifests", + expectedDeleteManifests, + alternativeNotEqualResults); + + List explicitNotResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE NOT(content = 0) " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "NOT(content = 0) should return all delete manifests", + expectedDeleteManifests, + explicitNotResults); + } + @TestTemplate public void testPositionDeletesTable() throws Exception { sql( diff --git a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java index 6e608f8c43a7..d563a56c7da1 100644 --- a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java +++ b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java @@ -65,6 +65,7 @@ import org.apache.spark.sql.Encoders; import org.apache.spark.sql.Row; import org.apache.spark.sql.RowFactory; +import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.catalyst.util.DateTimeUtils; import org.apache.spark.sql.types.StructType; import org.junit.jupiter.api.AfterEach; @@ -206,6 +207,78 @@ public void testUnpartitionedTable() throws Exception { TestHelpers.nonDerivedSchema(actualFilesDs), expectedFiles.get(1), actualFiles.get(1)); } + @TestTemplate + public void testAllManifestsTableWithNegatedContentFilters() throws NoSuchTableException { + sql( + "CREATE TABLE %s (id bigint, data string) USING iceberg TBLPROPERTIES" + + "('format-version'='%s', 'write.delete.mode'='merge-on-read')", + tableName, formatVersion); + + List records = + Lists.newArrayList( + new SimpleRecord(1, "a"), new SimpleRecord(2, "b"), new SimpleRecord(3, "c")); + + spark + .createDataset(records, Encoders.bean(SimpleRecord.class)) + .coalesce(1) + .writeTo(tableName) + .append(); + + // Create a delete manifest. + sql("DELETE FROM %s WHERE id = 1", tableName); + + List expectedDeleteManifests = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content = 1 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertThat(expectedDeleteManifests) + .as("Test setup should create at least one delete manifest") + .isNotEmpty(); + + List notEqualResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content != 0 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "content != 0 should return all delete manifests", + expectedDeleteManifests, + notEqualResults); + + List alternativeNotEqualResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE content <> 0 " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "content <> 0 should return all delete manifests", + expectedDeleteManifests, + alternativeNotEqualResults); + + List explicitNotResults = + sql( + "SELECT content, path, reference_snapshot_id " + + "FROM %s.all_manifests " + + "WHERE NOT(content = 0) " + + "ORDER BY path, reference_snapshot_id", + tableName); + + assertEquals( + "NOT(content = 0) should return all delete manifests", + expectedDeleteManifests, + explicitNotResults); + } + @TestTemplate public void testPositionDeletesTable() throws Exception { sql(