martin-g commented on code in PR #3051:
URL: https://github.com/apache/avro/pull/3051#discussion_r1705316426


##########
lang/rust/avro/src/types.rs:
##########
@@ -918,10 +922,25 @@ impl Value {
             Value::Long(n) => Ok(Value::Double(n as f64)),
             Value::Float(x) => Ok(Value::Double(f64::from(x))),
             Value::Double(x) => Ok(Value::Double(x)),
+            Value::String(x) => match Self::parse_special_float(&x) {
+                Some(f) => Ok(Value::Double(f.into())),
+                None => Err(Error::GetDouble(ValueKind::String)),
+            },
             other => Err(Error::GetDouble(other.into())),
         }
     }
 
+    /// IEEE 754 NaN and infinities are not valid JSON numbers.
+    /// So they are represented in JSON as strings.
+    fn parse_special_float(s: &str) -> Option<f32> {
+        match s.trim().to_ascii_lowercase().as_str() {
+            "nan" | "+nan" | "-nan" => Some(f32::NAN),

Review Comment:
   [Java](https://github.com/apache/avro/pull/3066) and 
[C++](https://github.com/apache/avro/blob/release-1.12.0/lang/c%2B%2B/impl/parsing/JsonCodec.cc#L536)
 use case-sensitive values.
   I suggest to compare against `NaN`, `Inf`, `Infinity`, etc., i.e. let's 
remove `to_asciilowercase()` 



##########
lang/rust/avro/src/types.rs:
##########
@@ -918,10 +922,25 @@ impl Value {
             Value::Long(n) => Ok(Value::Double(n as f64)),
             Value::Float(x) => Ok(Value::Double(f64::from(x))),
             Value::Double(x) => Ok(Value::Double(x)),
+            Value::String(x) => match Self::parse_special_float(&x) {
+                Some(f) => Ok(Value::Double(f.into())),
+                None => Err(Error::GetDouble(ValueKind::String)),
+            },
             other => Err(Error::GetDouble(other.into())),
         }
     }
 
+    /// IEEE 754 NaN and infinities are not valid JSON numbers.
+    /// So they are represented in JSON as strings.
+    fn parse_special_float(s: &str) -> Option<f32> {
+        match s.trim().to_ascii_lowercase().as_str() {
+            "nan" | "+nan" | "-nan" => Some(f32::NAN),

Review Comment:
   [Java](https://github.com/apache/avro/pull/3066) and 
[C++](https://github.com/apache/avro/blob/release-1.12.0/lang/c%2B%2B/impl/parsing/JsonCodec.cc#L536)
 use case-sensitive values.
   I suggest to compare against `NaN`, `Inf`, `Infinity`, etc., i.e. let's 
remove `to_ascii_lowercase()` 



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

Reply via email to