liu-du commented on a change in pull request #1355:
URL: https://github.com/apache/avro/pull/1355#discussion_r779289567
##########
File path:
lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java
##########
@@ -287,6 +288,26 @@ protected void readField(Object record, Field field,
Object oldDatum, ResolvingD
return;
}
}
+
+ if (field.schema().isUnion()) {
Review comment:
Hi opwvhk, thanks for reviewing.
In order to get correct conversion, I need both class type and logical type
(since two different classes are mapped to the same logical type). At line 298,
I can get the class type using `accessor.getField().getType()` and pass it to
`getData().getConversionByClass`. I guess for the same reason, the logic of
line 277-290 also ended up where it is now (see line 283 where it passes
`accessor.getField().getType()` to `getData().getConversionByClass`).
If the logic is moved to `GenericDatumReader#read(Object, Schema,
ResolvingDecoder)` then we _cannot_ get class type without passing an extra
argument into the `GenericDatumReader#read` method. This may requires changing
a lot of the code.
I think `ReflectData.AllowNull` doesn't generate the array of unions since
it only makes class fields nullable (see:
https://github.com/apache/avro/blob/master/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java#L100),
and therefore the logic is put into `ReflectDatumReader#readField` method.
--
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]
