JingsongLi commented on code in PR #9550:
URL: https://github.com/apache/paimon/pull/9550#discussion_r3921451153
##########
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:
[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.
##########
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) {
+ return false;
+ }
+
+ // Rule 4: legacy "array" and "<list>_tuple" encodings are not
wrappers; the repeated
+ // group itself is the element type.
+ return !LEGACY_LIST_ARRAY_NAME.equals(repeatedGroup.getName())
+ && !(listType.getName() +
"_tuple").equals(repeatedGroup.getName());
+ }
+
+ /**
+ * Returns true if the given group follows the canonical three-level
Parquet list layout ({@code
+ * list -> element}).
+ *
+ * <p>The canonical layout is described in the Parquet spec: <a
+ *
href="https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#lists">LogicalTypes#Lists</a>
+ */
+ public static boolean isCanonicalList(Type type) {
+ if (type.isPrimitive()) {
+ return false;
+ }
+
+ GroupType listGroup = type.asGroupType();
+ if (!isThreeLevelList(listGroup)) {
+ return false;
+ }
+
+ Type middle = listGroup.getType(0);
+ Type element = middle.asGroupType().getType(0);
+ return LIST_WRAPPER_NAME.equals(middle.getName())
+ && LIST_ELEMENT_NAME.equals(element.getName());
+ }
+
+ /**
+ * Returns the element type of the given LIST-annotated group according to
the Parquet spec's
+ * backward-compatibility rules for lists.
+ *
+ * <p>For a three-level list (Rule 5) the returned type is the single
child of the repeated
+ * wrapper. For Rules 1-4 the repeated field itself is returned because it
is the element type.
+ */
public static Type parquetListElementType(GroupType listType) {
- int level = listType.getType(0) instanceof GroupType ? 3 : 2;
- if (level == 3) {
- // Level 3 representation of list type.
- // List type should only have one middle group type, which is
repeated, and one element
- // type, which is optional.
+ checkArgument(
+ listType.getLogicalTypeAnnotation()
+ instanceof
LogicalTypeAnnotation.ListLogicalTypeAnnotation,
+ "Expected LIST-annotated group but got: %s",
+ listType);
+
+ if (isThreeLevelList(listType)) {
return listType.getType(0).asGroupType().getType(0);
- } else if (level == 2) {
- // Level 2 representation of list type
- return listType.getType(0);
- } else {
- throw new UnsupportedOperationException(
- "Parquet list type only have two level representation and
three level representation.");
}
+
+ return listType.getType(0);
Review 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.
--
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]