juntaozhang commented on code in PR #9550:
URL: https://github.com/apache/paimon/pull/9550#discussion_r3941329202
##########
paimon-format/src/main/java/org/apache/paimon/format/parquet/ParquetReaderFactory.java:
##########
@@ -336,11 +337,10 @@ private Type clipParquetType(DataType readType, Type
parquetType) {
// There are two representations for array type in parquet.
// See link:
//
https://impala.apache.org/docs/build/html/topics/impala_parquet_array_resolution.html.
- int level = arrayGroup.getType(0) instanceof GroupType ? 3 : 2;
Type elementType =
clipParquetType(elementReadType,
parquetListElementType(arrayGroup));
- if (level == 3) {
+ if (isThreeLevelList(arrayGroup)) {
Review Comment:
Thanks! Fixed by introduced a LayoutContext that records the list-layout
verdict from the file schema keyed by field path, so reader construction no
longer re-guesses from the reshaped requested schema. Covered by
testReadStructElementWithProjection — writes a Rule 2 LIST<STRUCT<x,y>> file
and reads it back as ARRAY<ROW<x>>.
##########
paimon-format/src/main/java/org/apache/paimon/format/parquet/ParquetSchemaConverter.java:
##########
@@ -490,20 +493,107 @@ public static DataField convertToPaimonField(Type
parquetType) {
return new DataField(parquetType.getId().intValue(),
parquetType.getName(), paimonDataType);
}
+ /** Returns true if the given group is annotated as a Parquet LIST logical
type. */
+ public static boolean isList(GroupType listType) {
+ return listType.getLogicalTypeAnnotation()
+ instanceof LogicalTypeAnnotation.ListLogicalTypeAnnotation;
+ }
+
+ /**
+ * Returns true if the given group is a three-level Parquet list.
+ *
+ * <p>In a three-level list the immediate repeated child is a wrapper
group whose single
+ * non-repeated child is the actual element type. This covers the
canonical layout ({@code list
+ * -> element}) as well as legacy wrappers such as Hive's {@code bag}
layout.
+ *
+ * <p>This corresponds to the Parquet spec's backward-compatibility
<b>Rule 5</b>: a repeated
+ * group that contains exactly one non-repeated child is a wrapper, unless
it matches one of
+ * Rules 1-4.
+ *
+ * <p>The compatibility encodings that are <em>not</em> three-level are:
+ *
+ * <ul>
+ * <li><b>Rule 1</b>: the repeated field is a primitive and is itself
the element type.
+ * <li><b>Rule 2</b>: the repeated field is a group with multiple fields
and is itself the
+ * element type.
+ * <li><b>Rule 3</b>: the repeated field is a group whose single child
is also repeated; the
+ * group itself is the element type.
+ * <li><b>Rule 4</b>: the repeated field is a group named {@code
"array"} or {@code
+ * "<list>_tuple"} with a single child; the group itself is the
element type.
+ * </ul>
+ *
+ * <p>See the Parquet spec: <a
+ *
href="https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#backward-compatibility-rules";>LogicalTypes#Backward-compatibility-rules</a>
+ */
+ public static boolean isThreeLevelList(GroupType listType) {
+ if (!isList(listType)) {
+ return false;
+ }
+
+ // A list must have exactly one repeated child (the middle level).
+ if (listType.getFieldCount() != 1) {
+ return false;
+ }
+ Type middle = listType.getType(0);
+ if (middle.isPrimitive() || middle.getRepetition() !=
Type.Repetition.REPEATED) {
+ return false;
+ }
+ GroupType repeatedGroup = middle.asGroupType();
+
+ // 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) {
Review Comment:
Thanks! Fixed: the unannotated nested repeated group is now inferred as a
nested ARRAY, and clipping plus ColumnIO traversal both treat its repeated
child as the nested list's element. Covered end-to-end by
testReadNestedLegacyList
--
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]
