amogh-jahagirdar commented on code in PR #17513:
URL: https://github.com/apache/iceberg/pull/17513#discussion_r3730830196
##########
api/src/main/java/org/apache/iceberg/types/TypeUtil.java:
##########
@@ -291,6 +291,28 @@ public static List<Types.NestedField>
ancestorFields(Schema schema, int fieldId)
return parents;
}
+ /**
+ * Returns whether a field can evaluate to null within the given schema.
+ *
+ * <p>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.
+ *
+ * <p>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) {
Review Comment:
Sorry if this was already discussed and I missed but what's the semantics of
this API in case the given field has a default value? Looks like based on the
implementation we don't really care, and that as long as the field or any of
its ancestors can possibly be null. So isNullable just indicates if a field +
it's ancestors aren't required, not neccessary that it is literally null.
Which makes sense, but I just want to make sure that the way this is going
to be consumed for pruning is going to fit nicely for fields that do have
default values?
##########
api/src/main/java/org/apache/iceberg/types/TypeUtil.java:
##########
@@ -291,6 +291,28 @@ public static List<Types.NestedField>
ancestorFields(Schema schema, int fieldId)
return parents;
}
+ /**
+ * Returns whether a field can evaluate to null within the given schema.
+ *
+ * <p>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.
+ *
+ * <p>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);
Review Comment:
I'm fine with deferring but I do agree with @anoopj and @dramaticlly that
caching in this case is important especially if we're invoking this per entry.
And I think this is actually another argument for maybe why this API should
actually be on Schema itself since that idToParent state can be lazily kept
there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
