wmoustafa commented on code in PR #4242: URL: https://github.com/apache/iceberg/pull/4242#discussion_r879875274
########## core/src/main/java/org/apache/iceberg/avro/AvroSchemaWithTypeVisitor.java: ########## @@ -79,11 +79,29 @@ private static <T> T visitRecord(Types.StructType struct, Schema record, AvroSch private static <T> T visitUnion(Type type, Schema union, AvroSchemaWithTypeVisitor<T> visitor) { List<Schema> types = union.getTypes(); List<T> options = Lists.newArrayListWithExpectedSize(types.size()); - for (Schema branch : types) { - if (branch.getType() == Schema.Type.NULL) { - options.add(visit((Type) null, branch, visitor)); - } else { - options.add(visit(type, branch, visitor)); + + // simple union case + if (AvroSchemaUtil.isOptionSchema(union)) { + for (Schema branch : types) { + if (branch.getType() == Schema.Type.NULL) { + options.add(visit((Type) null, branch, visitor)); + } else { + options.add(visit(type, branch, visitor)); + } + } + } else { // complex union case + Preconditions.checkArgument(type instanceof Types.StructType, + "Cannot visit invalid Iceberg type: %s for Avro complex union type: %s", type, union); Review Comment: > I don't think that doing this by order is a good idea. That could easily lead to worse cases where we're returning the wrong data. Is the concern because of an out-of-order schema (e.g., the reader schema or the expected schema)? @yiqiangin has tried both cases and an out of order reader schema throws an exception and an out of order expected schema still returns correct results (both after applying [this patch](https://github.com/linkedin/iceberg/pull/108) to address missing fields/projection pruning, so we may need to take that into account). -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org