martin-g commented on code in PR #533:
URL: https://github.com/apache/avro-rs/pull/533#discussion_r3108504322
##########
avro/src/types.rs:
##########
@@ -846,6 +846,20 @@ impl Value {
Ok(Value::Decimal(Decimal::from(bytes)))
}
}
+ // JSON string defaults per spec §Records: codepoints 0-255
+ // map to byte values 0-255. No precision check — defaults
+ // need only be valid `bytes`, not fit the declared precision.
+ Value::String(s) => {
+ let mut bytes = Vec::with_capacity(s.len());
Review Comment:
```suggestion
let mut bytes = Vec::with_capacity(s.chars().count());
```
##########
avro/src/types.rs:
##########
@@ -1776,6 +1790,69 @@ Field with name '"b"' is not a member of the map items"#,
Ok(())
}
+ #[test]
+ fn resolve_decimal_from_string_default() -> TestResult {
+ let value = Value::String("\u{0000}".to_string());
+ let resolved = value.resolve(&Schema::Decimal(DecimalSchema {
+ precision: 10,
+ scale: 4,
+ inner: InnerDecimalSchema::Bytes,
+ }))?;
+ assert_eq!(resolved, Value::Decimal(Decimal::from(vec![0u8])));
+
+ let mut all_bytes_str = String::new();
+ for b in 0u8..=255u8 {
+ all_bytes_str.push(char::from_u32(b as u32).unwrap());
+ }
+ let resolved =
Value::String(all_bytes_str).resolve(&Schema::Decimal(DecimalSchema {
+ precision: 10,
+ scale: 0,
+ inner: InnerDecimalSchema::Bytes,
+ }))?;
+ assert_eq!(
+ resolved,
+ Value::Decimal(Decimal::from((0u8..=255u8).collect::<Vec<_>>()))
+ );
+
+ let value = Value::String("\u{0100}".to_string());
+ assert!(
+ value
+ .resolve(&Schema::Decimal(DecimalSchema {
+ precision: 10,
+ scale: 4,
+ inner: InnerDecimalSchema::Bytes,
+ }))
+ .is_err()
+ );
+
+ Ok(())
+ }
+
+ #[test]
+ fn parse_schema_with_nullable_decimal_string_default() -> TestResult {
Review Comment:
```suggestion
fn avro_rs_533_parse_schema_with_nullable_decimal_string_default() ->
TestResult {
```
##########
avro/src/types.rs:
##########
@@ -846,6 +846,20 @@ impl Value {
Ok(Value::Decimal(Decimal::from(bytes)))
}
}
+ // JSON string defaults per spec §Records: codepoints 0-255
Review Comment:
The comment talks about the field's `default` but the match arm would be
used for any String->Decimal resolve.
Most probably we will need to update the error message for
https://github.com/apache/avro-rs/blob/5bb5c4becd5c2040837271369f18faf9dcf1d9e1/avro/src/error.rs#L191
##########
avro/src/types.rs:
##########
@@ -1776,6 +1790,69 @@ Field with name '"b"' is not a member of the map items"#,
Ok(())
}
+ #[test]
+ fn resolve_decimal_from_string_default() -> TestResult {
Review Comment:
```suggestion
fn avro_rs_533_resolve_decimal_from_string_default() -> TestResult {
```
##########
avro/src/types.rs:
##########
@@ -846,6 +846,20 @@ impl Value {
Ok(Value::Decimal(Decimal::from(bytes)))
}
}
+ // JSON string defaults per spec §Records: codepoints 0-255
+ // map to byte values 0-255. No precision check — defaults
+ // need only be valid `bytes`, not fit the declared precision.
+ Value::String(s) => {
+ let mut bytes = Vec::with_capacity(s.len());
+ for c in s.chars() {
+ let cp = c as u32;
+ if cp > 0xFF {
+ return
Err(Details::ResolveDecimal(Value::String(s)).into());
+ }
+ bytes.push(cp as u8);
+ }
+ Ok(Value::Decimal(Decimal::from(bytes)))
+ }
Review Comment:
nit: Let's use the functional style:
```rust
Value::String(s) => {
let bytes = s.chars()
.map(|c| {
let cp = c as u32;
if cp > 0xFF {
Err(Details::ResolveDecimal(Value::String(s.clone())).into())
} else {
Ok(cp as u8)
}
})
.collect::<Result<Vec<u8>, Error>>()?;
Ok(Value::Decimal(Decimal::from(bytes)))
}
```
--
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]
