From 80eda16c465a4514f661dbd76d3b956eb8f24705 Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Tue, 4 Aug 2026 12:20:47 +0200 Subject: [PATCH 1/3] API: Add TypeUtil.isNullable() Determining whether a field may contain nulls requires checking the field itself as well as every field that contains it. Expose this as a reusable utility so callers don't have to reimplement the ancestor walk. --- .../iceberg/expressions/UnboundPredicate.java | 9 +- .../org/apache/iceberg/types/TypeUtil.java | 22 +++ .../apache/iceberg/types/TestTypeUtil.java | 144 ++++++++++++++++++ 3 files changed, 168 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java index 75ca9d5835bc..17f3873d70af 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java +++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java @@ -127,7 +127,7 @@ private Expression bindUnaryOperation(StructType struct, BoundTerm boundTerm) switch (op()) { case IS_NULL: if (!boundTerm.producesNull() - && allAncestorFieldsAreRequired(struct, boundTerm.ref().fieldId())) { + && !TypeUtil.isNullable(struct.asSchema(), boundTerm.ref().fieldId())) { return Expressions.alwaysFalse(); } else if (boundTerm.type().equals(Types.UnknownType.get())) { return Expressions.alwaysTrue(); @@ -135,7 +135,7 @@ && allAncestorFieldsAreRequired(struct, boundTerm.ref().fieldId())) { return new BoundUnaryPredicate<>(Operation.IS_NULL, boundTerm); case NOT_NULL: if (!boundTerm.producesNull() - && allAncestorFieldsAreRequired(struct, boundTerm.ref().fieldId())) { + && !TypeUtil.isNullable(struct.asSchema(), boundTerm.ref().fieldId())) { return Expressions.alwaysTrue(); } else if (boundTerm.type().equals(Types.UnknownType.get())) { return Expressions.alwaysFalse(); @@ -158,11 +158,6 @@ && allAncestorFieldsAreRequired(struct, boundTerm.ref().fieldId())) { } } - private boolean allAncestorFieldsAreRequired(StructType struct, int fieldId) { - return TypeUtil.ancestorFields(struct.asSchema(), fieldId).stream() - .allMatch(Types.NestedField::isRequired); - } - private boolean floatingType(Type.TypeID typeID) { return Type.TypeID.DOUBLE.equals(typeID) || Type.TypeID.FLOAT.equals(typeID); } diff --git a/api/src/main/java/org/apache/iceberg/types/TypeUtil.java b/api/src/main/java/org/apache/iceberg/types/TypeUtil.java index 8e39ae7a43bc..10868e657e98 100644 --- a/api/src/main/java/org/apache/iceberg/types/TypeUtil.java +++ b/api/src/main/java/org/apache/iceberg/types/TypeUtil.java @@ -291,6 +291,28 @@ public static List ancestorFields(Schema schema, int fieldId) return parents; } + /** + * Returns whether a field can evaluate to null within the given schema. + * + *

A field can be null if it is declared optional, or if it is nested inside an optional field. + * For example, a required field inside an optional struct is effectively null whenever that + * struct is null. + * + *

