[GitHub] [pulsar] vroyer commented on a change in pull request #10428: [Issue #10427] Add AvroSchema UUID support fix

[GitBox]

vroyer commented on a change in pull request #10428:
URL: https://github.com/apache/pulsar/pull/10428#discussion_r635353156



##########
File path: 
pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/util/SchemaUtil.java
##########
@@ -88,8 +90,10 @@ public static Schema extractAvroSchema(SchemaDefinition 
schemaDefinition, Class
         try {
             return 
parseAvroSchema(pojo.getDeclaredField("SCHEMA$").get(null).toString());
         } catch (NoSuchFieldException | IllegalAccessException | 
IllegalArgumentException ignored) {
-            return schemaDefinition.getAlwaysAllowNull() ? 
ReflectData.AllowNull.get().getSchema(pojo)
-                    : ReflectData.get().getSchema(pojo);
+            ReflectData reflectData = schemaDefinition.getAlwaysAllowNull() ? 
ReflectData.AllowNull.get()
+                    : ReflectData.get();
+            AvroSchema.addLogicalTypeConversions(reflectData, 
schemaDefinition.isJsr310ConversionEnabled());

Review comment:
       Because there is in fact 2 singleton instances in the AVRO ReflectData 
class: the ReflectData.INSTANCE and the ReflectData.AllowNull.INSTANCE. So when 
your schema allows nulls, you need to register logical types conversion in the 
ReflectData.AllowNull.INSTANCE. That is already done like that in 
https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/StructSchema.java#L96
 (deprecated class)




-- 
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:
us...@infra.apache.org