fix tests and simplify

apache · nastra · Sep 19, 2025 · Aug 13, 2025 · Aug 14, 2025 · Aug 14, 2025
commit a05ba32d6f2edcdea31193a8b63d383e508cc758
diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml
@@ -1254,6 +1254,10 @@ acceptedBreaks:
       new: "method void org.apache.iceberg.data.parquet.BaseParquetWriter<T>::<init>()"
       justification: "Changing deprecated code"
   "1.9.0":
+    org.apache.iceberg:iceberg-api:
+    - code: "java.method.addedToInterface"
+      new: "method boolean org.apache.iceberg.Accessor<T>::hasOptionalFieldInPath()"
+      justification: "All subclasses implement name"
     org.apache.iceberg:iceberg-common:
     - code: "java.method.visibilityReduced"
       old: "method <R> R org.apache.iceberg.common.DynConstructors.Ctor<C>::invokeChecked(java.lang.Object,\

diff --git a/api/src/main/java/org/apache/iceberg/Accessor.java b/api/src/main/java/org/apache/iceberg/Accessor.java
@@ -26,13 +26,6 @@ public interface Accessor<T> extends Serializable {
 
   Type type();
 
-  /**
-   * Returns true if any ancestor in the access path to this field is optional.
-   */
-  boolean hasOptionalAncestor();
-
-  /**
-   * Returns true if this accessor is optional.
-   */
-  boolean isOptional();
+  /** Returns true if the current field or any ancestor in the access path is optional. */
+  boolean hasOptionalFieldInPath();
 }
diff --git a/api/src/main/java/org/apache/iceberg/Accessors.java b/api/src/main/java/org/apache/iceberg/Accessors.java
@@ -59,13 +59,13 @@ private static class PositionAccessor implements Accessor<StructLike> {
     private final int position;
     private final Type type;
     private final Class<?> javaClass;
-    private final boolean isOptional;
+    private final boolean hasOptionalFieldInPath;
 
     PositionAccessor(int pos, boolean isOptional, Type type) {
       this.position = pos;
       this.type = type;
       this.javaClass = type.typeId().javaClass();
-      this.isOptional = isOptional;
+      this.hasOptionalFieldInPath = isOptional;
     }
 
     @Override
@@ -87,13 +87,8 @@ public Class<?> javaClass() {
     }
 
     @Override
-    public boolean isOptional() {
-      return isOptional;
-    }
-
-    @Override
-    public boolean hasOptionalAncestor() {
-      return false;
+    public boolean hasOptionalFieldInPath() {
+      return hasOptionalFieldInPath;
     }
 
     @Override
@@ -107,14 +102,14 @@ private static class Position2Accessor implements Accessor<StructLike> {
     private final int p1;
     private final Type type;
     private final Class<?> javaClass;
-    private final boolean isOptional;
+    private final boolean hasOptionalFieldInPath;
 
     Position2Accessor(int pos, PositionAccessor wrapped, boolean isOptional) {
       this.p0 = pos;
       this.p1 = wrapped.position();
       this.type = wrapped.type();
       this.javaClass = wrapped.javaClass();
-      this.isOptional = isOptional;
+      this.hasOptionalFieldInPath = isOptional || wrapped.hasOptionalFieldInPath();
     }
 
     @Override
@@ -132,13 +127,8 @@ public Class<?> javaClass() {
     }
 
     @Override
-    public boolean isOptional() {
-      return isOptional;
-    }
-
-    @Override
-    public boolean hasOptionalAncestor() {
-      return false;
+    public boolean hasOptionalFieldInPath() {
+      return hasOptionalFieldInPath;
     }
 
     @Override
@@ -153,15 +143,15 @@ private static class Position3Accessor implements Accessor<StructLike> {
     private final int p2;
     private final Type type;
     private final Class<?> javaClass;
-    private final boolean isOptional;
+    private final boolean hasOptionalFieldInPath;
 
     Position3Accessor(int pos, Position2Accessor wrapped, boolean isOptional) {
       this.p0 = pos;
       this.p1 = wrapped.p0;
       this.p2 = wrapped.p1;
       this.type = wrapped.type();
       this.javaClass = wrapped.javaClass();
-      this.isOptional = isOptional;
+      this.hasOptionalFieldInPath = isOptional || wrapped.hasOptionalFieldInPath();
     }
 
     @Override
@@ -175,13 +165,8 @@ public Type type() {
     }
 
     @Override
-    public boolean isOptional() {
-      return isOptional;
-    }
-
-    @Override
-    public boolean hasOptionalAncestor() {
-      return false;
+    public boolean hasOptionalFieldInPath() {
+      return hasOptionalFieldInPath;
     }
 
     @Override
@@ -193,12 +178,12 @@ public String toString() {
   private static class WrappedPositionAccessor implements Accessor<StructLike> {
     private final int position;
     private final Accessor<StructLike> accessor;
-    private final boolean isOptional;
+    private final boolean hasOptionalFieldInPath;
 
     WrappedPositionAccessor(int pos, Accessor<StructLike> accessor, boolean isOptional) {
       this.position = pos;
       this.accessor = accessor;
-      this.isOptional = isOptional;
+      this.hasOptionalFieldInPath = isOptional || accessor.hasOptionalFieldInPath();
     }
 
     @Override
@@ -216,13 +201,8 @@ public Type type() {
     }
 
     @Override
-    public boolean isOptional() {
-      return isOptional;
-    }
-
-    @Override
-    public boolean hasOptionalAncestor() {
-      return accessor.isOptional() || accessor.hasOptionalAncestor();
+    public boolean hasOptionalFieldInPath() {
+      return hasOptionalFieldInPath;
     }
 
     @Override

diff --git a/api/src/main/java/org/apache/iceberg/expressions/BoundReference.java b/api/src/main/java/org/apache/iceberg/expressions/BoundReference.java
@@ -57,10 +57,9 @@ public Type type() {
 
   @Override
   public boolean producesNull() {
-    // A leaf required field can still evaluate to null if any ancestor struct on the path
-    // is optional and is null at runtime. Accessor implementations indicate this via
-    // hasOptionalAncestor().
-    return field.isOptional() || accessor.hasOptionalAncestor();
+    // A leaf required field can evaluate to null if it is optional itself or any
+    // ancestor on the path is optional.
+    return field.isOptional() || accessor.hasOptionalFieldInPath();
   }
 
   @Override

diff --git a/parquet/src/test/java/org/apache/iceberg/parquet/TestBloomRowGroupFilter.java b/parquet/src/test/java/org/apache/iceberg/parquet/TestBloomRowGroupFilter.java
@@ -297,9 +297,7 @@ public void testNotNull() {
     shouldRead =
         new ParquetBloomRowGroupFilter(SCHEMA, notNull("struct_not_null.int_field"))
             .shouldRead(parquetSchema, rowGroupMetadata, bloomStore);
-    assertThat(shouldRead)
-        .as("Should read: this field is required and are always not-null")
-        .isTrue();
+    assertThat(shouldRead).as("Should read: bloom filter doesn't help").isTrue();
   }
 
   @Test
@@ -322,9 +320,7 @@ public void testIsNull() {
     shouldRead =
         new ParquetBloomRowGroupFilter(SCHEMA, isNull("struct_not_null.int_field"))
             .shouldRead(parquetSchema, rowGroupMetadata, bloomStore);
-    assertThat(shouldRead)
-        .as("Should skip: this field is required and are always not-null")
-        .isFalse();
+    assertThat(shouldRead).as("Should read: bloom filter doesn't help").isTrue();
   }
 
   @Test