rdblue commented on code in PR #7392:
URL: https://github.com/apache/iceberg/pull/7392#discussion_r1199826105


##########
core/src/main/java/org/apache/iceberg/avro/AvroWithPartnerByStructureVisitor.java:
##########
@@ -93,19 +94,41 @@ private static <P, T> T visitRecord(
   private static <P, T> T visitUnion(
       P type, Schema union, AvroWithPartnerByStructureVisitor<P, T> visitor) {
     List<Schema> types = union.getTypes();
-    Preconditions.checkArgument(
-        AvroSchemaUtil.isOptionSchema(union), "Cannot visit non-option union: 
%s", union);
     List<T> options = Lists.newArrayListWithExpectedSize(types.size());
-    for (Schema branch : types) {
-      if (branch.getType() == Schema.Type.NULL) {
-        options.add(visit(visitor.nullType(), branch, visitor));
-      } else {
-        options.add(visit(type, branch, visitor));
+    if (AvroSchemaUtil.isOptionSchema(union)) {
+      for (Schema branch : types) {
+        if (branch.getType() == Schema.Type.NULL) {
+          options.add(visit(visitor.nullType(), branch, visitor));
+        } else {
+          options.add(visit(type, branch, visitor));
+        }
+      }
+    } else {
+      Types.StructType struct = (Types.StructType) type;
+      int index = 0;
+      for (Schema branch : types) {
+        if (branch.getType() != Schema.Type.NULL) {
+          options.add(visit(fieldTypeByName(type, "field" + index, visitor), 
branch, visitor));

Review Comment:
   This visitor is by structure and not by name. That should be sufficient for 
the purpose of this PR, because we know the structure of the schemas match (one 
was just converted from the other).
   
   Can you remove the by-name code here? It doesn't make sense for this visitor 
and I don't think we need it for the PR.



-- 
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]

Reply via email to