hudi-agent commented on code in PR #20337:
URL: https://github.com/apache/hudi/pull/20337#discussion_r4279736033
##########
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;
Review Comment:
🤖 nit: `unwrapListElementType` duplicates the unwrapping logic of
`getArrayElementColumn`; would it make sense to note (or share) which parts
must stay in sync so they don't drift apart?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink1.18.x/src/main/java/org/apache/hudi/table/format/cow/vector/reader/NestedColumnReader.java:
##########
@@ -139,6 +141,16 @@ 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();
+ for (int i = 0; i < children.size(); i++) {
Review Comment:
🤖 nit: could you hoist `((RowType) field.getType())` into a local variable
outside the loop, and consider extracting this placeholder-growing loop into a
small helper so `readRow` stays focused?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
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);
Review Comment:
🤖 non-blocking: `hudi-flink2.2.x` is the default profile and the only one CI
runs (the `flink1.18` matrix entries in `bot.yml` are commented out), and it
has the same `ARRAY` branch bug, so the new tests won't run in CI and the
default bundle still hits the NPE. Would it make sense to land this in
`hudi-flink2.2.x` too (or in place of 1.18), and add 2.2.x to the follow-up
list in the description?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
