martin-g commented on code in PR #591:
URL: https://github.com/apache/avro-rs/pull/591#discussion_r3601038676
##########
avro/src/serde/ser_schema/mod.rs:
##########
@@ -2216,4 +2218,33 @@ mod tests {
Ok(())
}
+
+ #[test]
+ fn avro_rs_529_enum_with_union_schema() -> TestResult {
+ #[derive(Debug, Serialize)]
+ enum TestEnum {
+ A,
+ B,
+ C,
+ }
+
+ let schema = Schema::parse_str(
+ r#"
+ [
+ "int",
Review Comment:
If I change this to `"null"` then the assertion below fails. It still works
fine with any other primitive type (double, long, string) and "array".
Looking at the code above I'd expect it to fail with an empty record too.
##########
avro/src/serde/ser_schema/mod.rs:
##########
@@ -445,15 +445,17 @@ impl<'s, 'w, W: Write, S: Borrow<Schema>> Serializer for
SchemaAwareSerializer<'
Err(self.error("unit variant", format!(r#"Expected symbol
"{variant}" at index {variant_index} in enum"#)))
}
}
- Schema::Union(union) => match
self.get_resolved_union_variant(union, variant_index)? {
+ Schema::Union(union) if (variant_index as usize) <
union.variants().len() => match self.get_resolved_union_variant(union,
variant_index)? {
// Bare union
Schema::Null => zig_i32(variant_index as i32, &mut
*self.writer),
Schema::Record(record) if record.fields.is_empty() &&
record.name.name() == variant => {
// Union of records
zig_i32(variant_index as i32, &mut *self.writer)
}
- _ => Err(self.error("unit variant", format!("Expected
Schema::Null | Schema::Record(name: {variant}, fields: []) at index
{variant_index} in the union"))),
+ _ => UnionSerializer::new(self.writer, union,
self.config).serialize_unit_variant(name, variant_index, variant)
}
+ // This branch happens if the enum has more variants than the
union, which is valid if the target schema is a enum schema in the union
Review Comment:
Let's add a test case for this too.
##########
avro/src/serde/ser_schema/union.rs:
##########
@@ -356,11 +356,37 @@ impl<'s, 'w, W: Write, S: Borrow<Schema>> Serializer for
UnionSerializer<'s, 'w,
fn serialize_unit_variant(
self,
- _: &'static str,
- _: u32,
- _: &'static str,
+ name: &'static str,
+ variant_index: u32,
+ variant: &'static str,
) -> Result<Self::Ok, Self::Error> {
- Err(self.error("unit variant", "Nested unions are not supported"))
+ if let Some((index, Schema::Enum(enum_schema))) =
+ self.union.find_named_schema(name, self.config.names)?
+ && enum_schema
+ .symbols
+ .get(variant_index as usize)
+ .map(String::as_str)
+ == Some(variant)
+ {
+ // Fast path for if the name and variant match (no #[serde(skip)]
or other trickery)
+ let mut bytes_written = zig_i32(index as i32, &mut *self.writer)?;
+ bytes_written += zig_i32(variant_index as i32, &mut *self.writer)?;
+ Ok(bytes_written)
+ } else {
+ for (index, schema) in self.union.variants().iter().enumerate() {
+ if let Schema::Enum(enum_schema) = schema
+ && let Some(symbol) =
enum_schema.symbols.iter().position(|s| s == variant)
Review Comment:
This may fail if there are two differently named enum schemas having symbol
with the same name.
##########
avro/src/serde/ser_schema/union.rs:
##########
@@ -356,11 +356,37 @@ impl<'s, 'w, W: Write, S: Borrow<Schema>> Serializer for
UnionSerializer<'s, 'w,
fn serialize_unit_variant(
self,
- _: &'static str,
- _: u32,
- _: &'static str,
+ name: &'static str,
+ variant_index: u32,
+ variant: &'static str,
) -> Result<Self::Ok, Self::Error> {
- Err(self.error("unit variant", "Nested unions are not supported"))
+ if let Some((index, Schema::Enum(enum_schema))) =
+ self.union.find_named_schema(name, self.config.names)?
+ && enum_schema
+ .symbols
+ .get(variant_index as usize)
+ .map(String::as_str)
+ == Some(variant)
+ {
+ // Fast path for if the name and variant match (no #[serde(skip)]
or other trickery)
+ let mut bytes_written = zig_i32(index as i32, &mut *self.writer)?;
+ bytes_written += zig_i32(variant_index as i32, &mut *self.writer)?;
+ Ok(bytes_written)
+ } else {
+ for (index, schema) in self.union.variants().iter().enumerate() {
+ if let Schema::Enum(enum_schema) = schema
Review Comment:
```suggestion
let resolved_schema = match schema {
Schema::Ref { name } =>
self.config.get_schema(name).unwrap_or(schema),
_ => schema,
};
if let Schema::Enum(enum_schema) = resolved_schema
```
Add support for schema references.
--
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]
