[GitHub] [avro] liu-du commented on a change in pull request #1355: AVRO-3224: Fix ReflectDatumReader.readField when using ReflectData.AllowNull with two conversions that convert to the same avro logical type

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

liu-du commented on a change in pull request #1355:
URL: https://github.com/apache/avro/pull/1355#discussion_r821215780



##########
File path: lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java
##########
@@ -152,68 +160,39 @@ protected Object readArray(Object old, Schema expected, ResolvingDecoder in) thr
         throw new AvroRuntimeException(msg);
       }
     } else {
-      return readJavaArray(array, expectedType, l, in);
-    }
-  }
-
-  private Object readJavaArray(Object array, Schema expectedType, long l, ResolvingDecoder in) throws IOException {
-    Class<?> elementType = array.getClass().getComponentType();
-    if (elementType.isPrimitive()) {
-      return readPrimitiveArray(array, elementType, l, in);
-    } else {
-      return readObjectArray((Object[]) array, expectedType, l, in);
+      return readJavaArray(array, elementClass, elementSchema, l, in);
     }
   }
 
-  private Object readPrimitiveArray(Object array, Class<?> c, long l, ResolvingDecoder in) throws IOException {
-    return ArrayAccessor.readArray(array, c, l, in);
-  }
-
-  private Object readObjectArray(Object[] array, Schema expectedType, long l, ResolvingDecoder in) throws IOException {
-    LogicalType logicalType = expectedType.getLogicalType();
-    Conversion<?> conversion = getData().getConversionFor(logicalType);
-    int index = 0;
-    if (logicalType != null && conversion != null) {
-      do {
-        int limit = index + (int) l;
-        while (index < limit) {
-          Object element = readWithConversion(null, expectedType, logicalType, conversion, in);
-          array[index] = element;
-          index++;
-        }
-      } while ((l = in.arrayNext()) > 0);
+  private Object readJavaArray(Object array, Class<?> elementClass, Schema expected, long l, ResolvingDecoder in)
+      throws IOException {
+    Class<?> arrayElementClass = array.getClass().getComponentType();
+    if (arrayElementClass.isPrimitive()) {
+      return ArrayAccessor.readArray(array, arrayElementClass, l, in);

Review comment:
       the call to "newArray" at line 138 accepts an elementClass, it will only return a primitive array if elementClass==null ,that is, if there is no conversion. I guess this might be a performance optimisation for the common case when there is no conversion? 




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