yyanyy commented on a change in pull request #1963:
URL: https://github.com/apache/iceberg/pull/1963#discussion_r572543387
##########
File path:
core/src/main/java/org/apache/iceberg/avro/AvroWithPartnerByStructureVisitor.java
##########
@@ -161,4 +186,17 @@ public T map(P sMap, Schema map, T value) {
public T primitive(P type, Schema primitive) {
return null;
}
+
+ // ---------------------------------- Helpers
---------------------------------------------
+
+ private Deque<String> fieldNames = Lists.newLinkedList();
+ private Deque<Schema> parentSchemas = Lists.newLinkedList();
Review comment:
I think I did think of that pattern, and from my notes the main reason I
didn't do it is that it won't be as clean as the existing pattern of
`before/afterField` in other visitors, as different data structure has
different way of retrieving field id information. For example, for fields that
are part of the struct, the field id is stored in its own `Schema.Field` so
that we can directly pass `field` to `before/afterField` within the for loop
when looping through fields; but for map value, the id is stored in the map's
schema instead of its own schema, so that `beforeMapValue` should be passed
with the parent schema. My thought was that to require different visitors to
implement `before/after` with all these different parameters, and to duplicate
the various ID retrieval logic among data types in different visitor
implementations could be messy to reason about, that keeping them here might
actually be cleaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
