rdblue commented on code in PR #12836:
URL: https://github.com/apache/iceberg/pull/12836#discussion_r2051039187
##########
parquet/src/main/java/org/apache/iceberg/data/parquet/BaseParquetReaders.java:
##########
@@ -237,55 +236,37 @@ public ParquetValueReader<?> struct(
int fieldD = type.getMaxDefinitionLevel(path(fieldType.getName())) -
1;
int id = fieldType.getId().intValue();
readersById.put(id, ParquetValueReaders.option(fieldType, fieldD,
fieldReader));
- typesById.put(id, fieldType);
- if (idToConstant.containsKey(id)) {
- maxDefinitionLevelsById.put(id, fieldD);
- }
}
}
- List<Types.NestedField> expectedFields =
- expected != null ? expected.fields() : ImmutableList.of();
+ int constantDefinitionLevel = type.getMaxDefinitionLevel(currentPath());
+ List<Types.NestedField> expectedFields = expected.fields();
List<ParquetValueReader<?>> reorderedFields =
Lists.newArrayListWithExpectedSize(expectedFields.size());
- List<Type> types =
Lists.newArrayListWithExpectedSize(expectedFields.size());
- // Defaulting to parent max definition level
- int defaultMaxDefinitionLevel =
type.getMaxDefinitionLevel(currentPath());
+
for (Types.NestedField field : expectedFields) {
int id = field.fieldId();
- ParquetValueReader<?> reader = readersById.get(id);
- if (idToConstant.containsKey(id)) {
- // containsKey is used because the constant may be null
- int fieldMaxDefinitionLevel =
- maxDefinitionLevelsById.getOrDefault(id,
defaultMaxDefinitionLevel);
- reorderedFields.add(
- ParquetValueReaders.constant(idToConstant.get(id),
fieldMaxDefinitionLevel));
- types.add(null);
- } else if (id == MetadataColumns.ROW_POSITION.fieldId()) {
- reorderedFields.add(ParquetValueReaders.position());
- types.add(null);
- } else if (id == MetadataColumns.IS_DELETED.fieldId()) {
- reorderedFields.add(ParquetValueReaders.constant(false));
- types.add(null);
- } else if (reader != null) {
- reorderedFields.add(reader);
- types.add(typesById.get(id));
- } else if (field.initialDefault() != null) {
- reorderedFields.add(
- ParquetValueReaders.constant(
- convertConstant(field.type(), field.initialDefault()),
- maxDefinitionLevelsById.getOrDefault(id,
defaultMaxDefinitionLevel)));
- types.add(typesById.get(id));
- } else if (field.isOptional()) {
- reorderedFields.add(ParquetValueReaders.nulls());
- types.add(null);
- } else {
- throw new IllegalArgumentException(
- String.format("Missing required field: %s", field.name()));
- }
+ ParquetValueReader<?> reader =
+ ParquetValueReaders.replaceWithMetadataReader(
+ id, readersById.get(id), idToConstant,
constantDefinitionLevel);
+ reorderedFields.add(defaultReader(field, reader,
constantDefinitionLevel));
Review Comment:
The changes here were needed to reduce the complexity of the `struct` method
that was failing checkstyle. I refactored into 2 components:
* `ParquetValueReaders.replaceWithMetadataReader` handles metadata columns
and constants
* `defaultReader` handles defaults based on the schema; this requires
`convertConstant` so it is not shared in `ParquetValueReaders`
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
