[ 
https://issues.apache.org/jira/browse/AVRO-3433?focusedWorklogId=737785&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-737785
 ]

ASF GitHub Bot logged work on AVRO-3433:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 07/Mar/22 21:02
            Start Date: 07/Mar/22 21:02
    Worklog Time Spent: 10m 
      Work Description: travisbrown commented on a change in pull request #1582:
URL: https://github.com/apache/avro/pull/1582#discussion_r821099959



##########
File path: lang/rust/avro/src/types.rs
##########
@@ -1323,4 +1356,269 @@ mod tests {
             JsonValue::String("936da01f-9abd-4d9d-80c7-02af85c822a8".into())
         );
     }
+
+    #[test]
+    fn test_recursive_resolves() {
+        let schema = Schema::parse_str(
+            r#"
+        {
+            "type":"record",
+            "name":"TestStruct",
+            "fields": [
+                {
+                    "name":"a",
+                    "type":{
+                        "type":"record",
+                        "name": "Inner",
+                        "fields": [ {
+                            "name":"z",
+                            "type":"int"
+                        }]
+                    }
+                },
+                {
+                    "name":"b",
+                    "type":"Inner"
+                }
+            ]
+        }"#,
+        )
+        .unwrap();
+
+        let inner_value1 = Value::Record(vec![("z".into(), Value::Int(3))]);
+        let inner_value2 = Value::Record(vec![("z".into(), Value::Int(6))]);
+        let outer = Value::Record(vec![("a".into(), inner_value1), 
("b".into(), inner_value2)]);
+        outer
+            .resolve(&schema)
+            .expect("Record definition defined in one field must be availible 
in other field");
+    }
+
+    #[test]
+    fn test_recursive_resolves2() {
+        let schema = Schema::parse_str(
+            r#"
+        {
+            "type":"record",
+            "name":"TestStruct",
+            "fields": [
+                {
+                    "name":"a",
+                    "type":{
+                        "type":"array",
+                        "items": {
+                            "type":"record",
+                            "name": "Inner",
+                            "fields": [ {
+                                "name":"z",
+                                "type":"int"
+                            }]
+                        }
+                    }
+                },
+                {
+                    "name":"b",
+                    "type": {
+                        "type":"map",
+                        "values":"Inner"
+                    }
+                }
+            ]
+        }"#,
+        )
+        .unwrap();
+
+        let inner_value1 = Value::Record(vec![("z".into(), Value::Int(3))]);
+        let inner_value2 = Value::Record(vec![("z".into(), Value::Int(6))]);
+        let outer_value = Value::Record(vec![
+            ("a".into(), Value::Array(vec![inner_value1])),
+            (
+                "b".into(),
+                Value::Map(vec![("akey".into(), 
inner_value2)].into_iter().collect()),
+            ),
+        ]);
+        outer_value
+            .resolve(&schema)
+            .expect("Record defined in array definition must be resolveable 
from map");
+    }
+
+    #[test]
+    fn test_recursive_resolves3() {
+        let schema = Schema::parse_str(
+            r#"
+        {
+            "type":"record",
+            "name":"TestStruct",
+            "fields": [
+                {
+                    "name":"a",
+                    "type":{
+                        "type":"record",
+                        "name": "Inner",
+                        "fields": [ {
+                            "name":"z",
+                            "type":"int"
+                        }]
+                    }
+                },
+                {
+                    "name":"b",
+                    "type": {
+                        "type":"map",
+                        "values":"Inner"
+                    }
+                }
+            ]
+        }"#,
+        )
+        .unwrap();
+
+        let inner_value1 = Value::Record(vec![("z".into(), Value::Int(3))]);
+        let inner_value2 = Value::Record(vec![("z".into(), Value::Int(6))]);
+        let outer_value = Value::Record(vec![
+            ("a".into(), inner_value1),
+            (
+                "b".into(),
+                Value::Map(vec![("akey".into(), 
inner_value2)].into_iter().collect()),
+            ),
+        ]);
+        outer_value
+            .resolve(&schema)
+            .expect("Record defined in record field must be resolvable from 
map field");
+    }
+
+    #[test]
+    fn test_recursive_resolves4() {
+        let schema = Schema::parse_str(
+            r#"
+        {
+            "type":"record",
+            "name":"TestStruct",
+            "fields": [
+                {
+                    "name":"a",
+                    "type":{
+                        "type":"record",
+                        "name": "Inner",
+                        "fields": [ {
+                            "name":"z",
+                            "type":"int"
+                        }]
+                    }
+                },
+                {
+                    "name":"b",
+                    "type": {
+                        "type":"record",
+                        "name": "InnerWrapper",
+                        "fields": [ {
+                            "name":"j",
+                            "type":"Inner"
+                        }]
+                    }
+                }
+            ]
+        }"#,
+        )
+        .unwrap();
+
+        let inner_value1 = Value::Record(vec![("z".into(), Value::Int(3))]);
+        let inner_value2 = Value::Record(vec![(
+            "j".into(),
+            Value::Record(vec![("z".into(), Value::Int(6))]),
+        )]);
+        let outer_value =
+            Value::Record(vec![("a".into(), inner_value1), ("b".into(), 
inner_value2)]);
+        outer_value.resolve(&schema).expect("Record schema defined in field 
must be resolvable in Record schema defined in other field");
+    }
+
+    #[test]
+    fn test_recursive_resolves5() {
+        let schema = Schema::parse_str(
+            r#"
+        {
+            "type":"record",
+            "name":"TestStruct",
+            "fields": [
+                {
+                    "name":"a",
+                    "type":{
+                        "type":"map",
+                        "values": {
+                            "type":"record",
+                            "name": "Inner",
+                            "fields": [ {
+                                "name":"z",
+                                "type":"int"
+                            }]
+                        }
+                    }
+                },
+                {
+                    "name":"b",
+                    "type": {
+                        "type":"array",
+                        "items":"Inner"
+                    }
+                }
+            ]
+        }"#,
+        )
+        .unwrap();
+
+        let inner_value1 = Value::Record(vec![("z".into(), Value::Int(3))]);
+        let inner_value2 = Value::Record(vec![("z".into(), Value::Int(6))]);
+        let outer_value = Value::Record(vec![
+            (
+                "a".into(),
+                Value::Map(vec![("akey".into(), 
inner_value2)].into_iter().collect()),
+            ),
+            ("b".into(), Value::Array(vec![inner_value1])),
+        ]);
+        outer_value
+            .resolve(&schema)
+            .expect("Record defined in map definition must be resolveable from 
array");
+    }
+
+    #[test]
+    fn test_recursive_resolves6() {
+        let schema = Schema::parse_str(
+            r#"
+        {
+            "type":"record",
+            "name":"TestStruct",
+            "fields": [
+                {
+                    "name":"a",
+                    "type":["null", {
+                        "type":"record",
+                        "name": "Inner",
+                        "fields": [ {
+                            "name":"z",
+                            "type":"int"
+                        }]
+                    }]
+                },
+                {
+                    "name":"b",
+                    "type":"Inner"
+                }
+            ]
+        }"#,
+        )
+        .unwrap();
+
+        let inner_value1 = Value::Record(vec![("z".into(), Value::Int(3))]);
+        let inner_value2 = Value::Record(vec![("z".into(), Value::Int(6))]);
+        let outer1 = Value::Record(vec![
+            ("a".into(), inner_value1),
+            ("b".into(), inner_value2.clone()),
+        ]);
+        outer1
+            .resolve(&schema)
+            .expect("Record definition defined in union must be resolvabled in 
other field");
+        let outer2 = Value::Record(vec![("a".into(), Value::Null), 
("b".into(), inner_value2)]);

Review comment:
       @martin-g Thanks! That does seem to resolve the resolution issue for me. 
I'm now running into what I think is a similar problem during encoding while 
writing: it's crashing on [this 
`unwrap`](https://github.com/apache/avro/blob/44b8cd23756b752bf9d0976574453c712dcd44d9/lang/rust/avro/src/encode.rs#L62).
 I haven't looked closely, but it seems like exactly the same issue (if the 
value hasn't required the part of the schema with the definition to be looked 
at, the reference lookup fails).
   
   In the longer term do you think it might be better to carry around the 
name-definition hash map in the schema, rather than having to rebuild it for 
each item in multiple places?




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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 737785)
    Time Spent: 3h 50m  (was: 3h 40m)

> Rust: The canonical form should preserve schema references
> ----------------------------------------------------------
>
>                 Key: AVRO-3433
>                 URL: https://issues.apache.org/jira/browse/AVRO-3433
>             Project: Apache Avro
>          Issue Type: Bug
>          Components: rust
>    Affects Versions: 1.12.0
>            Reporter: Martin Tzvetanov Grigorov
>            Assignee: Martin Tzvetanov Grigorov
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 3h 50m
>  Remaining Estimate: 0h
>
> Reported at 
> [https://github.com/flavray/avro-rs/issues/182#issuecomment-1059762821]
> =================================
> There still seems to be an issue with {{can't refine}} errors, at least in 
> some non-recursive cases. Take the following example:
> {code:java}
> fn main() {
>     let schema = r#"
>     {
>       "name": "test.test",
>       "type": "record",
>       "fields": [
>         {
>           "name": "bar",
>           "type": { "name": "test.foo", "type": "record", "fields": [{ 
> "name": "id", "type": "long" }] }
>         },
>         { "name": "baz", "type": "test.foo" }
>       ]
>     }
>     "#;
>     let schema = apache_avro::schema::Schema::parse_str(&schema).unwrap();
>     println!("{}", serde_json::to_string(&schema).unwrap());
> } {code}
> This prints the following (the same thing happens if the {{test.foo}} 
> definition is in a separate file):
> {code:java}
> $ target/release/avro-test | jq
> {
>   "type": "record",
>   "name": "test.test",
>   "fields": [
>     {
>       "name": "bar",
>       "type": {
>         "type": "record",
>         "name": "test.foo",
>         "fields": [
>           {
>             "name": "id",
>             "type": "long"
>           }
>         ]
>       }
>     },
>     {
>       "name": "baz",
>       "type": {
>         "type": "record",
>         "name": "test.foo",
>         "fields": [
>           {
>             "name": "id",
>             "type": "long"
>           }
>         ]
>       }
>     }
>   ]
> } {code}
> Which will cause the Java tooling to fail with the 
> {{org.apache.avro.SchemaParseException: Can't redefine: test}} error above.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to