pvary commented on code in PR #17320:
URL: https://github.com/apache/iceberg/pull/17320#discussion_r3844062527
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetSchemaUtil.java:
##########
@@ -129,12 +131,89 @@ public static Type fieldType(GroupType group, String
name) {
public static MessageType pruneColumns(MessageType fileSchema, Schema
expectedSchema) {
// column order must match the incoming type, so it doesn't matter that
the ids are unordered
- Set<Integer> selectedIds = TypeUtil.getProjectedIds(expectedSchema);
+ Set<Integer> selectedIds =
Sets.newHashSet(TypeUtil.getProjectedIds(expectedSchema));
+ // Retain one real leaf under each struct that projects only constants
like default values,
+ // so its definition level still shows whether its parent struct is null.
+ collectDefinitionLevelProbeIds(expectedSchema.asStruct(), fileSchema,
selectedIds);
return (MessageType)
TypeWithSchemaVisitor.visit(
expectedSchema.asStruct(), fileSchema, new
PruneColumns(selectedIds));
}
+ /**
+ * Walks the expected struct alongside the file schema. For each projected
struct whose fields are
+ * all constants and would otherwise retain no file leaf, adds one leaf id
under the matching file
+ * group so its definition level can signal whether the struct is null.
+ */
+ private static void collectDefinitionLevelProbeIds(
+ Types.StructType expectedStruct, GroupType fileGroup, Set<Integer>
selectedIds) {
+ for (Types.NestedField field : expectedStruct.fields()) {
+ if (!field.type().isStructType()) {
+ continue;
+ }
+
+ Types.StructType expectedFieldStruct = field.type().asStructType();
+ if (expectedFieldStruct.fields().isEmpty()) {
+ // an explicitly empty struct projection has no fields to read and no
default to apply,
+ // so there is no per-row null-ness to preserve
+ continue;
+ }
+
+ Type fileField = fieldById(fileGroup, field.fieldId());
+ if (fileField == null || fileField.isPrimitive()) {
+ continue;
+ }
+
+ GroupType fileFieldGroup = fileField.asGroupType();
+ if (isListOrMap(fileFieldGroup)) {
Review Comment:
I think we need to use an implementation of `TypeWithSchemaVisitor`, since
we can have a array<struct<...>> where the struct has a field with default
value.
--
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]
