JingsongLi commented on code in PR #9550:
URL: https://github.com/apache/paimon/pull/9550#discussion_r3986953094
##########
paimon-format/src/main/java/org/apache/paimon/format/parquet/reader/ParquetReaderUtil.java:
##########
@@ -163,34 +173,26 @@ private static ParquetField constructField(
type instanceof ArrayType
? ((ArrayType) type).getElementType()
: ((VectorType) type).getElementType();
- ColumnIO elementTypeColumnIO;
- if (columnIO instanceof GroupColumnIO) {
- GroupColumnIO groupColumnIO = (GroupColumnIO) columnIO;
- if (!StringUtils.isNullOrWhitespaceOnly(fieldName)) {
- // Column lookup is case-insensitive; the wrapper-group
names can
- // therefore differ in case from the requested field name.
- while
(!groupColumnIO.getName().equalsIgnoreCase(fieldName)) {
- groupColumnIO = (GroupColumnIO)
groupColumnIO.getChild(0);
- }
- elementTypeColumnIO = groupColumnIO;
- } else {
- if (elementType instanceof RowType) {
- elementTypeColumnIO = groupColumnIO;
- } else {
- elementTypeColumnIO = groupColumnIO.getChild(0);
- }
- }
- } else if (columnIO instanceof PrimitiveColumnIO) {
- elementTypeColumnIO = columnIO;
- } else {
- throw new RuntimeException(String.format("Unknown ColumnIO,
%s", columnIO));
- }
+ GroupColumnIO groupColumnIO = (GroupColumnIO) columnIO;
+ GroupType requestedGroup = parquetType.asGroupType();
+
+ boolean threeLevel =
+ listLayout.isThreeLevelList(requestedGroup,
groupColumnIO.getFieldPath());
Review Comment:
[P2] Use the original LIST layout when inferring fallback fields too
Reading a newly added nested field can now fail during reader construction.
For a file containing `s.a: ARRAY<ROW<x INT, y INT>>` encoded with Rule 2,
reading only `s.newField` enters the all-fields-missing fallback.
`findCheapestGroupField` retains `a.element.x` to preserve whether `s` is null,
and the extra-field loop above infers that clipped schema as `ARRAY<INT>`.
However, this lookup remembers that the original list element was a struct, so
`constructField` receives a `GroupColumnIO` for the inferred primitive and
throws `ClassCastException`.
I reproduced this with an actual three-row Parquet file, including a null
parent and an empty list: the baseline reader implementation passes, while this
head fails at `ParquetReaderUtil.java:208`. Please make fallback-field type
inference use the original layout as well, or preserve its logical element type
during pruning, and add an end-to-end missing-nested-field projection test.
##########
paimon-format/src/main/java/org/apache/paimon/format/parquet/ParquetSchemaConverter.java:
##########
@@ -462,10 +462,18 @@ public static DataField convertToPaimonField(Type
parquetType) {
parquetType.getId().intValue(), parquetType.getName(),
paimonDataType);
} else {
GroupType groupType = parquetType.asGroupType();
- if (logicalType instanceof
LogicalTypeAnnotation.ListLogicalTypeAnnotation) {
+ if (ParquetListLayoutResolver.isList(groupType)) {
+ Type parquetElementType =
ParquetListLayoutResolver.resolveElementType(groupType);
+ DataType elementDataType =
convertToPaimonField(parquetElementType).type();
+ if (!ParquetListLayoutResolver.isThreeLevelList(groupType)) {
+ // Rules 1-4: the repeated node itself is the element. A
REPEATED node is
+ // never null, so the element type is not nullable.
+ elementDataType = elementDataType.notNull();
+ }
+ paimonDataType = new ArrayType(elementDataType);
+ } else if
(ParquetListLayoutResolver.isLegacyNestedList(groupType)) {
paimonDataType =
- new ArrayType(
-
convertToPaimonField(parquetListElementType(groupType)).type());
+ new
ArrayType(convertToPaimonField(groupType.getType(0)).type().notNull());
Review Comment:
[P2] Preserve MAP annotations before applying legacy nested-list inference
`isLegacyNestedList` also matches a repeated MAP group containing its single
repeated `key_value` child. Because this branch precedes the MAP branch, a
legacy list with this shape is inferred as `ARRAY<ARRAY<ROW<key, value>>>`
instead of `ARRAY<MAP<STRING, INT>>`:
```text
optional group my_list (LIST) {
repeated group array (MAP) {
repeated group key_value {
required binary key (STRING);
optional int32 value;
}
}
}
```
This is produced by [parquet-avro's old LIST
encoding](https://github.com/apache/parquet-java/blob/master/parquet-avro/src/main/java/org/apache/parquet/avro/AvroSchemaConverter.java#L218-L234):
it converts array elements with `REPEATED` repetition and preserves that
repetition for MAP elements. A converter probe confirms that the MAP node
itself converted to `MapType` on the baseline and now converts to `ArrayType`.
Whole-list inference was already incorrect on the baseline, so this is a gap in
the new compatibility handling, rather than a claim that these complete files
previously inferred correctly.
Please handle MAP annotations before this fallback, or restrict the fallback
to unannotated groups, and add an inference test for an array of maps using the
old Avro LIST layout.
--
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]
