kbendick commented on code in PR #4627:
URL: https://github.com/apache/iceberg/pull/4627#discussion_r859351268
##########
flink/v1.14/flink/src/main/java/org/apache/iceberg/flink/data/FlinkParquetReaders.java:
##########
@@ -116,7 +116,11 @@ public ParquetValueReader<RowData> struct(Types.StructType
expected, GroupType s
int id = field.fieldId();
if (idToConstant.containsKey(id)) {
// containsKey is used because the constant may be null
-
reorderedFields.add(ParquetValueReaders.constant(idToConstant.get(id)));
+
+ // We use the max definition level of the parent node to infer the
max definition level of the constant field
Review Comment:
Nit: typically we avoid the use of pronouns (we, etc) in comments.
Also, does it make sense to first get the result of `typesById` (or similar
look up) and then check if the max definition call is needed? Given the
importance of this code, it would be good to avoid unnecessary looks up if at
all possible.
Though I'm not sure how expensive the `type.getMaxDefinitionLevel` and
`currentPath` functions are.
Or potentially we can make the call lazily evaluated and/or cache the
result. But based on the comment, it seems like the `fieldId` determined is not
found (which it looks like we search for it above anyway).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
