[format] Follow Parquet list backward-compatibility rules in schema clipping - #9550
[format] Follow Parquet list backward-compatibility rules in schema clipping#9550juntaozhang wants to merge 1 commit into
Conversation
| clipParquetType(elementReadType, parquetListElementType(arrayGroup)); | ||
|
|
||
| if (level == 3) { | ||
| if (isThreeLevelList(arrayGroup)) { |
There was a problem hiding this comment.
[P1] Preserve the original Rule 2 interpretation after clipping
Rule 2 is identified from the original two-field repeated group here, but after recursive projection the rebuilt requested schema may contain only one field. ParquetReaderUtil.getArrayElementColumn then reclassifies that clipped group as a Rule 5 wrapper and unwraps it. For LIST<STRUCT<x,y>> read as ARRAY<ROW>, reader construction pairs the RowType with a PrimitiveColumnIO and throws ClassCastException. Please carry the original list interpretation into field construction, or otherwise keep Rule 2 unambiguous after projection, and add a one-field projection read test.
|
|
||
| // Rule 5: the repeated group is a wrapper containing exactly one non-repeated child. | ||
| if (repeatedGroup.getFieldCount() != 1 | ||
| || repeatedGroup.getType(0).getRepetition() == Type.Repetition.REPEATED) { |
There was a problem hiding this comment.
[P1] Complete Rule 3 handling beyond element selection
This correctly keeps the repeated group as the element, but downstream schema conversion still treats its repeated child as a scalar. With the Rule 3 shape added in the tests and two inner values [8, 9], the inferred type returns only 8; using the semantically correct nested array type instead fails during clipping. Please convert the nested repeated child as an array and align clipping and ColumnIO traversal, with an end-to-end multi-value read test.
| "Parquet list type only have two level representation and three level representation."); | ||
| } | ||
|
|
||
| return listType.getType(0); |
There was a problem hiding this comment.
[P2] Preserve required element nullability for Rules 1-4
The returned element node still has REPEATED repetition, while convertToPaimonField only applies notNull() to REQUIRED nodes. As a result, Rules 1-4 are converted to nullable Paimon elements even though the Parquet compatibility rules define them as required. Please force the converted element DataType to non-null for non-Rule-5 layouts, without changing the physical repeated Type used by clipping, and add conversion-level nullability assertions.
|
Overall assessment: HIGH RISK Requirement fit: SUPPORTED I consider this PR high risk in its current form. It changes persisted Parquet schema interpretation across schema conversion, requested-schema clipping, and reader construction. Focused reproductions exposed both silent data loss in Rule 3 handling and a deterministic reader-construction failure for a Rule 2 projection, plus incorrect element nullability for Rules 1-4. Helper-shape unit tests alone are not sufficient for this compatibility-sensitive path. Please address the inline findings and add end-to-end legacy LIST read coverage before merging. |
2 participants
Purpose
Follow Parquet's backward-compatibility rules for the LIST logical type when clipping Parquet schemas, so that nested variant column pruning and schema conversion correctly distinguish wrapper groups from element types across all standard and legacy list encodings:
arrayor<list>_tupleis the element type.Previously the list-structure check did not fully apply these rules, which could misidentify legacy two-level encodings as three-level lists and return the wrong element type during nested variant pruning.
Tests
mvn -pl paimon-format -DwildcardSuites=none -Dtest=ParquetSchemaConverterTest test