If the field is not present in the schema, this method returns true because its nullability + * cannot be determined. + * + * @param schema The schema that contains the field ID + * @param fieldId The field ID to check + * @return true if the field may be null, false if it cannot be null + */ + public static boolean isNullable(Schema schema, int fieldId) { + Types.NestedField field = schema.findField(fieldId); + + return field == null + || field.isOptional() + || ancestorFields(schema, fieldId).stream().anyMatch(Types.NestedField::isOptional); + } + /** * Assigns fresh ids from the {@link NextID nextId function} for all fields in a type. * diff --git a/api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java b/api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java index d540d239614e..e7881619198e 100644 --- a/api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java +++ b/api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java @@ -972,6 +972,150 @@ public void ancestorFieldsInNestedSchema() { assertThat(TypeUtil.ancestorFields(schema, 17)).containsExactly(pointsElement, points); } + @Test + public void isNullableWithEmptySchemaOrUnknownFields() { + assertThat(TypeUtil.isNullable(new Schema(), 1)).isTrue(); + assertThat(TypeUtil.isNullable(new Schema(required(1, "id", IntegerType.get())), 2)).isTrue(); + } + + @Test + public void isNullableWithTopLevelFields() { + Schema schema = + new Schema( + required(1, "id", IntegerType.get()), optional(2, "data", Types.StringType.get())); + + assertThat(TypeUtil.isNullable(schema, 1)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 2)).isTrue(); + } + + @Test + public void isNullableWithNestedStructs() { + Schema schema = + new Schema( + required( + 1, + "required_location", + Types.StructType.of( + required(3, "required_lat", Types.DoubleType.get()), + optional(4, "optional_lon", Types.DoubleType.get()), + required( + 5, + "required_inner", + Types.StructType.of(required(6, "required_zip", IntegerType.get()))))), + optional( + 2, + "optional_location", + Types.StructType.of( + required(7, "required_lat", Types.DoubleType.get()), + required( + 8, + "required_inner", + Types.StructType.of(required(9, "required_zip", IntegerType.get())))))); + + // a required field is not nullable when every field that contains it is required + assertThat(TypeUtil.isNullable(schema, 1)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 3)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 5)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 6)).isFalse(); + + // an optional field is nullable regardless of the fields that contain it + assertThat(TypeUtil.isNullable(schema, 4)).isTrue(); + + // a required field nested in an optional struct is nullable + assertThat(TypeUtil.isNullable(schema, 2)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 7)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 8)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 9)).isTrue(); + } + + @Test + public void isNullableWithListsAndMaps() { + Schema schema = + new Schema( + required( + 1, + "required_points", + Types.ListType.ofRequired( + 4, Types.StructType.of(required(5, "required_x", Types.LongType.get())))), + optional( + 2, + "optional_points", + Types.ListType.ofOptional( + 6, Types.StructType.of(required(7, "required_x", Types.LongType.get())))), + required( + 3, + "locations", + Types.MapType.ofRequired( + 8, + 9, + Types.StringType.get(), + Types.StructType.of(required(10, "required_lat", Types.DoubleType.get())))), + optional( + 11, + "optional_lines", + Types.ListType.ofRequired( + 12, Types.StructType.of(required(13, "required_x", Types.LongType.get())))), + required( + 14, + "required_shapes", + Types.ListType.ofOptional( + 15, Types.StructType.of(required(16, "required_x", Types.LongType.get())))), + optional( + 17, + "optional_locations", + Types.MapType.ofRequired( + 18, + 19, + Types.StringType.get(), + Types.StructType.of(required(20, "required_lat", Types.DoubleType.get())))), + required( + 21, + "required_locations", + Types.MapType.ofOptional( + 22, + 23, + Types.StringType.get(), + Types.StructType.of(required(24, "required_lat", Types.DoubleType.get()))))); + + // a required element of a required list is not nullable, nor is anything it contains + assertThat(TypeUtil.isNullable(schema, 1)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 4)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 5)).isFalse(); + + // an optional element is nullable, as is anything it contains + assertThat(TypeUtil.isNullable(schema, 2)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 6)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 7)).isTrue(); + + // required map keys and values are not nullable + assertThat(TypeUtil.isNullable(schema, 3)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 8)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 9)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 10)).isFalse(); + + // a required element of an optional list is nullable + assertThat(TypeUtil.isNullable(schema, 11)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 12)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 13)).isTrue(); + + // an optional element of a required list is nullable, as is anything it contains + assertThat(TypeUtil.isNullable(schema, 14)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 15)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 16)).isTrue(); + + // required keys and values of an optional map are nullable + assertThat(TypeUtil.isNullable(schema, 17)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 18)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 19)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 20)).isTrue(); + + // an optional value of a required map is nullable, as is anything it contains, but keys are not + assertThat(TypeUtil.isNullable(schema, 21)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 22)).isFalse(); + assertThat(TypeUtil.isNullable(schema, 23)).isTrue(); + assertThat(TypeUtil.isNullable(schema, 24)).isTrue(); + } + @Test public void testIndexStatsNames() { Schema schema = From b0b7a9a3c2ccc371e3932baa6c52d6c61729975a Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Fri, 7 Aug 2026 12:00:20 +0200 Subject: [PATCH 2/3] Move stuff to Schema.isNullable() + add caching --- .../main/java/org/apache/iceberg/Schema.java | 45 ++++- .../iceberg/expressions/UnboundPredicate.java | 7 +- .../org/apache/iceberg/types/TypeUtil.java | 22 --- .../java/org/apache/iceberg/TestSchema.java | 155 ++++++++++++++++++ .../apache/iceberg/types/TestTypeUtil.java | 144 ---------------- 5 files changed, 200 insertions(+), 173 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/Schema.java b/api/src/main/java/org/apache/iceberg/Schema.java index 3e59998be476..c32a44d45a65 100644 --- a/api/src/main/java/org/apache/iceberg/Schema.java +++ b/api/src/main/java/org/apache/iceberg/Schema.java @@ -80,6 +80,7 @@ public class Schema implements Serializable { private transient Map lowerCaseNameToId = null; private transient Map> idToAccessor = null; private transient Map idToName = null; + private transient Map idToParent = null; private transient Set identifierFieldIdSet = null; private final transient Map idsToReassigned; private final transient Map idsToOriginal; @@ -150,8 +151,8 @@ public Schema( // validate IdentifierField if (identifierFieldIds != null) { - Map idToParent = TypeUtil.indexParents(struct); - identifierFieldIds.forEach(id -> validateIdentifierField(id, lazyIdToField(), idToParent)); + identifierFieldIds.forEach( + id -> validateIdentifierField(id, lazyIdToField(), lazyIdToParent())); } this.identifierFieldIds = @@ -233,6 +234,13 @@ private Map lazyIdToName() { return idToName; } + private Map lazyIdToParent() { + if (idToParent == null) { + this.idToParent = TypeUtil.indexParents(struct); + } + return idToParent; + } + private Map lazyLowerCaseNameToId() { if (lowerCaseNameToId == null) { this.lowerCaseNameToId = ImmutableMap.copyOf(TypeUtil.indexByLowerCaseName(struct)); @@ -449,6 +457,39 @@ public String idToAlias(Integer fieldId) { return null; } + /** + * Returns whether the sub-field identified by the field id can be null. + * + *

A field can be null if it is declared optional, or if it is nested inside an optional field. + * For example, a required field inside an optional struct is effectively null whenever that + * struct is null. Field defaults are not taken into account, so an optional field with a non-null + * default is still nullable. + * + *

If the field is not present in this schema, this method returns true because its nullability + * cannot be determined. + * + * @param id a field id + * @return true if the field may be null, false if it cannot be null + */ + public boolean isNullable(int id) { + NestedField field = findField(id); + if (field == null || field.isOptional()) { + return true; + } + + Map parents = lazyIdToParent(); + Integer parentId = parents.get(id); + while (parentId != null) { + if (findField(parentId).isOptional()) { + return true; + } + + parentId = parents.get(parentId); + } + + return false; + } + /** * Returns an accessor for retrieving the data from {@link StructLike}. * diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java index 17f3873d70af..b07f16998617 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java +++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java @@ -27,7 +27,6 @@ import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.relocated.com.google.common.collect.Sets; import org.apache.iceberg.types.Type; -import org.apache.iceberg.types.TypeUtil; import org.apache.iceberg.types.Types; import org.apache.iceberg.types.Types.StructType; import org.apache.iceberg.util.CharSequenceSet; @@ -126,16 +125,14 @@ public Expression bind(StructType struct, boolean caseSensitive) { private Expression bindUnaryOperation(StructType struct, BoundTerm boundTerm) { switch (op()) { case IS_NULL: - if (!boundTerm.producesNull() - && !TypeUtil.isNullable(struct.asSchema(), boundTerm.ref().fieldId())) { + if (!boundTerm.producesNull() && !struct.asSchema().isNullable(boundTerm.ref().fieldId())) { return Expressions.alwaysFalse(); } else if (boundTerm.type().equals(Types.UnknownType.get())) { return Expressions.alwaysTrue(); } return new BoundUnaryPredicate<>(Operation.IS_NULL, boundTerm); case NOT_NULL: - if (!boundTerm.producesNull() - && !TypeUtil.isNullable(struct.asSchema(), boundTerm.ref().fieldId())) { + if (!boundTerm.producesNull() && !struct.asSchema().isNullable(boundTerm.ref().fieldId())) { return Expressions.alwaysTrue(); } else if (boundTerm.type().equals(Types.UnknownType.get())) { return Expressions.alwaysFalse(); diff --git a/api/src/main/java/org/apache/iceberg/types/TypeUtil.java b/api/src/main/java/org/apache/iceberg/types/TypeUtil.java index 10868e657e98..8e39ae7a43bc 100644 --- a/api/src/main/java/org/apache/iceberg/types/TypeUtil.java +++ b/api/src/main/java/org/apache/iceberg/types/TypeUtil.java @@ -291,28 +291,6 @@ public static List ancestorFields(Schema schema, int fieldId) return parents; } - /** - * Returns whether a field can evaluate to null within the given schema. - * - *

A field can be null if it is declared optional, or if it is nested inside an optional field. - * For example, a required field inside an optional struct is effectively null whenever that - * struct is null. - * - *

If the field is not present in the schema, this method returns true because its nullability - * cannot be determined. - * - * @param schema The schema that contains the field ID - * @param fieldId The field ID to check - * @return true if the field may be null, false if it cannot be null - */ - public static boolean isNullable(Schema schema, int fieldId) { - Types.NestedField field = schema.findField(fieldId); - - return field == null - || field.isOptional() - || ancestorFields(schema, fieldId).stream().anyMatch(Types.NestedField::isOptional); - } - /** * Assigns fresh ids from the {@link NextID nextId function} for all fields in a type. * diff --git a/api/src/test/java/org/apache/iceberg/TestSchema.java b/api/src/test/java/org/apache/iceberg/TestSchema.java index 7abc3505d52e..0c15fa0d127d 100644 --- a/api/src/test/java/org/apache/iceberg/TestSchema.java +++ b/api/src/test/java/org/apache/iceberg/TestSchema.java @@ -21,6 +21,8 @@ import static org.apache.iceberg.Schema.DEFAULT_VALUES_MIN_FORMAT_VERSION; import static org.apache.iceberg.Schema.MIN_FORMAT_VERSIONS; import static org.apache.iceberg.TestHelpers.MAX_FORMAT_VERSION; +import static org.apache.iceberg.types.Types.NestedField.optional; +import static org.apache.iceberg.types.Types.NestedField.required; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -312,4 +314,157 @@ public void testIndexFieldsNestedSchema() { assertThat(fields.get(5).name()).isEqualTo("email"); assertThat(((Types.StructType) fields.get(2).type()).fields()).hasSize(3); } + + @Test + void isNullableWithEmptySchemaOrUnknownFields() { + assertThat(new Schema().isNullable(1)).isTrue(); + assertThat(new Schema(required(1, "id", Types.IntegerType.get())).isNullable(2)).isTrue(); + } + + @Test + void isNullableWithTopLevelFields() { + Schema schema = + new Schema( + required(1, "id", Types.IntegerType.get()), + optional(2, "data", Types.StringType.get())); + + assertThat(schema.isNullable(1)).isFalse(); + assertThat(schema.isNullable(2)).isTrue(); + } + + @Test + void isNullableWithNestedStructs() { + Schema schema = + new Schema( + required( + 1, + "required_location", + Types.StructType.of( + required(3, "required_lat", Types.DoubleType.get()), + optional(4, "optional_lon", Types.DoubleType.get()), + required( + 5, + "required_inner", + Types.StructType.of( + required(6, "required_zip", Types.IntegerType.get()))))), + optional( + 2, + "optional_location", + Types.StructType.of( + required(7, "required_lat", Types.DoubleType.get()), + required( + 8, + "required_inner", + Types.StructType.of( + required(9, "required_zip", Types.IntegerType.get())))))); + + // a required field is not nullable when every field that contains it is required + assertThat(schema.isNullable(1)).isFalse(); + assertThat(schema.isNullable(3)).isFalse(); + assertThat(schema.isNullable(5)).isFalse(); + assertThat(schema.isNullable(6)).isFalse(); + + // an optional field is nullable regardless of the fields that contain it + assertThat(schema.isNullable(4)).isTrue(); + + // a required field nested in an optional struct is nullable + assertThat(schema.isNullable(2)).isTrue(); + assertThat(schema.isNullable(7)).isTrue(); + assertThat(schema.isNullable(8)).isTrue(); + assertThat(schema.isNullable(9)).isTrue(); + } + + @Test + void isNullableWithLists() { + Schema schema = + new Schema( + required( + 1, + "required_points", + Types.ListType.ofRequired( + 2, Types.StructType.of(required(3, "required_x", Types.LongType.get())))), + optional( + 4, + "optional_points", + Types.ListType.ofOptional( + 5, Types.StructType.of(required(6, "required_x", Types.LongType.get())))), + optional( + 7, + "optional_lines", + Types.ListType.ofRequired( + 8, Types.StructType.of(required(9, "required_x", Types.LongType.get())))), + required( + 10, + "required_shapes", + Types.ListType.ofOptional( + 11, Types.StructType.of(required(12, "required_x", Types.LongType.get()))))); + + // a required element of a required list is not nullable, nor is anything it contains + assertThat(schema.isNullable(1)).isFalse(); + assertThat(schema.isNullable(2)).isFalse(); + assertThat(schema.isNullable(3)).isFalse(); + + // an optional element is nullable, as is anything it contains + assertThat(schema.isNullable(4)).isTrue(); + assertThat(schema.isNullable(5)).isTrue(); + assertThat(schema.isNullable(6)).isTrue(); + + // a required element of an optional list is nullable + assertThat(schema.isNullable(7)).isTrue(); + assertThat(schema.isNullable(8)).isTrue(); + assertThat(schema.isNullable(9)).isTrue(); + + // an optional element of a required list is nullable, as is anything it contains + assertThat(schema.isNullable(10)).isFalse(); + assertThat(schema.isNullable(11)).isTrue(); + assertThat(schema.isNullable(12)).isTrue(); + } + + @Test + void isNullableWithMaps() { + Schema schema = + new Schema( + required( + 1, + "required_locations", + Types.MapType.ofRequired( + 2, + 3, + Types.StringType.get(), + Types.StructType.of(required(4, "required_lat", Types.DoubleType.get())))), + optional( + 5, + "optional_locations", + Types.MapType.ofRequired( + 6, + 7, + Types.StringType.get(), + Types.StructType.of(required(8, "required_lat", Types.DoubleType.get())))), + required( + 9, + "locations_with_optional_values", + Types.MapType.ofOptional( + 10, + 11, + Types.StringType.get(), + Types.StructType.of(required(12, "required_lat", Types.DoubleType.get()))))); + + // required map keys and values are not nullable + assertThat(schema.isNullable(1)).isFalse(); + assertThat(schema.isNullable(2)).isFalse(); + assertThat(schema.isNullable(3)).isFalse(); + assertThat(schema.isNullable(4)).isFalse(); + + // required keys and values of an optional map are nullable + assertThat(schema.isNullable(5)).isTrue(); + assertThat(schema.isNullable(6)).isTrue(); + assertThat(schema.isNullable(7)).isTrue(); + assertThat(schema.isNullable(8)).isTrue(); + + // an optional value of a required map is nullable, as is anything it contains, but keys are not + assertThat(schema.isNullable(9)).isFalse(); + assertThat(schema.isNullable(10)).isFalse(); + assertThat(schema.isNullable(11)).isTrue(); + assertThat(schema.isNullable(12)).isTrue(); + } } diff --git a/api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java b/api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java index e7881619198e..d540d239614e 100644 --- a/api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java +++ b/api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java @@ -972,150 +972,6 @@ public void ancestorFieldsInNestedSchema() { assertThat(TypeUtil.ancestorFields(schema, 17)).containsExactly(pointsElement, points); } - @Test - public void isNullableWithEmptySchemaOrUnknownFields() { - assertThat(TypeUtil.isNullable(new Schema(), 1)).isTrue(); - assertThat(TypeUtil.isNullable(new Schema(required(1, "id", IntegerType.get())), 2)).isTrue(); - } - - @Test - public void isNullableWithTopLevelFields() { - Schema schema = - new Schema( - required(1, "id", IntegerType.get()), optional(2, "data", Types.StringType.get())); - - assertThat(TypeUtil.isNullable(schema, 1)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 2)).isTrue(); - } - - @Test - public void isNullableWithNestedStructs() { - Schema schema = - new Schema( - required( - 1, - "required_location", - Types.StructType.of( - required(3, "required_lat", Types.DoubleType.get()), - optional(4, "optional_lon", Types.DoubleType.get()), - required( - 5, - "required_inner", - Types.StructType.of(required(6, "required_zip", IntegerType.get()))))), - optional( - 2, - "optional_location", - Types.StructType.of( - required(7, "required_lat", Types.DoubleType.get()), - required( - 8, - "required_inner", - Types.StructType.of(required(9, "required_zip", IntegerType.get())))))); - - // a required field is not nullable when every field that contains it is required - assertThat(TypeUtil.isNullable(schema, 1)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 3)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 5)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 6)).isFalse(); - - // an optional field is nullable regardless of the fields that contain it - assertThat(TypeUtil.isNullable(schema, 4)).isTrue(); - - // a required field nested in an optional struct is nullable - assertThat(TypeUtil.isNullable(schema, 2)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 7)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 8)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 9)).isTrue(); - } - - @Test - public void isNullableWithListsAndMaps() { - Schema schema = - new Schema( - required( - 1, - "required_points", - Types.ListType.ofRequired( - 4, Types.StructType.of(required(5, "required_x", Types.LongType.get())))), - optional( - 2, - "optional_points", - Types.ListType.ofOptional( - 6, Types.StructType.of(required(7, "required_x", Types.LongType.get())))), - required( - 3, - "locations", - Types.MapType.ofRequired( - 8, - 9, - Types.StringType.get(), - Types.StructType.of(required(10, "required_lat", Types.DoubleType.get())))), - optional( - 11, - "optional_lines", - Types.ListType.ofRequired( - 12, Types.StructType.of(required(13, "required_x", Types.LongType.get())))), - required( - 14, - "required_shapes", - Types.ListType.ofOptional( - 15, Types.StructType.of(required(16, "required_x", Types.LongType.get())))), - optional( - 17, - "optional_locations", - Types.MapType.ofRequired( - 18, - 19, - Types.StringType.get(), - Types.StructType.of(required(20, "required_lat", Types.DoubleType.get())))), - required( - 21, - "required_locations", - Types.MapType.ofOptional( - 22, - 23, - Types.StringType.get(), - Types.StructType.of(required(24, "required_lat", Types.DoubleType.get()))))); - - // a required element of a required list is not nullable, nor is anything it contains - assertThat(TypeUtil.isNullable(schema, 1)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 4)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 5)).isFalse(); - - // an optional element is nullable, as is anything it contains - assertThat(TypeUtil.isNullable(schema, 2)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 6)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 7)).isTrue(); - - // required map keys and values are not nullable - assertThat(TypeUtil.isNullable(schema, 3)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 8)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 9)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 10)).isFalse(); - - // a required element of an optional list is nullable - assertThat(TypeUtil.isNullable(schema, 11)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 12)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 13)).isTrue(); - - // an optional element of a required list is nullable, as is anything it contains - assertThat(TypeUtil.isNullable(schema, 14)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 15)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 16)).isTrue(); - - // required keys and values of an optional map are nullable - assertThat(TypeUtil.isNullable(schema, 17)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 18)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 19)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 20)).isTrue(); - - // an optional value of a required map is nullable, as is anything it contains, but keys are not - assertThat(TypeUtil.isNullable(schema, 21)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 22)).isFalse(); - assertThat(TypeUtil.isNullable(schema, 23)).isTrue(); - assertThat(TypeUtil.isNullable(schema, 24)).isTrue(); - } - @Test public void testIndexStatsNames() { Schema schema = From 0b9e76bc489525fb2a3d35d9235650d7a7ef9136 Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Wed, 12 Aug 2026 15:30:01 +0200 Subject: [PATCH 3/3] rename to isOptional and throw when field can't be found --- .../main/java/org/apache/iceberg/Schema.java | 18 ++-- .../iceberg/expressions/UnboundPredicate.java | 4 +- .../java/org/apache/iceberg/TestSchema.java | 89 ++++++++++--------- 3 files changed, 58 insertions(+), 53 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/Schema.java b/api/src/main/java/org/apache/iceberg/Schema.java index c32a44d45a65..ef1e5ef65ea7 100644 --- a/api/src/main/java/org/apache/iceberg/Schema.java +++ b/api/src/main/java/org/apache/iceberg/Schema.java @@ -458,22 +458,22 @@ public String idToAlias(Integer fieldId) { } /** - * Returns whether the sub-field identified by the field id can be null. + * Returns whether the sub-field identified by the field id is effectively optional. * - *

A field can be null if it is declared optional, or if it is nested inside an optional field. - * For example, a required field inside an optional struct is effectively null whenever that + *

A field is effectively optional if it is declared optional, or if it is nested inside an + * optional field. For example, a required field inside an optional struct is null whenever that * struct is null. Field defaults are not taken into account, so an optional field with a non-null - * default is still nullable. - * - *

If the field is not present in this schema, this method returns true because its nullability - * cannot be determined. + * default is still optional. * * @param id a field id * @return true if the field may be null, false if it cannot be null + * @throws IllegalArgumentException if the field is not present in this schema */ - public boolean isNullable(int id) { + public boolean isOptional(int id) { NestedField field = findField(id); - if (field == null || field.isOptional()) { + Preconditions.checkArgument(field != null, "Cannot find field with id: %s", id); + + if (field.isOptional()) { return true; } diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java index b07f16998617..8527ad4137b2 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java +++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java @@ -125,14 +125,14 @@ public Expression bind(StructType struct, boolean caseSensitive) { private Expression bindUnaryOperation(StructType struct, BoundTerm boundTerm) { switch (op()) { case IS_NULL: - if (!boundTerm.producesNull() && !struct.asSchema().isNullable(boundTerm.ref().fieldId())) { + if (!boundTerm.producesNull() && !struct.asSchema().isOptional(boundTerm.ref().fieldId())) { return Expressions.alwaysFalse(); } else if (boundTerm.type().equals(Types.UnknownType.get())) { return Expressions.alwaysTrue(); } return new BoundUnaryPredicate<>(Operation.IS_NULL, boundTerm); case NOT_NULL: - if (!boundTerm.producesNull() && !struct.asSchema().isNullable(boundTerm.ref().fieldId())) { + if (!boundTerm.producesNull() && !struct.asSchema().isOptional(boundTerm.ref().fieldId())) { return Expressions.alwaysTrue(); } else if (boundTerm.type().equals(Types.UnknownType.get())) { return Expressions.alwaysFalse(); diff --git a/api/src/test/java/org/apache/iceberg/TestSchema.java b/api/src/test/java/org/apache/iceberg/TestSchema.java index 0c15fa0d127d..3a5b40e19f16 100644 --- a/api/src/test/java/org/apache/iceberg/TestSchema.java +++ b/api/src/test/java/org/apache/iceberg/TestSchema.java @@ -316,24 +316,29 @@ public void testIndexFieldsNestedSchema() { } @Test - void isNullableWithEmptySchemaOrUnknownFields() { - assertThat(new Schema().isNullable(1)).isTrue(); - assertThat(new Schema(required(1, "id", Types.IntegerType.get())).isNullable(2)).isTrue(); + void isOptionalWithEmptySchemaOrUnknownFields() { + assertThatThrownBy(() -> new Schema().isOptional(1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot find field with id: 1"); + + assertThatThrownBy(() -> new Schema(required(1, "id", Types.IntegerType.get())).isOptional(2)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot find field with id: 2"); } @Test - void isNullableWithTopLevelFields() { + void isOptionalWithTopLevelFields() { Schema schema = new Schema( required(1, "id", Types.IntegerType.get()), optional(2, "data", Types.StringType.get())); - assertThat(schema.isNullable(1)).isFalse(); - assertThat(schema.isNullable(2)).isTrue(); + assertThat(schema.isOptional(1)).isFalse(); + assertThat(schema.isOptional(2)).isTrue(); } @Test - void isNullableWithNestedStructs() { + void isOptionalWithNestedStructs() { Schema schema = new Schema( required( @@ -359,23 +364,23 @@ void isNullableWithNestedStructs() { required(9, "required_zip", Types.IntegerType.get())))))); // a required field is not nullable when every field that contains it is required - assertThat(schema.isNullable(1)).isFalse(); - assertThat(schema.isNullable(3)).isFalse(); - assertThat(schema.isNullable(5)).isFalse(); - assertThat(schema.isNullable(6)).isFalse(); + assertThat(schema.isOptional(1)).isFalse(); + assertThat(schema.isOptional(3)).isFalse(); + assertThat(schema.isOptional(5)).isFalse(); + assertThat(schema.isOptional(6)).isFalse(); // an optional field is nullable regardless of the fields that contain it - assertThat(schema.isNullable(4)).isTrue(); + assertThat(schema.isOptional(4)).isTrue(); // a required field nested in an optional struct is nullable - assertThat(schema.isNullable(2)).isTrue(); - assertThat(schema.isNullable(7)).isTrue(); - assertThat(schema.isNullable(8)).isTrue(); - assertThat(schema.isNullable(9)).isTrue(); + assertThat(schema.isOptional(2)).isTrue(); + assertThat(schema.isOptional(7)).isTrue(); + assertThat(schema.isOptional(8)).isTrue(); + assertThat(schema.isOptional(9)).isTrue(); } @Test - void isNullableWithLists() { + void isOptionalWithLists() { Schema schema = new Schema( required( @@ -400,28 +405,28 @@ void isNullableWithLists() { 11, Types.StructType.of(required(12, "required_x", Types.LongType.get()))))); // a required element of a required list is not nullable, nor is anything it contains - assertThat(schema.isNullable(1)).isFalse(); - assertThat(schema.isNullable(2)).isFalse(); - assertThat(schema.isNullable(3)).isFalse(); + assertThat(schema.isOptional(1)).isFalse(); + assertThat(schema.isOptional(2)).isFalse(); + assertThat(schema.isOptional(3)).isFalse(); // an optional element is nullable, as is anything it contains - assertThat(schema.isNullable(4)).isTrue(); - assertThat(schema.isNullable(5)).isTrue(); - assertThat(schema.isNullable(6)).isTrue(); + assertThat(schema.isOptional(4)).isTrue(); + assertThat(schema.isOptional(5)).isTrue(); + assertThat(schema.isOptional(6)).isTrue(); // a required element of an optional list is nullable - assertThat(schema.isNullable(7)).isTrue(); - assertThat(schema.isNullable(8)).isTrue(); - assertThat(schema.isNullable(9)).isTrue(); + assertThat(schema.isOptional(7)).isTrue(); + assertThat(schema.isOptional(8)).isTrue(); + assertThat(schema.isOptional(9)).isTrue(); // an optional element of a required list is nullable, as is anything it contains - assertThat(schema.isNullable(10)).isFalse(); - assertThat(schema.isNullable(11)).isTrue(); - assertThat(schema.isNullable(12)).isTrue(); + assertThat(schema.isOptional(10)).isFalse(); + assertThat(schema.isOptional(11)).isTrue(); + assertThat(schema.isOptional(12)).isTrue(); } @Test - void isNullableWithMaps() { + void isOptionalWithMaps() { Schema schema = new Schema( required( @@ -450,21 +455,21 @@ void isNullableWithMaps() { Types.StructType.of(required(12, "required_lat", Types.DoubleType.get()))))); // required map keys and values are not nullable - assertThat(schema.isNullable(1)).isFalse(); - assertThat(schema.isNullable(2)).isFalse(); - assertThat(schema.isNullable(3)).isFalse(); - assertThat(schema.isNullable(4)).isFalse(); + assertThat(schema.isOptional(1)).isFalse(); + assertThat(schema.isOptional(2)).isFalse(); + assertThat(schema.isOptional(3)).isFalse(); + assertThat(schema.isOptional(4)).isFalse(); // required keys and values of an optional map are nullable - assertThat(schema.isNullable(5)).isTrue(); - assertThat(schema.isNullable(6)).isTrue(); - assertThat(schema.isNullable(7)).isTrue(); - assertThat(schema.isNullable(8)).isTrue(); + assertThat(schema.isOptional(5)).isTrue(); + assertThat(schema.isOptional(6)).isTrue(); + assertThat(schema.isOptional(7)).isTrue(); + assertThat(schema.isOptional(8)).isTrue(); // an optional value of a required map is nullable, as is anything it contains, but keys are not - assertThat(schema.isNullable(9)).isFalse(); - assertThat(schema.isNullable(10)).isFalse(); - assertThat(schema.isNullable(11)).isTrue(); - assertThat(schema.isNullable(12)).isTrue(); + assertThat(schema.isOptional(9)).isFalse(); + assertThat(schema.isOptional(10)).isFalse(); + assertThat(schema.isOptional(11)).isTrue(); + assertThat(schema.isOptional(12)).isTrue(); } }