diff --git a/api/src/main/java/org/apache/iceberg/Schema.java b/api/src/main/java/org/apache/iceberg/Schema.java index 3e59998be476..ef1e5ef65ea7 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 is effectively optional. + * + *

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 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 isOptional(int id) { + NestedField field = findField(id); + Preconditions.checkArgument(field != null, "Cannot find field with id: %s", id); + + if (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 75ca9d5835bc..8527ad4137b2 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() - && allAncestorFieldsAreRequired(struct, 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() - && allAncestorFieldsAreRequired(struct, 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(); @@ -158,11 +155,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/test/java/org/apache/iceberg/TestSchema.java b/api/src/test/java/org/apache/iceberg/TestSchema.java index 7abc3505d52e..3a5b40e19f16 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,162 @@ public void testIndexFieldsNestedSchema() { assertThat(fields.get(5).name()).isEqualTo("email"); assertThat(((Types.StructType) fields.get(2).type()).fields()).hasSize(3); } + + @Test + 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 isOptionalWithTopLevelFields() { + Schema schema = + new Schema( + required(1, "id", Types.IntegerType.get()), + optional(2, "data", Types.StringType.get())); + + assertThat(schema.isOptional(1)).isFalse(); + assertThat(schema.isOptional(2)).isTrue(); + } + + @Test + void isOptionalWithNestedStructs() { + 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.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.isOptional(4)).isTrue(); + + // a required field nested in an optional struct is nullable + assertThat(schema.isOptional(2)).isTrue(); + assertThat(schema.isOptional(7)).isTrue(); + assertThat(schema.isOptional(8)).isTrue(); + assertThat(schema.isOptional(9)).isTrue(); + } + + @Test + void isOptionalWithLists() { + 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.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.isOptional(4)).isTrue(); + assertThat(schema.isOptional(5)).isTrue(); + assertThat(schema.isOptional(6)).isTrue(); + + // a required element of an optional list is nullable + 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.isOptional(10)).isFalse(); + assertThat(schema.isOptional(11)).isTrue(); + assertThat(schema.isOptional(12)).isTrue(); + } + + @Test + void isOptionalWithMaps() { + 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.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.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.isOptional(9)).isFalse(); + assertThat(schema.isOptional(10)).isFalse(); + assertThat(schema.isOptional(11)).isTrue(); + assertThat(schema.isOptional(12)).isTrue(); + } }