matthieun commented on code in PR #988:
URL: https://github.com/apache/parquet-mr/pull/988#discussion_r951644453
##########
parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoSchemaConverter.java:
##########
@@ -79,12 +80,20 @@ public MessageType convert(Class<? extends Message>
protobufClass) {
}
/* Iterates over list of fields. **/
- private <T> GroupBuilder<T> convertFields(GroupBuilder<T> groupBuilder,
List<FieldDescriptor> fieldDescriptors) {
+ private <T> GroupBuilder<T> convertFields(GroupBuilder<T> groupBuilder,
List<FieldDescriptor> fieldDescriptors, List<String> parentNames) {
for (FieldDescriptor fieldDescriptor : fieldDescriptors) {
- groupBuilder =
- addField(fieldDescriptor, groupBuilder)
+ final String name = fieldDescriptor.getFullName();
+ final List<String> newParentNames = new ArrayList<>(parentNames);
+ newParentNames.add(name);
+ if (parentNames.contains(name)) {
Review Comment:
The list is mostly used to keep the ordering, so that the dependency chain
is printed in order in the warning message. I understand that in case the
schema definition is really deep with nested types it might be slower, but
overall that list is not growing any bigger than the deepest nesting in the
schema.
If this is still a concern, I am happy to switch to HashSet at the expense
of maybe dumming down the log message (printing the nesting chain out of order
would not be valuable anyway I think).
--
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]
