skywalker0618 commented on code in PR #20337:
URL: https://github.com/apache/hudi/pull/20337#discussion_r4297997671
##########
hudi-flink-datasource/hudi-flink1.18.x/src/main/java/org/apache/hudi/table/format/cow/vector/reader/NestedColumnReader.java:
##########
@@ -139,6 +141,17 @@ private Tuple2<LevelDelegation, WritableColumnVector>
readRow(
levelDelegation.getDefinitionLevel(),
levelDelegation.getRepetitionLevel());
+ // Inside an array the row has one position per element, which can exceed
the batch size the
+ // placeholders of absent fields were sized with, so grow them to the
position count.
+ int positionsCount = rowPosition.getPositionsCount();
+ RowType rowType = (RowType) field.getType();
+ for (int i = 0; i < children.size(); i++) {
+ if (children.get(i) == null && vectorLength(finalChildrenVectors[i]) <
positionsCount) {
Review Comment:
Thanks, fixed. I found this problem exists in flink too, and was brought in
hudi during the backport.
##########
hudi-flink-datasource/hudi-flink1.18.x/src/main/java/org/apache/hudi/table/format/cow/ParquetSplitReaderUtil.java:
##########
@@ -562,10 +563,11 @@ private static WritableColumnVector
createWritableColumnVector(
return new HeapDecimalVector(batchSize);
case ARRAY:
ArrayType arrayType = (ArrayType) fieldType;
+ Pair<Type, Integer> element = unwrapListElementType(physicalType,
depth);
return new HeapArrayVector(
batchSize,
createWritableColumnVector(
- batchSize, arrayType.getElementType(), physicalType,
descriptors, depth));
+ batchSize, arrayType.getElementType(), element.getLeft(),
descriptors, element.getRight()));
Review Comment:
Great catch, thanks! The allocator has accepted only bytes for decimals
since HUDI-2430, and arrays of rows only avoided that check because of the
unwrapping bug fixed here. Fixed the allocation and added UTs to cover.
##########
hudi-flink-datasource/hudi-flink1.18.x/src/main/java/org/apache/hudi/table/format/cow/ParquetSplitReaderUtil.java:
##########
@@ -622,6 +624,40 @@ private static WritableColumnVector
createWritableColumnVector(
}
}
+ /**
+ * Returns the element type of a LIST physical type and its depth,
unwrapping it with the same
+ * backward-compatibility rules as {@link #getArrayElementColumn} so that
the vectors line up with
+ * the {@code ParquetField} tree.
+ */
+ private static Pair<Type, Integer> unwrapListElementType(Type listType, int
depth) {
+ Type elementType = listType;
+ String listName = null;
+ int elementDepth = depth;
+ while (!elementType.isPrimitive() && !elementType.isRepetition(REPEATED)) {
+ listName = elementType.getName();
+ elementType = elementType.asGroupType().getType(0);
+ elementDepth++;
+ }
+ if (isThreeLevelListRepeatedType(elementType, listName)) {
+ elementType = elementType.asGroupType().getType(0);
+ elementDepth++;
+ }
+ return Pair.of(elementType, elementDepth);
+ }
+
+ /**
+ * Returns whether the repeated type of a list is the synthetic wrapper of a
three-level list,
+ * rather than the element itself. Type-based counterpart of the check in
+ * {@link #getArrayElementColumn}.
+ */
+ private static boolean isThreeLevelListRepeatedType(Type repeatedType,
@Nullable String listName) {
+ return !repeatedType.isPrimitive()
+ && repeatedType.getLogicalTypeAnnotation() == null
+ && repeatedType.asGroupType().getFieldCount() == 1
+ && !repeatedType.getName().equals("array")
+ && (listName == null || !repeatedType.getName().equals(listName +
"_tuple"));
Review Comment:
Fixed and added test cases too
--
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]
