jecsand838 commented on code in PR #8046:
URL: https://github.com/apache/arrow-rs/pull/8046#discussion_r2255366106


##########
arrow-avro/src/codec.rs:
##########
@@ -897,4 +1010,68 @@ mod tests {
             _ => panic!("Expected SchemaError"),
         }
     }
+
+    #[test]
+    fn test_nested_record_type_reuse_without_namespace() {
+        let nested_record = create_record("Nested", None, vec![
+            create_primitive_field("nested_int", PrimitiveType::Int),
+        ]);
+        let main_record = create_record("Record", None, vec![
+            create_record_field("nested", nested_record),
+            create_ref_field("nestedRecord", "Nested"),
+            create_array_field("nestedArray", 
Schema::TypeName(TypeName::Ref("Nested"))),
+            create_map_field("nestedMap", 
Schema::TypeName(TypeName::Ref("Nested"))),
+        ]);
+        let schema = Schema::Complex(ComplexType::Record(main_record));
+
+        run_schema_test(schema, |avro_field| {
+            let codec = &avro_field.data_type().codec;
+
+            if let Codec::Struct(fields) = codec {
+                assert_eq!(fields.len(), 4);
+                let field_names: Vec<&str> = fields.iter().map(|f| 
f.name()).collect();
+                assert_eq!(field_names, vec!["nested", "nestedRecord", 
"nestedArray", "nestedMap"]);
+            }
+
+            validate_struct_field(codec, 0, "nested", |c| 
validate_nested_record(c, "nested_int", |codec| matches!(codec, Codec::Int32)));
+            validate_struct_field(codec, 1, "nestedRecord", |c| 
validate_nested_record(c, "nested_int", |codec| matches!(codec, Codec::Int32)));
+            validate_struct_field(codec, 2, "nestedArray", |c| {
+                validate_collection_items(c, |item_codec| 
validate_nested_record(item_codec, "nested_int", |codec| matches!(codec, 
Codec::Int32)));
+            });
+            validate_struct_field(codec, 3, "nestedMap", |c| {
+                validate_collection_items(c, |value_codec| 
validate_nested_record(value_codec, "nested_int", |codec| matches!(codec, 
Codec::Int32)));
+            });
+        });
+    }

Review Comment:
   You could probably simplify these tests using an approach like this. 
   
   This is the test I used to replicate the bug you reported. 
   
   ```rust
       #[test]
       fn test_nested_record_resolution() {
           let schema_str = r#"
           {
             "type": "record",
             "name": "Sample",
             "namespace": "test",
             "fields": [
               { "name": "key", "type": "string" },
               {
                 "name": "nested",
                 "type": {
                   "type": "record",
                   "name": "Nested",
                   "fields": [
                     { "name": "nested_int", "type": "int" }
                   ]
                 }
               },
               { "name": "nestedAgain", "type": "Nested" },
               { "name": "nestedArray", "type": { "type": "array", "items": 
"Nested" } },
               { "name": "nestedMap", "type": { "type": "map", "values": 
"Nested" } }
             ]
           }
           "#;
   
           let schema: Schema = serde_json::from_str(schema_str).unwrap();
   
           let mut resolver = Resolver::default();
           let avro_data_type = make_data_type(&schema, None, &mut resolver, 
false, false).unwrap();
   
           if let Codec::Struct(fields) = avro_data_type.codec() {
               assert_eq!(fields.len(), 5);
   
               // key
               assert_eq!(fields[0].name(), "key");
               assert!(matches!(fields[0].data_type().codec(), Codec::Utf8));
   
               // nested
               assert_eq!(fields[1].name(), "nested");
               let nested_data_type = fields[1].data_type();
               if let Codec::Struct(nested_fields) = nested_data_type.codec() {
                   assert_eq!(nested_fields.len(), 1);
                   assert_eq!(nested_fields[0].name(), "nested_int");
                   assert!(matches!(nested_fields[0].data_type().codec(), 
Codec::Int32));
               } else {
                   panic!("'nested' field is not a struct but {:?}", 
nested_data_type.codec());
               }
   
               // nestedAgain
               assert_eq!(fields[2].name(), "nestedAgain");
               let nested_again_data_type = fields[2].data_type();
               assert_eq!(nested_again_data_type.codec().data_type(), 
nested_data_type.codec().data_type());
   
               // nestedArray
               assert_eq!(fields[3].name(), "nestedArray");
               if let Codec::List(item_type) = fields[3].data_type().codec() {
                   assert_eq!(item_type.codec().data_type(), 
nested_data_type.codec().data_type());
               } else {
                   panic!("'nestedArray' field is not a list");
               }
   
               // nestedMap
               assert_eq!(fields[4].name(), "nestedMap");
               if let Codec::Map(value_type) = fields[4].data_type().codec() {
                   assert_eq!(value_type.codec().data_type(), 
nested_data_type.codec().data_type());
               } else {
                   panic!("'nestedMap' field is not a map");
               }
           } else {
               panic!("Top-level schema is not a struct");
           }
       }
   ```



-- 
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: github-unsubscr...@arrow.apache.org

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

Reply via email to