pvary commented on code in PR #17320:
URL: https://github.com/apache/iceberg/pull/17320#discussion_r3933116583
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetSchemaUtil.java:
##########
@@ -129,12 +131,94 @@ 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 the struct is null
+ TypeWithSchemaVisitor.visit(
+ expectedSchema.asStruct(),
+ fileSchema,
+ new DefinitionLevelProbeSelector(fileSchema, selectedIds));
return (MessageType)
TypeWithSchemaVisitor.visit(
expectedSchema.asStruct(), fileSchema, new
PruneColumns(selectedIds));
}
+ /**
+ * Adds one leaf id under each projected struct whose fields are all
constants and would otherwise
+ * retain no file leaf. That leaf's definition level is what still shows
whether the struct is
+ * null.
+ */
+ private static class DefinitionLevelProbeSelector extends
TypeWithSchemaVisitor<Void> {
+ private final MessageType fileSchema;
+ private final Set<Integer> selectedIds;
+
+ private DefinitionLevelProbeSelector(MessageType fileSchema, Set<Integer>
selectedIds) {
+ this.fileSchema = fileSchema;
+ this.selectedIds = selectedIds;
+ }
+
+ @Override
+ public Void struct(Types.StructType expected, GroupType struct, List<Void>
fields) {
+ // nothing projected under this struct, so there is nothing to probe for
+ if (expected == null || expected.fields().isEmpty()) {
+ return null;
+ }
+
+ // a struct that can never be null needs no probe
+ if (fileSchema.getMaxDefinitionLevel(currentPath()) <= 0) {
+ return null;
+ }
+
+ List<ColumnDescriptor> leaves = leafColumns(fileSchema, currentPath());
+ // add a probe leaf only if no real leaf under the struct is already read
+ boolean readsRealLeaf = leaves.stream().anyMatch(leaf ->
selectedIds.contains(leafId(leaf)));
+ if (!readsRealLeaf && !leaves.isEmpty()) {
+ selectedIds.add(leafId(leaves.get(0)));
+ }
+
+ return null;
+ }
+
+ @Override
+ public Void variant(Types.VariantType expected, GroupType variantGroup,
Void result) {
+ return null;
+ }
+ }
+
+ /** Returns the leaf columns with ids under the given path. */
+ static List<ColumnDescriptor> leafColumns(MessageType fileSchema, String[]
path) {
+ List<ColumnDescriptor> columns = Lists.newArrayList();
+ for (ColumnDescriptor column : fileSchema.getColumns()) {
+ // a probe leaf is kept in the read set by id, so a leaf without one
cannot be a probe
+ if (column.getPrimitiveType().getId() == null) {
Review Comment:
We need to handle the repetition level here. Otherwise leafs inside maps or
lists not handled correctly.
Could be checked by this in BaseFormatModelTests:
```
@ParameterizedTest
@FieldSource("FILE_FORMATS")
void testNestedDefaultValueWhenParentStructWithListIsNull(FileFormat
fileFormat)
throws IOException {
assumeSupports(fileFormat, FEATURE_READER_DEFAULT);
Types.NestedField idField = Types.NestedField.required(1, "id",
Types.LongType.get());
Types.NestedField nestedField =
Types.NestedField.optional("nested")
.withId(2)
.ofType(
Types.StructType.of(
Types.NestedField.optional(
3, "tags", Types.ListType.ofRequired(4,
Types.StringType.get())),
Types.NestedField.required(5, "inner",
Types.StringType.get())))
.build();
Schema writeSchema = new Schema(idField, nestedField);
Types.StructType nestedType = nestedField.type().asStructType();
// alternate present and null structs, with a different number of list
elements per row so that
// a reader tracking the list column would fall behind by more than one
value per row
List<Record> genericRecords = Lists.newArrayList();
for (int i = 0; i < 5; i += 1) {
Record record = GenericRecord.create(writeSchema);
record.setField("id", (long) i);
if (i % 2 == 0) {
Record nested = GenericRecord.create(nestedType);
nested.setField("tags", IntStream.range(0, i).mapToObj(j -> "tag-" +
j).toList());
nested.setField("inner", "inner-" + i);
record.setField("nested", nested);
}
genericRecords.add(record);
}
writeGenericRecords(fileFormat, writeSchema, genericRecords);
Schema expectedSchema = schemaWithOnlyDefaultedNestedField();
readAndAssertEngineRecords(
fileFormat, expectedSchema, genericRecords,
defaultedNestedRecord(expectedSchema));
}
```
--
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]
