martin-g commented on code in PR #1837:
URL: https://github.com/apache/avro/pull/1837#discussion_r954547190
##########
lang/rust/avro/src/types.rs:
##########
@@ -1279,14 +1325,13 @@ mod tests {
.validate(&schema));
assert!(!Value::Map(
- vec![("c".to_string(), Value::Long(123_i64)),]
+ vec![("d".to_string(), Value::Long(123_i64)),]
.into_iter()
.collect()
)
.validate(&schema));
assert_logged(
- r#"Invalid value: Map({"c": Long(123)}) for schema: Record { name:
Name { name: "some_record", namespace: None }, aliases: None, doc: None,
fields: [RecordField { name: "a", doc: None, default: None, schema: Long,
order: Ascending, position: 0, custom_attributes: {} }, RecordField { name:
"b", doc: None, default: None, schema: String, order: Ascending, position: 1,
custom_attributes: {} }], lookup: {"a": 0, "b": 1}, attributes: {} }. Reason:
Field with name '"a"' is not a member of the map items
-Field with name '"b"' is not a member of the map items"#,
+ "Invalid value: Map({\"d\": Long(123)}) for schema: Record { name:
Name { name: \"some_record\", namespace: None }, aliases: None, doc: None,
fields: [RecordField { name: \"a\", doc: None, default: None, schema: Long,
order: Ascending, position: 0, custom_attributes: {} }, RecordField { name:
\"b\", doc: None, default: None, schema: String, order: Ascending, position: 1,
custom_attributes: {} }, RecordField { name: \"c\", doc: None, default:
Some(Null), schema: Union(UnionSchema { schemas: [Null, Int], variant_index:
{Null: 0, Int: 1} }), order: Ascending, position: 2, custom_attributes: {} }],
lookup: {\"a\": 0, \"b\": 1, \"c\": 2}, attributes: {} }. Reason: Field with
name '\"a\"' is not a member of the map items\nField with name '\"b\"' is not a
member of the map items",
Review Comment:
Again - please use `r##`
##########
lang/rust/avro/src/types.rs:
##########
@@ -1247,25 +1283,35 @@ mod tests {
("b".to_string(), Value::String("foo".to_string())),
]);
assert!(!value.validate(&schema));
- assert_logged("Invalid value: Record([(\"a\", Boolean(false)), (\"b\",
String(\"foo\"))]) for schema: Record { name: Name { name: \"some_record\",
namespace: None }, aliases: None, doc: None, fields: [RecordField { name:
\"a\", doc: None, default: None, schema: Long, order: Ascending, position: 0,
custom_attributes: {} }, RecordField { name: \"b\", doc: None, default: None,
schema: String, order: Ascending, position: 1, custom_attributes: {} }],
lookup: {\"a\": 0, \"b\": 1}, attributes: {} }. Reason: Unsupported
value-schema combination");
+ assert_logged("Invalid value: Record([(\"a\", Boolean(false)), (\"b\",
String(\"foo\"))]) for schema: Record { name: Name { name: \"some_record\",
namespace: None }, aliases: None, doc: None, fields: [RecordField { name:
\"a\", doc: None, default: None, schema: Long, order: Ascending, position: 0,
custom_attributes: {} }, RecordField { name: \"b\", doc: None, default: None,
schema: String, order: Ascending, position: 1, custom_attributes: {} },
RecordField { name: \"c\", doc: None, default: Some(Null), schema:
Union(UnionSchema { schemas: [Null, Int], variant_index: {Null: 0, Int: 1} }),
order: Ascending, position: 2, custom_attributes: {} }], lookup: {\"a\": 0,
\"b\": 1, \"c\": 2}, attributes: {} }. Reason: Unsupported value-schema
combination");
let value = Value::Record(vec![
("a".to_string(), Value::Long(42i64)),
("c".to_string(), Value::String("foo".to_string())),
]);
assert!(!value.validate(&schema));
assert_logged(
- "Invalid value: Record([(\"a\", Long(42)), (\"c\",
String(\"foo\"))]) for schema: Record { name: Name { name: \"some_record\",
namespace: None }, aliases: None, doc: None, fields: [RecordField { name:
\"a\", doc: None, default: None, schema: Long, order: Ascending, position: 0,
custom_attributes: {} }, RecordField { name: \"b\", doc: None, default: None,
schema: String, order: Ascending, position: 1, custom_attributes: {} }],
lookup: {\"a\": 0, \"b\": 1}, attributes: {} }. Reason: There is no schema
field for field 'c'"
+ "Invalid value: Record([(\"a\", Long(42)), (\"c\",
String(\"foo\"))]) for schema: Record { name: Name { name: \"some_record\",
namespace: None }, aliases: None, doc: None, fields: [RecordField { name:
\"a\", doc: None, default: None, schema: Long, order: Ascending, position: 0,
custom_attributes: {} }, RecordField { name: \"b\", doc: None, default: None,
schema: String, order: Ascending, position: 1, custom_attributes: {} },
RecordField { name: \"c\", doc: None, default: Some(Null), schema:
Union(UnionSchema { schemas: [Null, Int], variant_index: {Null: 0, Int: 1} }),
order: Ascending, position: 2, custom_attributes: {} }], lookup: {\"a\": 0,
\"b\": 1, \"c\": 2}, attributes: {} }. Reason: Could not find matching type in
union"
+ );
+
+ let value = Value::Record(vec![
+ ("a".to_string(), Value::Long(42i64)),
+ ("d".to_string(), Value::String("foo".to_string())),
+ ]);
+ assert!(!value.validate(&schema));
+ assert_logged(
+ "Invalid value: Record([(\"a\", Long(42)), (\"d\",
String(\"foo\"))]) for schema: Record { name: Name { name: \"some_record\",
namespace: None }, aliases: None, doc: None, fields: [RecordField { name:
\"a\", doc: None, default: None, schema: Long, order: Ascending, position: 0,
custom_attributes: {} }, RecordField { name: \"b\", doc: None, default: None,
schema: String, order: Ascending, position: 1, custom_attributes: {} },
RecordField { name: \"c\", doc: None, default: Some(Null), schema:
Union(UnionSchema { schemas: [Null, Int], variant_index: {Null: 0, Int: 1} }),
order: Ascending, position: 2, custom_attributes: {} }], lookup: {\"a\": 0,
\"b\": 1, \"c\": 2}, attributes: {} }. Reason: There is no schema field for
field 'd'"
);
let value = Value::Record(vec![
("a".to_string(), Value::Long(42i64)),
("b".to_string(), Value::String("foo".to_string())),
("c".to_string(), Value::Null),
+ ("d".to_string(), Value::Null),
]);
assert!(!value.validate(&schema));
assert_logged(
- r#"Invalid value: Record([("a", Long(42)), ("b", String("foo")),
("c", Null)]) for schema: Record { name: Name { name: "some_record", namespace:
None }, aliases: None, doc: None, fields: [RecordField { name: "a", doc: None,
default: None, schema: Long, order: Ascending, position: 0, custom_attributes:
{} }, RecordField { name: "b", doc: None, default: None, schema: String, order:
Ascending, position: 1, custom_attributes: {} }], lookup: {"a": 0, "b": 1},
attributes: {} }. Reason: The value's records length (3) is different than the
schema's (2)"#,
+ "Invalid value: Record([(\"a\", Long(42)), (\"b\",
String(\"foo\")), (\"c\", Null), (\"d\", Null)]) for schema: Record { name:
Name { name: \"some_record\", namespace: None }, aliases: None, doc: None,
fields: [RecordField { name: \"a\", doc: None, default: None, schema: Long,
order: Ascending, position: 0, custom_attributes: {} }, RecordField { name:
\"b\", doc: None, default: None, schema: String, order: Ascending, position: 1,
custom_attributes: {} }, RecordField { name: \"c\", doc: None, default:
Some(Null), schema: Union(UnionSchema { schemas: [Null, Int], variant_index:
{Null: 0, Int: 1} }), order: Ascending, position: 2, custom_attributes: {} }],
lookup: {\"a\": 0, \"b\": 1, \"c\": 2}, attributes: {} }. Reason: The value's
records length (4) is greater than the schema's (3 fields)",
Review Comment:
It is easier to read with `r##`, isn't it ?
##########
lang/rust/avro/src/types.rs:
##########
@@ -1465,6 +1510,56 @@ Field with name '"b"' is not a member of the map items"#,
assert!(value.resolve(&Schema::TimestampMicros).is_err());
}
+ #[test]
+ fn resolve_to_nullable_union() {
Review Comment:
please prefix the method name with `avro_1234` where `1234` is the number of
the JIRA ticker you create
##########
lang/rust/avro/src/schema.rs:
##########
@@ -634,6 +634,13 @@ impl RecordField {
}
custom_attributes
}
+
+ pub fn is_nullable(&self) -> bool {
Review Comment:
Please add rustdoc and new unit tests for this method.
##########
lang/rust/avro/src/types.rs:
##########
@@ -1465,6 +1510,56 @@ Field with name '"b"' is not a member of the map items"#,
assert!(value.resolve(&Schema::TimestampMicros).is_err());
}
+ #[test]
+ fn resolve_to_nullable_union() {
+ let value = Value::Record(vec![
Review Comment:
please move the `value` after `schema`, i.e. to line 1552
--
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]
