pvary commented on code in PR #17320:
URL: https://github.com/apache/iceberg/pull/17320#discussion_r3933340667
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetValueReaders.java:
##########
@@ -231,6 +251,87 @@ public static ParquetValueReader<?>
replaceWithMetadataReader(
return reader;
}
+ /**
+ * Builds readers for a struct's expected fields, in field order. A field
present in the file uses
+ * its column reader; a field missing from the file uses a metadata or
partition constant, or its
+ * initial default. When no expected field reads a file column, one default
reader is given a
+ * probe column so its definition level tracks the struct's null-ness.
+ */
+ public static List<ParquetValueReader<?>> structFieldReaders(
+ MessageType fileSchema,
+ String[] structPath,
+ List<Types.NestedField> expectedFields,
+ Map<Integer, ParquetValueReader<?>> readersById,
+ Map<Integer, ?> idToConstant,
+ BiFunction<org.apache.iceberg.types.Type, Object, Object>
convertConstant) {
+ int constantDefinitionLevel = fileSchema.getMaxDefinitionLevel(structPath);
+ ColumnDescriptor probe =
+ definitionLevelProbe(
+ fileSchema, structPath, constantDefinitionLevel, expectedFields,
readersById);
+ Integer probeHostId = probe == null ? null :
firstInitialDefaultFieldId(expectedFields);
+
+ List<ParquetValueReader<?>> readers =
Lists.newArrayListWithExpectedSize(expectedFields.size());
+ for (Types.NestedField field : expectedFields) {
+ int id = field.fieldId();
+ ParquetValueReader<?> reader =
+ replaceWithMetadataReader(id, readersById.get(id), idToConstant,
constantDefinitionLevel);
+ ColumnDescriptor fieldProbe = probeHostId != null && id == probeHostId ?
probe : null;
+ readers.add(
+ defaultReader(field, reader, constantDefinitionLevel, fieldProbe,
convertConstant));
+ }
+
+ return readers;
+ }
+
+ private static ParquetValueReader<?> defaultReader(
+ Types.NestedField field,
+ ParquetValueReader<?> reader,
+ int constantDefinitionLevel,
+ ColumnDescriptor probe,
+ BiFunction<org.apache.iceberg.types.Type, Object, Object>
convertConstant) {
+ if (reader != null) {
+ return reader;
+ } else if (field.initialDefault() != null) {
+ Object value = convertConstant.apply(field.type(),
field.initialDefault());
+ return probe != null ? constant(value, probe) : constant(value,
constantDefinitionLevel);
+ } else if (field.isOptional()) {
+ return nulls();
Review Comment:
If we have a nested struct which contains not projected fields, and the
projcted field has no default, then we should return Racord(null), and not null.
Could be tested with a test in ReadFormatModelTest like:
```
@ParameterizedTest
@FieldSource("FILE_FORMATS")
void testNestedProjectionWithoutDefaultWhenParentStructIsNull(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(
2,
"nested",
Types.StructType.of(Types.NestedField.required(3, "inner",
Types.StringType.get())));
Schema writeSchema = new Schema(idField, nestedField);
Record present = GenericRecord.create(writeSchema);
present.setField("id", 1L);
Record presentNested =
GenericRecord.create(nestedField.type().asStructType());
presentNested.setField("inner", "a");
present.setField("nested", presentNested);
Record nullNested = GenericRecord.create(writeSchema);
nullNested.setField("id", 2L);
nullNested.setField("nested", null);
List<Record> genericRecords = List.of(present, nullNested);
writeGenericRecords(fileFormat, writeSchema, genericRecords);
// the only projected field of "nested" is missing from the file and has
no initial default, so
// a present struct must still be read as a struct with a null field,
not as a null struct
Schema expectedSchema =
new Schema(
idField,
Types.NestedField.optional(
2,
"nested",
Types.StructType.of(
Types.NestedField.optional(4, "added",
Types.StringType.get()))));
Types.StructType expectedNestedType =
expectedSchema.findField("nested").type().asStructType();
readAndAssertEngineRecords(
fileFormat,
expectedSchema,
genericRecords,
record -> {
Record expected = GenericRecord.create(expectedSchema);
expected.setField("id", record.getField("id"));
if (record.getField("nested") != null) {
expected.setField("nested",
GenericRecord.create(expectedNestedType));
}
return expected;
});
}
```
--
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]
