Skip to content

Commit 8959c7e

Browse files
pedrocarlopedrocarlo
authored
Merge 'make Numeric the source of truth for number comparisons' from Pedro Muniz
I caught some panics with differential fuzzer when executing `compare_immutable` that originate from our `partial_cmp` impl for `ValueRef` not using the comparison logic from `Numeric` when handling NaN. The main problem really originates from the fact that we use arithmetic operations with `Value` and when NaN appears we need to properly return `Null`. I made the bold move to try and remove `Integer` and `Float` variants from `Value` and `ValueRef`, and substituted it with `Numeric` variant. This forces our code to always handle possible `Null` or `None` that originate from `Numeric` operations, avoiding these panics. I also fixed the `PartialOrd` and `PartialEq` impls to delegate numeric comparisons to `Numeric` Fix panics and avoid edge cases where we use some bespoke method to compare Numbers incorrectly Initial mass migration, and then adjustments in the last 2 commits were made by me Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com> Closes #5099
2 parents 53e2d1b + 0175f1c commit 8959c7e

File tree

93 files changed

+3761
-3430
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

93 files changed

+3761
-3430
lines changed

‎bindings/dotnet/rs_src/lib.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ pub fn to_value(value: TursoValue) -> Value {
7171
match value.value_type {
7272
ValueType::Empty => Value::Null,
7373
ValueType::Null => Value::Null,
74-
ValueType::Integer => Value::Integer(unsafe { value.value.int_val }),
75-
ValueType::Float => Value::Float(unsafe { value.value.real_val }),
74+
ValueType::Integer => Value::from_i64(unsafe { value.value.int_val }),
75+
ValueType::Float => Value::from_f64(unsafe { value.value.real_val }),
7676
ValueType::Blob => Value::Blob(to_vec(unsafe { value.value.blob })),
7777
ValueType::Text => {
7878
let slice =
@@ -418,14 +418,14 @@ pub unsafe extern "C" fn db_statement_get_value(
418418
value_type: ValueType::Null,
419419
value: TursoValueUnion { int_val: 0 },
420420
},
421-
Value::Integer(int_val) => TursoValue {
421+
Value::Numeric(turso_core::Numeric::Integer(int_val)) => TursoValue {
422422
value_type: ValueType::Integer,
423423
value: TursoValueUnion { int_val: *int_val },
424424
},
425-
Value::Float(float_value) => TursoValue {
425+
Value::Numeric(turso_core::Numeric::Float(float_value)) => TursoValue {
426426
value_type: ValueType::Float,
427427
value: TursoValueUnion {
428-
real_val: *float_value,
428+
real_val: f64::from(*float_value),
429429
},
430430
},
431431
Value::Text(text) => {

‎bindings/java/rs_src/turso_statement.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,11 @@ fn row_to_obj_array<'local>(
104104
for (i, value) in row.get_values().enumerate() {
105105
let obj = match value {
106106
turso_core::Value::Null => JObject::null(),
107-
turso_core::Value::Integer(i) => {
107+
turso_core::Value::Numeric(turso_core::Numeric::Integer(i)) => {
108108
env.new_object("java/lang/Long", "(J)V", &[JValue::Long(*i)])?
109109
}
110-
turso_core::Value::Float(f) => {
111-
env.new_object("java/lang/Double", "(D)V", &[JValue::Double(*f)])?
110+
turso_core::Value::Numeric(turso_core::Numeric::Float(f)) => {
111+
env.new_object("java/lang/Double", "(D)V", &[JValue::Double(f64::from(*f))])?
112112
}
113113
turso_core::Value::Text(s) => env.new_string(s.as_str())?.into(),
114114
turso_core::Value::Blob(b) => env.byte_array_from_slice(b.as_slice())?.into(),
@@ -181,7 +181,7 @@ pub extern "system" fn Java_tech_turso_core_TursoStatement_bindLong<'local>(
181181

182182
stmt.stmt.bind_at(
183183
NonZero::new(position as usize).unwrap(),
184-
Value::Integer(value),
184+
Value::from_i64(value),
185185
);
186186
SQLITE_OK
187187
}
@@ -204,7 +204,7 @@ pub extern "system" fn Java_tech_turso_core_TursoStatement_bindDouble<'local>(
204204

205205
stmt.stmt.bind_at(
206206
NonZero::new(position as usize).unwrap(),
207-
Value::Float(value),
207+
Value::from_f64(value),
208208
);
209209
SQLITE_OK
210210
}

‎bindings/javascript/src/lib.rs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -607,25 +607,25 @@ impl Statement {
607607
ValueType::Number => {
608608
let n: f64 = unsafe { value.cast()? };
609609
if n.fract() == 0.0 && n >= i64::MIN as f64 && n <= i64::MAX as f64 {
610-
turso_core::Value::Integer(n as i64)
610+
turso_core::Value::from_i64(n as i64)
611611
} else {
612-
turso_core::Value::Float(n)
612+
turso_core::Value::from_f64(n)
613613
}
614614
}
615615
ValueType::BigInt => {
616616
let bigint_str = value.coerce_to_string()?.into_utf8()?.as_str()?.to_owned();
617617
let bigint_value = bigint_str
618618
.parse::<i64>()
619619
.map_err(|e| to_error(Status::NumberExpected, "failed to parse BigInt", e))?;
620-
turso_core::Value::Integer(bigint_value)
620+
turso_core::Value::from_i64(bigint_value)
621621
}
622622
ValueType::String => {
623623
let s = value.coerce_to_string()?.into_utf8()?;
624624
turso_core::Value::Text(s.as_str()?.to_owned().into())
625625
}
626626
ValueType::Boolean => {
627627
let b: bool = unsafe { value.cast()? };
628-
turso_core::Value::Integer(if b { 1 } else { 0 })
628+
turso_core::Value::from_i64(if b { 1 } else { 0 })
629629
}
630630
ValueType::Object => {
631631
let obj = value.coerce_to_object()?;
@@ -808,15 +808,17 @@ fn to_js_value<'a>(
808808
) -> napi::Result<Unknown<'a>> {
809809
match value {
810810
turso_core::Value::Null => ToNapiValue::into_unknown(Null, env),
811-
turso_core::Value::Integer(i) => {
811+
turso_core::Value::Numeric(turso_core::Numeric::Integer(i)) => {
812812
if safe_integers {
813813
let bigint = BigInt::from(*i);
814814
ToNapiValue::into_unknown(bigint, env)
815815
} else {
816816
ToNapiValue::into_unknown(*i as f64, env)
817817
}
818818
}
819-
turso_core::Value::Float(f) => ToNapiValue::into_unknown(*f, env),
819+
turso_core::Value::Numeric(turso_core::Numeric::Float(f)) => {
820+
ToNapiValue::into_unknown(f64::from(*f), env)
821+
}
820822
turso_core::Value::Text(s) => ToNapiValue::into_unknown(s.as_str(), env),
821823
turso_core::Value::Blob(b) => {
822824
#[cfg(not(feature = "browser"))]

‎bindings/javascript/sync/src/lib.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,21 @@ fn core_change_type_to_js(value: DatabaseChangeType) -> DatabaseChangeTypeJs {
5959
fn js_value_to_core(value: Either5<Null, i64, f64, String, Vec<u8>>) -> turso_core::Value {
6060
match value {
6161
Either5::A(_) => turso_core::Value::Null,
62-
Either5::B(value) => turso_core::Value::Integer(value),
63-
Either5::C(value) => turso_core::Value::Float(value),
62+
Either5::B(value) => turso_core::Value::from_i64(value),
63+
Either5::C(value) => turso_core::Value::from_f64(value),
6464
Either5::D(value) => turso_core::Value::Text(turso_core::types::Text::new(value)),
6565
Either5::E(value) => turso_core::Value::Blob(value),
6666
}
6767
}
6868
fn core_value_to_js(value: turso_core::Value) -> Either5<Null, i64, f64, String, Vec<u8>> {
6969
match value {
7070
turso_core::Value::Null => Either5::<Null, i64, f64, String, Vec<u8>>::A(Null),
71-
turso_core::Value::Integer(value) => Either5::<Null, i64, f64, String, Vec<u8>>::B(value),
72-
turso_core::Value::Float(value) => Either5::<Null, i64, f64, String, Vec<u8>>::C(value),
71+
turso_core::Value::Numeric(turso_core::Numeric::Integer(value)) => {
72+
Either5::<Null, i64, f64, String, Vec<u8>>::B(value)
73+
}
74+
turso_core::Value::Numeric(turso_core::Numeric::Float(value)) => {
75+
Either5::<Null, i64, f64, String, Vec<u8>>::C(f64::from(value))
76+
}
7377
turso_core::Value::Text(value) => {
7478
Either5::<Null, i64, f64, String, Vec<u8>>::D(value.as_str().to_string())
7579
}

‎bindings/python/src/turso.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use pyo3::{
33
types::{PyBytes, PyTuple},
44
};
55
use std::sync::Arc;
6-
use turso_sdk_kit::rsapi::{self, EncryptionOpts, TursoError, TursoStatusCode, Value, ValueRef};
6+
use turso_sdk_kit::rsapi::{
7+
self, EncryptionOpts, Numeric, TursoError, TursoStatusCode, Value, ValueRef,
8+
};
79

810
use pyo3::create_exception;
911
use pyo3::exceptions::PyException;
@@ -360,8 +362,8 @@ impl PyTursoStatement {
360362
fn db_value_to_py(py: Python, value: rsapi::ValueRef) -> PyResult<Py<PyAny>> {
361363
match value {
362364
ValueRef::Null => Ok(py.None()),
363-
ValueRef::Integer(i) => Ok(i.into_pyobject(py)?.into()),
364-
ValueRef::Float(f) => Ok(f.into_pyobject(py)?.into()),
365+
ValueRef::Numeric(Numeric::Integer(i)) => Ok(i.into_pyobject(py)?.into()),
366+
ValueRef::Numeric(Numeric::Float(f)) => Ok(f64::from(f).into_pyobject(py)?.into()),
365367
ValueRef::Text(s) => Ok(s.as_str().into_pyobject(py)?.into()),
366368
ValueRef::Blob(b) => Ok(PyBytes::new(py, b).into()),
367369
}
@@ -372,9 +374,9 @@ fn py_to_db_value(obj: Bound<PyAny>) -> PyResult<Value> {
372374
if obj.is_none() {
373375
Ok(Value::Null)
374376
} else if let Ok(integer) = obj.extract::<i64>() {
375-
Ok(Value::Integer(integer))
377+
Ok(Value::from_i64(integer))
376378
} else if let Ok(float) = obj.extract::<f64>() {
377-
Ok(Value::Float(float))
379+
Ok(Value::from_f64(float))
378380
} else if let Ok(string) = obj.extract::<String>() {
379381
Ok(Value::Text(string.into()))
380382
} else if let Ok(bytes) = obj.cast::<PyBytes>() {

‎bindings/rust/src/rows.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,13 @@ impl Row {
5555
))
5656
})?;
5757
match val {
58-
turso_sdk_kit::rsapi::Value::Integer(i) => Ok(Value::Integer(*i)),
58+
turso_sdk_kit::rsapi::Value::Numeric(turso_sdk_kit::rsapi::Numeric::Integer(i)) => {
59+
Ok(Value::Integer(*i))
60+
}
61+
turso_sdk_kit::rsapi::Value::Numeric(turso_sdk_kit::rsapi::Numeric::Float(f)) => {
62+
Ok(Value::Real(f64::from(*f)))
63+
}
5964
turso_sdk_kit::rsapi::Value::Null => Ok(Value::Null),
60-
turso_sdk_kit::rsapi::Value::Float(f) => Ok(Value::Real(*f)),
6165
turso_sdk_kit::rsapi::Value::Text(text) => Ok(Value::Text(text.to_string())),
6266
turso_sdk_kit::rsapi::Value::Blob(items) => Ok(Value::Blob(items.to_vec())),
6367
}

‎bindings/rust/src/value.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,12 @@ impl From<turso_sdk_kit::rsapi::Value> for Value {
114114
fn from(val: turso_sdk_kit::rsapi::Value) -> Self {
115115
match val {
116116
turso_sdk_kit::rsapi::Value::Null => Value::Null,
117-
turso_sdk_kit::rsapi::Value::Integer(n) => Value::Integer(n),
118-
turso_sdk_kit::rsapi::Value::Float(n) => Value::Real(n),
117+
turso_sdk_kit::rsapi::Value::Numeric(turso_sdk_kit::rsapi::Numeric::Integer(n)) => {
118+
Value::Integer(n)
119+
}
120+
turso_sdk_kit::rsapi::Value::Numeric(turso_sdk_kit::rsapi::Numeric::Float(n)) => {
121+
Value::Real(f64::from(n))
122+
}
119123
turso_sdk_kit::rsapi::Value::Text(t) => Value::Text(t.into()),
120124
turso_sdk_kit::rsapi::Value::Blob(items) => Value::Blob(items),
121125
}
@@ -126,8 +130,8 @@ impl From<Value> for turso_sdk_kit::rsapi::Value {
126130
fn from(val: Value) -> Self {
127131
match val {
128132
Value::Null => turso_sdk_kit::rsapi::Value::Null,
129-
Value::Integer(n) => turso_sdk_kit::rsapi::Value::Integer(n),
130-
Value::Real(n) => turso_sdk_kit::rsapi::Value::Float(n),
133+
Value::Integer(n) => turso_sdk_kit::rsapi::Value::from_i64(n),
134+
Value::Real(n) => turso_sdk_kit::rsapi::Value::from_f64(n),
131135
Value::Text(t) => turso_sdk_kit::rsapi::Value::from_text(t),
132136
Value::Blob(items) => turso_sdk_kit::rsapi::Value::from_blob(items),
133137
}

‎cli/app.rs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ use std::{
3333

3434
use tracing_appender::non_blocking::WorkerGuard;
3535
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilter};
36-
use turso_core::{Connection, Database, LimboError, OpenFlags, QueryMode, Statement, Value};
36+
use turso_core::{
37+
Connection, Database, LimboError, Numeric, OpenFlags, QueryMode, Statement, Value,
38+
};
3739

3840
#[derive(Parser, Debug)]
3941
#[command(name = "Turso")]
@@ -1106,8 +1108,7 @@ impl Limbo {
11061108
for (idx, value) in row.get_values().enumerate() {
11071109
let (content, alignment) = match value {
11081110
Value::Null => (null_value.clone(), CellAlignment::Left),
1109-
Value::Integer(_) => (format!("{value}"), CellAlignment::Right),
1110-
Value::Float(_) => (format!("{value}"), CellAlignment::Right),
1111+
Value::Numeric(_) => (format!("{value}"), CellAlignment::Right),
11111112
Value::Text(_) => (format!("{value}"), CellAlignment::Left),
11121113
Value::Blob(_) => (format!("{value}"), CellAlignment::Left),
11131114
};
@@ -1559,7 +1560,11 @@ impl Limbo {
15591560
let conn = self.conn.clone();
15601561
let mut databases = Vec::new();
15611562
self.handle_row(sql, |row| {
1562-
if let (Ok(Value::Integer(seq)), Ok(Value::Text(name)), Ok(file_value)) = (
1563+
if let (
1564+
Ok(Value::Numeric(Numeric::Integer(seq))),
1565+
Ok(Value::Text(name)),
1566+
Ok(file_value),
1567+
) = (
15631568
row.get::<&Value>(0),
15641569
row.get::<&Value>(1),
15651570
row.get::<&Value>(2),
@@ -1777,8 +1782,8 @@ impl Limbo {
17771782
fn write_sql_value_from_value<W: Write>(out: &mut W, v: &Value) -> io::Result<()> {
17781783
match v {
17791784
Value::Null => out.write_all(b"NULL"),
1780-
Value::Integer(i) => out.write_all(format!("{i}").as_bytes()),
1781-
Value::Float(f) => write!(out, "{f}").map(|_| ()),
1785+
Value::Numeric(Numeric::Integer(i)) => out.write_all(format!("{i}").as_bytes()),
1786+
Value::Numeric(Numeric::Float(f)) => write!(out, "{}", f64::from(*f)).map(|_| ()),
17821787
Value::Text(s) => {
17831788
out.write_all(b"'")?;
17841789
let bytes = s.value.as_bytes();

‎cli/mcp_server.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::sync::mpsc;
88
use std::sync::{Arc, Mutex};
99
use std::thread;
1010
use std::time::Duration;
11-
use turso_core::{Connection, Database, DatabaseOpts, OpenFlags, Value as DbValue};
11+
use turso_core::{Connection, Database, DatabaseOpts, Numeric, OpenFlags, Value as DbValue};
1212

1313
#[derive(Debug, Serialize, Deserialize)]
1414
struct JsonRpcRequest {
@@ -505,13 +505,13 @@ impl TursoMcpServer {
505505
"{} {} {} {} {}",
506506
col_name,
507507
col_type,
508-
if matches!(not_null, DbValue::Integer(1)) {
508+
if matches!(not_null, DbValue::Numeric(Numeric::Integer(1))) {
509509
"NOT NULL"
510510
} else {
511511
"NULL"
512512
},
513513
default_str,
514-
if matches!(pk, DbValue::Integer(1)) {
514+
if matches!(pk, DbValue::Numeric(Numeric::Integer(1))) {
515515
"PRIMARY KEY"
516516
} else {
517517
""

‎cli/sync_server.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -633,8 +633,8 @@ fn encode_length_delimited(output: &mut Vec<u8>, data: &[u8]) {
633633
fn convert_value_to_core(value: &Value) -> CoreValue {
634634
match value {
635635
Value::None | Value::Null => CoreValue::Null,
636-
Value::Integer { value } => CoreValue::Integer(*value),
637-
Value::Float { value } => CoreValue::Float(*value),
636+
Value::Integer { value } => CoreValue::from_i64(*value),
637+
Value::Float { value } => CoreValue::from_f64(*value),
638638
Value::Text { value } => CoreValue::Text(turso_core::types::Text {
639639
value: std::borrow::Cow::Owned(value.clone()),
640640
subtype: turso_core::types::TextSubtype::Text,
@@ -646,8 +646,10 @@ fn convert_value_to_core(value: &Value) -> CoreValue {
646646
fn convert_core_to_value(value: CoreValue) -> Value {
647647
match value {
648648
CoreValue::Null => Value::Null,
649-
CoreValue::Integer(v) => Value::Integer { value: v },
650-
CoreValue::Float(v) => Value::Float { value: v },
649+
CoreValue::Numeric(turso_core::Numeric::Integer(v)) => Value::Integer { value: v },
650+
CoreValue::Numeric(turso_core::Numeric::Float(v)) => Value::Float {
651+
value: f64::from(v),
652+
},
651653
CoreValue::Text(t) => Value::Text {
652654
value: t.value.to_string(),
653655
},

0 commit comments

Comments
 (0)