This is an automated email from the ASF dual-hosted git repository.
szehon-ho pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 6b65d514cd Core: Fix pruning for negated all_manifests filters (#17346)
6b65d514cd is described below
commit 6b65d514cde9f4998d5f7552eab4cf12264fccad
Author: yangshangqing <[email protected]>
AuthorDate: Fri Jul 31 23:29:19 2026 -0400
Core: Fix pruning for negated all_manifests filters (#17346)
---
.../java/org/apache/iceberg/AllManifestsTable.java | 7 +-
.../org/apache/iceberg/TestMetadataTableScans.java | 123 +++++++++++++++++++++
.../spark/extensions/TestMetadataTables.java | 73 ++++++++++++
.../spark/extensions/TestMetadataTables.java | 73 ++++++++++++
.../spark/extensions/TestMetadataTables.java | 73 ++++++++++++
5 files changed, 343 insertions(+), 6 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/AllManifestsTable.java
b/core/src/main/java/org/apache/iceberg/AllManifestsTable.java
index 2435de62f0..288b6d6baa 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 @@ public class AllManifestsTable extends BaseMetadataTable {
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 class AllManifestsTable extends BaseMetadataTable {
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 75edd882d5..34fa316bd4 100644
--- a/core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java
+++ b/core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java
@@ -490,6 +490,129 @@ public class TestMetadataTableScans extends
MetadataTableScanTestBase {
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 9ca29635f0..1bc93012a5 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.Dataset;
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 class TestMetadataTables extends ExtensionsTestBase
{
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<SimpleRecord> 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<Object[]> 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<Object[]> 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<Object[]> 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<Object[]> 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 9ca29635f0..1bc93012a5 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.Dataset;
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 class TestMetadataTables extends ExtensionsTestBase
{
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<SimpleRecord> 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<Object[]> 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<Object[]> 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<Object[]> 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<Object[]> 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 6e608f8c43..d563a56c7d 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.Dataset;
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 class TestMetadataTables extends ExtensionsTestBase
{
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<SimpleRecord> 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<Object[]> 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<Object[]> 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<Object[]> 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<Object[]> 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(
