(parquet-java) branch master updated: GH-3114: Fix LogicalType conversions for nested records on Avro <= 1.8 (#3111)

[fokko]

This is an automated email from the ASF dual-hosted git repository.

fokko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/parquet-java.git


The following commit(s) were added to refs/heads/master by this push:
     new d6c80d7dd GH-3114: Fix LogicalType conversions for nested records on 
Avro <= 1.8 (#3111)
d6c80d7dd is described below

commit d6c80d7ddda59e288970d0a9e40fa94b32ecdcd1
Author: Claire McGinty <clai...@spotify.com>
AuthorDate: Tue Jan 14 10:22:32 2025 -0500

    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/parquet/avro/AvroRecordConverter.java   |  8 ++---
 .../parquet/avro/TestAvroRecordConverter.java      | 37 ++++++++++++++++++++--
 2 files changed, 38 insertions(+), 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
index 441428bfa..a82d0148c 100644
--- 
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 @@ class AvroRecordConverter<T> extends 
AvroConverters.AvroGroupConverter {
                 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
index 76e4b99d0..315320bbd 100644
--- 
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 class TestAvroRecordConverter {
   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 class TestAvroRecordConverter {
     };
   }
 
+  // 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 class TestAvroRecordConverter {
       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
+  }
 }