[GitHub] [avro] belugabehr commented on a change in pull request #1301: AVRO-3184: Cache Datum Type Strings in Resolve Union

[GitBox <gi...@apache.org>]

belugabehr commented on a change in pull request #1301:
URL: https://github.com/apache/avro/pull/1301#discussion_r702475769



##########
File path: lang/java/avro/src/main/java/org/apache/avro/generic/GenericData.java
##########
@@ -892,6 +902,9 @@ public int resolveUnion(Schema union, Object datum) {
   protected String getSchemaName(Object datum) {
     if (datum == null || datum == JsonProperties.NULL_VALUE)
       return Type.NULL.getName();
+    String primativeType = PRIMATIVE_DATUM_TYPES.get(datum.getClass());
+    if (primativeType != null)
+      return primativeType;

Review comment:
       Hello,
   
   It was my desire to remove these methods, but I feared I would be breaking backwards compatibility (even more than this already does).  It may still be possible for developers to override these methods. For example:
   
   ```java
       if (isInteger(datum))
         return Type.INT.getName();
   ```
   
   A developer could allow for the primitive case to be handled by the cache, but also extend the functionality to allow for other logical types to, in this case, return an INT type by overriding `isInteger`.  I'm happy to take away that ability if it offers no real-world use cases, but I didn't want to be so heavy-handed with it.




-- 
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-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org