From 4070e05cd2a452638bbcaafdabcc3253592adf92 Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Wed, 10 Sep 2025 13:42:49 -0300 Subject: [PATCH] fix: math function parameter conversion --- core/numeric/mod.rs | 103 +++++++++++++++++++++++++------- core/types.rs | 26 +------- core/util.rs | 9 ++- core/vdbe/execute.rs | 51 ++++++---------- fuzz/Cargo.lock | 10 ++-- fuzz/fuzz_targets/expression.rs | 35 +++++++++++ 6 files changed, 150 insertions(+), 84 deletions(-) diff --git a/core/numeric/mod.rs b/core/numeric/mod.rs index 821c2f4b7..c0abc0e85 100644 --- a/core/numeric/mod.rs +++ b/core/numeric/mod.rs @@ -46,6 +46,44 @@ pub enum Numeric { } impl Numeric { + pub fn from_value_strict(value: &Value) -> Numeric { + match value { + Value::Null | Value::Blob(_) => Self::Null, + Value::Integer(v) => Self::Integer(*v), + Value::Float(v) => match NonNan::new(*v) { + Some(v) => Self::Float(v), + None => Self::Null, + }, + Value::Text(text) => { + let s = text.as_str(); + + match str_to_f64(s) { + None + | Some(StrToF64::FractionalPrefix(_)) + | Some(StrToF64::DecimalPrefix(_)) => Self::Null, + Some(StrToF64::Fractional(value)) => Self::Float(value), + Some(StrToF64::Decimal(real)) => { + let integer = str_to_i64(s).unwrap_or(0); + + if real == integer as f64 { + Self::Integer(integer) + } else { + Self::Float(real) + } + } + } + } + } + } + + pub fn try_into_f64(&self) -> Option { + match self { + Numeric::Null => None, + Numeric::Integer(v) => Some(*v as _), + Numeric::Float(v) => Some((*v).into()), + } + } + pub fn try_into_bool(&self) -> Option { match self { Numeric::Null => None, @@ -82,8 +120,10 @@ impl> From for Numeric { match str_to_f64(text) { None => Self::Integer(0), - Some(StrToF64::Fractional(value)) => Self::Float(value), - Some(StrToF64::Decimal(real)) => { + Some(StrToF64::Fractional(value) | StrToF64::FractionalPrefix(value)) => { + Self::Float(value) + } + Some(StrToF64::Decimal(real) | StrToF64::DecimalPrefix(real)) => { let integer = str_to_i64(text).unwrap_or(0); if real == integer as f64 { @@ -460,9 +500,23 @@ pub fn str_to_i64(input: impl AsRef) -> Option { ) } +#[derive(Debug)] pub enum StrToF64 { Fractional(NonNan), Decimal(NonNan), + FractionalPrefix(NonNan), + DecimalPrefix(NonNan), +} + +impl From for f64 { + fn from(value: StrToF64) -> Self { + match value { + StrToF64::Fractional(non_nan) => non_nan.into(), + StrToF64::Decimal(non_nan) => non_nan.into(), + StrToF64::FractionalPrefix(non_nan) => non_nan.into(), + StrToF64::DecimalPrefix(non_nan) => non_nan.into(), + } + } } pub fn str_to_f64(input: impl AsRef) -> Option { @@ -480,10 +534,6 @@ pub fn str_to_f64(input: impl AsRef) -> Option { let mut had_digits = false; let mut is_fractional = false; - if matches!(input.peek(), Some('e' | 'E')) { - return None; - } - let mut significant: u64 = 0; // Copy as many significant digits as we can @@ -509,12 +559,12 @@ pub fn str_to_f64(input: impl AsRef) -> Option { } if input.next_if(|ch| matches!(ch, '.')).is_some() { - if matches!(input.peek(), Some('e' | 'E')) { - return None; + if had_digits { + is_fractional = true; } - if had_digits || input.peek().is_some_and(char::is_ascii_digit) { - is_fractional = true + if input.peek().is_some_and(char::is_ascii_digit) { + is_fractional = true; } while let Some(digit) = input.peek().and_then(|ch| ch.to_digit(10)) { @@ -527,27 +577,32 @@ pub fn str_to_f64(input: impl AsRef) -> Option { } }; - if input.next_if(|ch| matches!(ch, 'e' | 'E')).is_some() { + let mut valid_exponent = true; + + if (had_digits || is_fractional) && input.next_if(|ch| matches!(ch, 'e' | 'E')).is_some() { let sign = match input.next_if(|ch| matches!(ch, '-' | '+')) { Some('-') => -1, _ => 1, }; if input.peek().is_some_and(char::is_ascii_digit) { - is_fractional = true - } + is_fractional = true; + let mut e = 0; - let e = input.map_while(|ch| ch.to_digit(10)).fold(0, |acc, digit| { - if acc < 1000 { - acc * 10 + digit as i32 - } else { - 1000 + while let Some(ch) = input.next_if(char::is_ascii_digit) { + e = (e * 10 + ch.to_digit(10).unwrap() as i32).min(1000); } - }); - exponent += sign * e; + exponent += sign * e; + } else { + valid_exponent = false; + } }; + if !(had_digits || is_fractional) { + return None; + } + while exponent.is_positive() && significant < MAX_EXACT / 10 { significant *= 10; exponent -= 1; @@ -591,6 +646,14 @@ pub fn str_to_f64(input: impl AsRef) -> Option { let result = NonNan::new(f64::from(result) * sign) .unwrap_or_else(|| NonNan::new(sign * f64::INFINITY).unwrap()); + if !valid_exponent || input.count() > 0 { + if is_fractional { + return Some(StrToF64::FractionalPrefix(result)); + } else { + return Some(StrToF64::DecimalPrefix(result)); + } + } + Some(if is_fractional { StrToF64::Fractional(result) } else { diff --git a/core/types.rs b/core/types.rs index 735c30b3e..ef66086fe 100644 --- a/core/types.rs +++ b/core/types.rs @@ -226,10 +226,7 @@ where { let s = String::deserialize(deserializer)?; match crate::numeric::str_to_f64(s) { - Some(result) => Ok(match result { - crate::numeric::StrToF64::Fractional(non_nan) => non_nan.into(), - crate::numeric::StrToF64::Decimal(non_nan) => non_nan.into(), - }), + Some(result) => Ok(result.into()), None => Err(serde::de::Error::custom("")), } } @@ -667,7 +664,7 @@ impl PartialEq for Value { match (self, other) { (Self::Integer(int_left), Self::Integer(int_right)) => int_left == int_right, (Self::Integer(int), Self::Float(float)) | (Self::Float(float), Self::Integer(int)) => { - int_float_cmp(*int, *float).is_eq() + sqlite_int_float_compare(*int, *float).is_eq() } (Self::Float(float_left), Self::Float(float_right)) => float_left == float_right, (Self::Integer(_) | Self::Float(_), Self::Text(_) | Self::Blob(_)) => false, @@ -682,25 +679,6 @@ impl PartialEq for Value { } } -fn int_float_cmp(int: i64, float: f64) -> std::cmp::Ordering { - if float.is_nan() { - return std::cmp::Ordering::Greater; - } - - if float < -9223372036854775808.0 { - return std::cmp::Ordering::Greater; - } - - if float >= 9223372036854775808.0 { - return std::cmp::Ordering::Less; - } - - match int.cmp(&(float as i64)) { - std::cmp::Ordering::Equal => (int as f64).total_cmp(&float), - cmp => cmp, - } -} - #[allow(clippy::non_canonical_partial_ord_impl)] impl PartialOrd for Value { fn partial_cmp(&self, other: &Self) -> Option { diff --git a/core/util.rs b/core/util.rs index b259a90f3..9026da710 100644 --- a/core/util.rs +++ b/core/util.rs @@ -1,5 +1,6 @@ #![allow(unused)] use crate::incremental::view::IncrementalView; +use crate::numeric::StrToF64; use crate::translate::expr::WalkControl; use crate::types::IOResult; use crate::{ @@ -1185,8 +1186,12 @@ pub fn parse_numeric_literal(text: &str) -> Result { return Ok(Value::Integer(int_value)); } - let float_value = text.parse::()?; - Ok(Value::Float(float_value)) + let Some(StrToF64::Fractional(float) | StrToF64::Decimal(float)) = + crate::numeric::str_to_f64(text) + else { + unreachable!(); + }; + Ok(Value::Float(float.into())) } pub fn parse_signed_number(expr: &Expr) -> Result { diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 83e164adc..43f4e9aa8 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -8217,27 +8217,19 @@ impl Value { } } - fn to_f64(&self) -> Option { - match self { - Value::Integer(i) => Some(*i as f64), - Value::Float(f) => Some(*f), - Value::Text(t) => t.as_str().parse::().ok(), - _ => None, - } - } - fn exec_math_unary(&self, function: &MathFunc) -> Value { + let v = Numeric::from_value_strict(self); + // In case of some functions and integer input, return the input as is - if let Value::Integer(_) = self { + if let Numeric::Integer(i) = v { if matches! { function, MathFunc::Ceil | MathFunc::Ceiling | MathFunc::Floor | MathFunc::Trunc } { - return self.clone(); + return Value::Integer(i); } } - let f = match self.to_f64() { - Some(f) => f, - None => return Value::Null, + let Some(f) = v.try_into_f64() else { + return Value::Null; }; let result = match function { @@ -8274,14 +8266,12 @@ impl Value { } fn exec_math_binary(&self, rhs: &Value, function: &MathFunc) -> Value { - let lhs = match self.to_f64() { - Some(f) => f, - None => return Value::Null, + let Some(lhs) = Numeric::from_value_strict(self).try_into_f64() else { + return Value::Null; }; - let rhs = match rhs.to_f64() { - Some(f) => f, - None => return Value::Null, + let Some(rhs) = Numeric::from_value_strict(rhs).try_into_f64() else { + return Value::Null; }; let result = match function { @@ -8299,16 +8289,13 @@ impl Value { } fn exec_math_log(&self, base: Option<&Value>) -> Value { - let f = match self.to_f64() { - Some(f) => f, - None => return Value::Null, + let Some(f) = Numeric::from_value_strict(self).try_into_f64() else { + return Value::Null; }; - let base = match base { - Some(base) => match base.to_f64() { - Some(f) => f, - None => return Value::Null, - }, + let base = match base.map(|value| Numeric::from_value_strict(value).try_into_f64()) { + Some(Some(f)) => f, + Some(None) => return Value::Null, None => 10.0, }; @@ -8389,11 +8376,9 @@ impl Value { pub fn exec_concat(&self, rhs: &Value) -> Value { if let (Value::Blob(lhs), Value::Blob(rhs)) = (self, rhs) { - return Value::build_text(String::from_utf8_lossy(dbg!(&[ - lhs.as_slice(), - rhs.as_slice() - ] - .concat()))); + return Value::build_text(String::from_utf8_lossy( + &[lhs.as_slice(), rhs.as_slice()].concat(), + )); } let Some(lhs) = self.cast_text() else { diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 3e8691c4b..12f1409ac 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -1182,7 +1182,7 @@ dependencies = [ [[package]] name = "turso_core" -version = "0.1.5-pre.3" +version = "0.1.5" dependencies = [ "aegis", "aes", @@ -1225,7 +1225,7 @@ dependencies = [ [[package]] name = "turso_ext" -version = "0.1.5-pre.3" +version = "0.1.5" dependencies = [ "chrono", "getrandom 0.3.1", @@ -1234,7 +1234,7 @@ dependencies = [ [[package]] name = "turso_macros" -version = "0.1.5-pre.3" +version = "0.1.5" dependencies = [ "proc-macro2", "quote", @@ -1243,7 +1243,7 @@ dependencies = [ [[package]] name = "turso_parser" -version = "0.1.5-pre.3" +version = "0.1.5" dependencies = [ "bitflags", "miette", @@ -1255,7 +1255,7 @@ dependencies = [ [[package]] name = "turso_sqlite3_parser" -version = "0.1.5-pre.3" +version = "0.1.5" dependencies = [ "bitflags", "cc", diff --git a/fuzz/fuzz_targets/expression.rs b/fuzz/fuzz_targets/expression.rs index a694f3b12..289d93d15 100644 --- a/fuzz/fuzz_targets/expression.rs +++ b/fuzz/fuzz_targets/expression.rs @@ -116,11 +116,26 @@ impl rusqlite::types::FromSql for Value { } } +str_enum! { + enum UnaryFunc { + Ceil => "ceil", + Floor => "floor", + } +} + +str_enum! { + enum BinaryFunc { + Power => "pow", + } +} + #[derive(Debug, Arbitrary)] enum Expr { Value(Value), Binary(Binary, Box, Box), Unary(Unary, Box), + UnaryFunc(UnaryFunc, Box), + BinaryFunc(BinaryFunc, Box, Box), } #[derive(Debug)] @@ -158,6 +173,26 @@ impl Expr { depth: lhs.depth.max(rhs.depth) + 1, } } + Expr::BinaryFunc(func, lhs, rhs) => { + let mut lhs = lhs.lower(); + let mut rhs = rhs.lower(); + Output { + query: format!("{func}({}, {})", lhs.query, rhs.query), + parameters: { + lhs.parameters.append(&mut rhs.parameters); + lhs.parameters + }, + depth: lhs.depth.max(rhs.depth) + 1, + } + } + Expr::UnaryFunc(func, expr) => { + let expr = expr.lower(); + Output { + query: format!("{func}({})", expr.query), + parameters: expr.parameters, + depth: expr.depth + 1, + } + } } } }