GH-3114: Fix LogicalType conversions for nested records on Avro <= 1.8 (

#3111) * Fix LogicalType conversions for nested records on Avro <= 1.8 * Move inner field traversal outside finally block
apache · Jan 14, 2025 · d6c80d7 · d6c80d7
1 parent 7f77908
commit d6c80d7
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 7 deletions.
diff --git a/parquet-avro/src/main/java/org/apache/parquet/avro/AvroRecordConverter.java b/parquet-avro/src/main/java/org/apache/parquet/avro/AvroRecordConverter.java
@@ -194,13 +194,13 @@ private static void addLogicalTypeConversion(SpecificData model, Schema schema,
                 model.addLogicalTypeConversion(conversion);
               }
             }
-
-            for (Schema.Field field : schema.getFields()) {
-              addLogicalTypeConversion(model, field.schema(), seenSchemas);
-            }
           } catch (NoSuchFieldException e) {
             // Avro classes without logical types (denoted by the "conversions" field)
           }
+
+          for (Schema.Field field : schema.getFields()) {
+            addLogicalTypeConversion(model, field.schema(), seenSchemas);
+          }
         }
         break;
       case MAP:

diff --git a/parquet-avro/src/test/java/org/apache/parquet/avro/TestAvroRecordConverter.java b/parquet-avro/src/test/java/org/apache/parquet/avro/TestAvroRecordConverter.java
@@ -88,14 +88,22 @@ public void testModelForGenericRecord() {
   public void testModelForSpecificRecordWithLogicalTypesWithDeprecatedAvro1_8() {
     Mockito.when(AvroRecordConverter.getRuntimeAvroVersion()).thenReturn("1.8.2");
 
-    // Test that model is generated correctly
-    final SpecificData model = AvroRecordConverter.getModelForSchema(LogicalTypesTestDeprecated.SCHEMA$);
+    // Test that model is generated correctly when record contains both top-level and nested logical types
+    SpecificData model = AvroRecordConverter.getModelForSchema(LogicalTypesTestDeprecated.SCHEMA$);
     // Test that model is generated correctly
     Collection<Conversion<?>> conversions = model.getConversions();
-    assertEquals(conversions.size(), 3);
+    assertEquals(3, conversions.size());
     assertNotNull(model.getConversionByClass(Instant.class));
     assertNotNull(model.getConversionByClass(LocalDate.class));
     assertNotNull(model.getConversionByClass(LocalTime.class));
+
+    // Test that model is generated correctly when record contains only nested logical types
+    model = AvroRecordConverter.getModelForSchema(NestedOnlyLogicalTypesDeprecated.SCHEMA$);
+    // Test that model is generated correctly
+    conversions = model.getConversions();
+    assertEquals(2, conversions.size());
+    assertNotNull(model.getConversionByClass(LocalDate.class));
+    assertNotNull(model.getConversionByClass(LocalTime.class));
   }
 
   @Test
@@ -147,6 +155,7 @@ public static org.apache.avro.Schema getClassSchema() {
     };
   }
 
+  // An Avro class generated from Avro 1.8 that contains both nested and top-level logical type fields
   @org.apache.avro.specific.AvroGenerated
   public abstract static class LogicalTypesTestDeprecated extends org.apache.avro.specific.SpecificRecordBase
       implements org.apache.avro.specific.SpecificRecord {
@@ -179,4 +188,26 @@ public static org.apache.avro.Schema getClassSchema() {
       new org.apache.avro.data.TimeConversions.TimestampMillisConversion(), null, null
     };
   }
+
+  // An Avro class generated from Avro 1.8 that contains only nested logical type fields
+  @org.apache.avro.specific.AvroGenerated
+  public abstract static class NestedOnlyLogicalTypesDeprecated extends org.apache.avro.specific.SpecificRecordBase
+      implements org.apache.avro.specific.SpecificRecord {
+    public static final org.apache.avro.Schema SCHEMA$ = SchemaBuilder.builder()
+        .record("NestedOnlyLogicalTypesDeprecated")
+        .namespace("org.apache.parquet.avro.TestAvroRecordConverter")
+        .fields()
+        .name("local_date_time")
+        .type(LocalDateTimeTestDeprecated.getClassSchema())
+        .noDefault()
+        .endRecord();
+
+    public static org.apache.avro.Schema getClassSchema() {
+      return SCHEMA$;
+    }
+
+    private static SpecificData MODEL$ = new SpecificData();
+
+    // No top-level conversions field, since logical types are all nested
+  }
 }