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



##########
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:
       I guess the subsequent code wouldn't be **entirely** dead, since 
somebody might have an implementation that turns their special `MyInteger` 
class into a `Type.INT`.  Hypothetically, this change would make it impossible 
for a subclass to specify that all `java.lang.Integer` should be represented by 
a `Type.LONG`, I guess! 
   
   While I doubt anybody is actually doing that, why don't we play it with a 
bit of security and do: 
   
   ```
   String primativeType = getPrimitiveTypeCache().get(datum.getClass());
   ```
   
   where `getPrimitiveTypeCache()` is protected and returns 
`PRIMATIVE_DATUMS_TYPES` by default?  That adds one method call but would give 
a subclass an "escape route" if they do have a specialised behaviour.  
Alternatively, a protected data member that is initialised to the static hash 
instance perhaps?




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


Reply via email to