chore(core): Value unification part 1 - NotNan floats (#11291)

Author: fuchsnj

Cargo.lock

@@ -47,6 +47,7 @@ chrono = { version = "0.4", optional = true }
 derivative = "2.1.3"
 metrics = { version = "0.17.0", default-features = false, features = ["std"] }
 nom = { version = "7", optional = true }
+ordered-float = { version = "2.10.0", default-features = false }
 serde_json = { version = "1.0.78", default-features = false, features = ["std", "raw_value"] }
 serde = { version = "1.0.136", optional = true, features = ["derive"] }
 smallvec = { version = "1", default-features = false }

lib/vector-common/Cargo.toml

@@ -6,6 +6,7 @@ use std::{
 
 use bytes::Bytes;
 use chrono::{DateTime, ParseError as ChronoParseError, TimeZone as _, Utc};
+use ordered_float::NotNan;
 use snafu::{ResultExt, Snafu};
 
 use super::datetime::{datetime_to_utc, TimeZone};
@@ -40,6 +41,8 @@ pub enum Error {
     BoolParse { s: String },
     #[snafu(display("Invalid integer {:?}: {}", s, source))]
     IntParse { s: String, source: ParseIntError },
+    #[snafu(display("NaN number not supported {:?}", s))]
+    NanFloat { s: String },
     #[snafu(display("Invalid floating point number {:?}: {}", s, source))]
     FloatParse { s: String, source: ParseFloatError },
     #[snafu(
@@ -140,7 +143,7 @@ impl Conversion {
     /// Returns errors from the underlying conversion functions. See `enum Error`.
     pub fn convert<T>(&self, bytes: Bytes) -> Result<T, Error>
     where
-        T: From<Bytes> + From<i64> + From<f64> + From<bool> + From<DateTime<Utc>>,
+        T: From<Bytes> + From<i64> + From<NotNan<f64>> + From<bool> + From<DateTime<Utc>>,
     {
         Ok(match self {
             Self::Bytes => bytes.into(),
@@ -152,9 +155,11 @@ impl Conversion {
             }
             Self::Float => {
                 let s = String::from_utf8_lossy(&bytes);
-                s.parse::<f64>()
-                    .with_context(|_| FloatParseSnafu { s })?
-                    .into()
+                let parsed = s
+                    .parse::<f64>()
+                    .with_context(|_| FloatParseSnafu { s: s.clone() })?;
+                let f = NotNan::new(parsed).map_err(|_| Error::NanFloat { s: s.to_string() })?;
+                f.into()
             }
             Self::Boolean => parse_bool(&String::from_utf8_lossy(&bytes))?.into(),
             Self::Timestamp(tz) => parse_timestamp(*tz, &String::from_utf8_lossy(&bytes))?.into(),

lib/vector-common/src/conversion.rs

@@ -1,5 +1,6 @@
 use bytes::Bytes;
 use chrono::{DateTime, Utc};
+use ordered_float::NotNan;
 
 use crate::conversion::parse_bool;
 
@@ -33,6 +34,12 @@ impl From<f64> for StubValue {
     }
 }
 
+impl From<NotNan<f64>> for StubValue {
+    fn from(v: NotNan<f64>) -> Self {
+        StubValue::Float(v.into_inner())
+    }
+}
+
 impl From<i64> for StubValue {
     fn from(v: i64) -> Self {
         StubValue::Integer(v)

lib/vector-common/src/conversion/tests/mod.rs

@@ -1,6 +1,7 @@
 use bytes::Bytes;
 use chrono::{DateTime, NaiveDateTime, TimeZone as _, Utc};
 use chrono_tz::{Australia, Tz};
+use ordered_float::NotNan;
 
 use crate::{
     conversion::{parse_timestamp, tests::StubValue, Conversion, Error},
@@ -56,7 +57,7 @@ fn dateref() -> DateTime<Utc> {
 
 fn convert<T>(fmt: &str, value: &'static str) -> Result<T, Error>
 where
-    T: From<Bytes> + From<i64> + From<f64> + From<bool> + From<DateTime<Utc>>,
+    T: From<Bytes> + From<i64> + From<NotNan<f64>> + From<bool> + From<DateTime<Utc>>,
 {
     std::env::set_var("TZ", TIMEZONE_NAME);
     Conversion::parse(fmt, TimeZone::Local)

lib/vector-common/src/conversion/tests/unix.rs

@@ -58,7 +58,7 @@ fn value_eq(this: &Value, other: &Value) -> bool {
         (Value::Timestamp(this), Value::Timestamp(other)) => this.eq(other),
         (Value::Null, Value::Null) => true,
         // Non-trivial.
-        (Value::Float(this), Value::Float(other)) => f64_eq(*this, *other),
+        (Value::Float(this), Value::Float(other)) => f64_eq(this.into_inner(), other.into_inner()),
         (Value::Array(this), Value::Array(other)) => array_eq(this, other),
         (Value::Map(this), Value::Map(other)) => map_eq(this, other),
         // Type mismatch.
@@ -125,7 +125,7 @@ fn hash_value<H: Hasher>(hasher: &mut H, value: &Value) {
         Value::Integer(val) => val.hash(hasher),
         Value::Timestamp(val) => val.hash(hasher),
         // Non-trivial.
-        Value::Float(val) => hash_f64(hasher, *val),
+        Value::Float(val) => hash_f64(hasher, val.into_inner()),
         Value::Array(val) => hash_array(hasher, val),
         Value::Map(val) => hash_map(hasher, val),
         Value::Null => hash_null(hasher),

lib/vector-core/src/event/discriminant.rs

@@ -13,6 +13,7 @@ use std::{
 };
 
 use indexmap::map::IndexMap;
+use ordered_float::NotNan;
 use pest::{iterators::Pair, Parser};
 pub use segment::Segment;
 use serde::{
@@ -118,7 +119,10 @@ impl Lookup {
         match value {
             TomlValue::String(s) => discoveries.insert(lookup, Value::from(s)),
             TomlValue::Integer(i) => discoveries.insert(lookup, Value::from(i)),
-            TomlValue::Float(f) => discoveries.insert(lookup, Value::from(f)),
+            TomlValue::Float(f) => {
+                let value = NotNan::new(f).map_err(|_| "NaN value not supported")?;
+                discoveries.insert(lookup, Value::Float(value))
+            }
             TomlValue::Boolean(b) => discoveries.insert(lookup, Value::from(b)),
             TomlValue::Datetime(dt) => {
                 let dt = dt.to_string();

lib/vector-core/src/event/legacy_lookup/mod.rs

@@ -1,4 +1,5 @@
 use mlua::prelude::*;
+use ordered_float::NotNan;
 
 use super::util::{table_is_timestamp, table_to_timestamp, timestamp_to_table};
 use crate::event::Value;
@@ -9,7 +10,7 @@ impl<'a> ToLua<'a> for Value {
         match self {
             Value::Bytes(b) => lua.create_string(b.as_ref()).map(LuaValue::String),
             Value::Integer(i) => Ok(LuaValue::Integer(i)),
-            Value::Float(f) => Ok(LuaValue::Number(f)),
+            Value::Float(f) => Ok(LuaValue::Number(f.into_inner())),
             Value::Boolean(b) => Ok(LuaValue::Boolean(b)),
             Value::Timestamp(t) => timestamp_to_table(lua, t).map(LuaValue::Table),
             Value::Map(m) => lua.create_table_from(m.into_iter()).map(LuaValue::Table),
@@ -24,7 +25,14 @@ impl<'a> FromLua<'a> for Value {
         match value {
             LuaValue::String(s) => Ok(Value::Bytes(Vec::from(s.as_bytes()).into())),
             LuaValue::Integer(i) => Ok(Value::Integer(i)),
-            LuaValue::Number(f) => Ok(Value::Float(f)),
+            LuaValue::Number(f) => {
+                let f = NotNan::new(f).map_err(|_| mlua::Error::FromLuaConversionError {
+                    from: value.type_name(),
+                    to: "Value",
+                    message: Some("NaN not supported".to_string()),
+                })?;
+                Ok(Value::Float(f))
+            }
             LuaValue::Boolean(b) => Ok(Value::Boolean(b)),
             LuaValue::Table(t) => {
                 if t.len()? > 0 {
@@ -58,13 +66,13 @@ mod test {
                 Value::Bytes("\u{237a}\u{3b2}\u{3b3}".into()),
             ),
             ("123", Value::Integer(123)),
-            ("4.333", Value::Float(4.333)),
+            ("4.333", Value::from(4.333)),
             ("true", Value::Boolean(true)),
             (
                 "{ x = 1, y = '2', nested = { other = 5.678 } }",
                 Value::Map(
                     vec![
-                        ("x".into(), 1.into()),
+                        ("x".into(), 1_i64.into()),
                         ("y".into(), "2".into()),
                         (
                             "nested".into(),
@@ -77,7 +85,7 @@ mod test {
             ),
             (
                 "{1, '2', 0.57721566}",
-                Value::Array(vec![1.into(), "2".into(), 0.577_215_66.into()]),
+                Value::Array(vec![1_i64.into(), "2".into(), 0.577_215_66.into()]),
             ),
             (
                 "os.date('!*t', 1584297428)",
@@ -122,7 +130,7 @@ mod test {
                 "#,
             ),
             (
-                Value::Float(4.333),
+                Value::from(4.333),
                 r#"
                 function (value)
                     return value == 4.333
@@ -140,7 +148,7 @@ mod test {
             (
                 Value::Map(
                     vec![
-                        ("x".into(), 1.into()),
+                        ("x".into(), 1_i64.into()),
                         ("y".into(), "2".into()),
                         (
                             "nested".into(),
@@ -159,7 +167,7 @@ mod test {
                 "#,
             ),
             (
-                Value::Array(vec![1.into(), "2".into(), 0.577_215_66.into()]),
+                Value::Array(vec![1_i64.into(), "2".into(), 0.577_215_66.into()]),
                 r#"
                 function (value)
                     return value[1] == 1 and

lib/vector-core/src/event/lua/value.rs

@@ -1,4 +1,5 @@
 use chrono::TimeZone;
+use ordered_float::NotNan;
 
 use crate::{
     event::{self, BTreeMap, WithMetadata},
@@ -325,7 +326,7 @@ fn decode_value(input: Value) -> Option<event::Value> {
             chrono::Utc.timestamp(ts.seconds, ts.nanos as u32),
         )),
         Some(value::Kind::Integer(value)) => Some(event::Value::Integer(value)),
-        Some(value::Kind::Float(value)) => Some(event::Value::Float(value)),
+        Some(value::Kind::Float(value)) => Some(event::Value::Float(NotNan::new(value).unwrap())),
         Some(value::Kind::Boolean(value)) => Some(event::Value::Boolean(value)),
         Some(value::Kind::Map(map)) => decode_map(map.fields),
         Some(value::Kind::Array(array)) => decode_array(array.items),
@@ -370,7 +371,7 @@ fn encode_value(value: event::Value) -> Value {
                 nanos: ts.timestamp_subsec_nanos() as i32,
             })),
             event::Value::Integer(value) => Some(value::Kind::Integer(value)),
-            event::Value::Float(value) => Some(value::Kind::Float(value)),
+            event::Value::Float(value) => Some(value::Kind::Float(value.into_inner())),
             event::Value::Boolean(value) => Some(value::Kind::Boolean(value)),
             event::Value::Map(fields) => Some(value::Kind::Map(encode_map(fields))),
             event::Value::Array(items) => Some(value::Kind::Array(encode_array(items))),

lib/vector-core/src/event/proto.rs

@@ -562,7 +562,13 @@ impl Arbitrary for Value {
                 Value::Bytes(Bytes::from(bytes))
             }
             1 => Value::Integer(i64::arbitrary(g)),
-            2 => Value::Float(f64::arbitrary(g) % MAX_F64_SIZE),
+            2 => {
+                let mut f = f64::arbitrary(g) % MAX_F64_SIZE;
+                if f.is_nan() {
+                    f = 0.0;
+                }
+                Value::from(f)
+            }
             3 => Value::Boolean(bool::arbitrary(g)),
             4 => Value::Timestamp(datetime(g)),
             5 => {