From 936ae307b79d8f76378e72e0f661551799da17ac Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Wed, 26 Feb 2025 09:41:22 +0200 Subject: [PATCH] core: Kill value type We currently have two value types, `Value` and `OwnedValue`. The original thinking was that `Value` is external type and `OwnedValue` is internal type. However, this just results in unnecessary transformation between the types as data crosses the Limbo library boundary. Let's just follow SQLite here and consolidate on a single value type (where `sqlite3_value` is just an alias for the internal `Mem` type). The way this will eventually work is that we can have bunch of pre-allocated `OwnedValue` objects in `ProgramState` and basically return a reference to them all the way to the application itself, which extracts the actual value. --- bindings/go/rs_src/rows.rs | 3 +- bindings/go/rs_src/types.rs | 49 ++++++------ bindings/java/rs_src/limbo_statement.rs | 29 +++---- bindings/python/src/lib.rs | 13 ++-- bindings/wasm/lib.rs | 17 ++-- cli/app.rs | 43 ++++++----- core/lib.rs | 5 +- core/types.rs | 77 +------------------ core/vdbe/mod.rs | 54 +++++++++++-- simulator/generation/plan.rs | 16 ++-- sqlite3/src/lib.rs | 37 ++++----- .../functions/test_function_rowid.rs | 14 ++-- tests/integration/fuzz/mod.rs | 13 ++-- .../query_processing/test_read_path.rs | 38 ++++----- .../query_processing/test_write_path.rs | 48 ++++++------ tests/integration/wal/test_wal.rs | 7 +- 16 files changed, 220 insertions(+), 243 deletions(-) diff --git a/bindings/go/rs_src/rows.rs b/bindings/go/rs_src/rows.rs index f296c9076..a1e6382d9 100644 --- a/bindings/go/rs_src/rows.rs +++ b/bindings/go/rs_src/rows.rs @@ -76,8 +76,7 @@ pub extern "C" fn rows_get_value(ctx: *mut c_void, col_idx: usize) -> *const c_v if let Some(row) = ctx.stmt.row() { if let Some(value) = row.get_values().get(col_idx) { - let value = value.to_value(); - return LimboValue::from_value(&value).to_ptr(); + return LimboValue::from_value(value).to_ptr(); } } std::ptr::null() diff --git a/bindings/go/rs_src/types.rs b/bindings/go/rs_src/types.rs index 11c9b251f..2d5235696 100644 --- a/bindings/go/rs_src/types.rs +++ b/bindings/go/rs_src/types.rs @@ -1,4 +1,6 @@ use std::ffi::{c_char, c_void}; +use std::rc::Rc; + #[allow(dead_code)] #[repr(C)] pub enum ResultCode { @@ -50,25 +52,18 @@ struct Blob { pub struct AllocPool { strings: Vec, - blobs: Vec>, } impl AllocPool { pub fn new() -> Self { AllocPool { strings: Vec::new(), - blobs: Vec::new(), } } pub fn add_string(&mut self, s: String) -> &String { self.strings.push(s); self.strings.last().unwrap() } - - pub fn add_blob(&mut self, b: Vec) -> &Vec { - self.blobs.push(b); - self.blobs.last().unwrap() - } } #[no_mangle] @@ -148,61 +143,65 @@ impl LimboValue { Box::into_raw(Box::new(self)) as *const c_void } - pub fn from_value(value: &limbo_core::Value<'_>) -> Self { + pub fn from_value(value: &limbo_core::OwnedValue) -> Self { match value { - limbo_core::Value::Integer(i) => { + limbo_core::OwnedValue::Integer(i) => { LimboValue::new(ValueType::Integer, ValueUnion::from_int(*i)) } - limbo_core::Value::Float(r) => { + limbo_core::OwnedValue::Float(r) => { LimboValue::new(ValueType::Real, ValueUnion::from_real(*r)) } - limbo_core::Value::Text(s) => LimboValue::new(ValueType::Text, ValueUnion::from_str(s)), - limbo_core::Value::Blob(b) => { + limbo_core::OwnedValue::Text(s) => { + LimboValue::new(ValueType::Text, ValueUnion::from_str(s.as_str())) + } + limbo_core::OwnedValue::Blob(b) => { LimboValue::new(ValueType::Blob, ValueUnion::from_bytes(b)) } - limbo_core::Value::Null => LimboValue::new(ValueType::Null, ValueUnion::from_null()), + limbo_core::OwnedValue::Null => { + LimboValue::new(ValueType::Null, ValueUnion::from_null()) + } + _ => unreachable!(), } } // The values we get from Go need to be temporarily owned by the statement until they are bound // then they can be cleaned up immediately afterwards - pub fn to_value<'pool>(&self, pool: &'pool mut AllocPool) -> limbo_core::Value<'pool> { + pub fn to_value(&self, pool: &mut AllocPool) -> limbo_core::OwnedValue { match self.value_type { ValueType::Integer => { if unsafe { self.value.int_val == 0 } { - return limbo_core::Value::Null; + return limbo_core::OwnedValue::Null; } - limbo_core::Value::Integer(unsafe { self.value.int_val }) + limbo_core::OwnedValue::Integer(unsafe { self.value.int_val }) } ValueType::Real => { if unsafe { self.value.real_val == 0.0 } { - return limbo_core::Value::Null; + return limbo_core::OwnedValue::Null; } - limbo_core::Value::Float(unsafe { self.value.real_val }) + limbo_core::OwnedValue::Float(unsafe { self.value.real_val }) } ValueType::Text => { if unsafe { self.value.text_ptr.is_null() } { - return limbo_core::Value::Null; + return limbo_core::OwnedValue::Null; } let cstr = unsafe { std::ffi::CStr::from_ptr(self.value.text_ptr) }; match cstr.to_str() { Ok(utf8_str) => { let owned = utf8_str.to_owned(); let borrowed = pool.add_string(owned); - limbo_core::Value::Text(borrowed) + limbo_core::OwnedValue::build_text(borrowed) } - Err(_) => limbo_core::Value::Null, + Err(_) => limbo_core::OwnedValue::Null, } } ValueType::Blob => { if unsafe { self.value.blob_ptr.is_null() } { - return limbo_core::Value::Null; + return limbo_core::OwnedValue::Null; } let bytes = self.value.to_bytes(); - let borrowed = pool.add_blob(bytes.to_vec()); - limbo_core::Value::Blob(borrowed) + limbo_core::OwnedValue::Blob(Rc::new(bytes.to_vec())) } - ValueType::Null => limbo_core::Value::Null, + ValueType::Null => limbo_core::OwnedValue::Null, } } } diff --git a/bindings/java/rs_src/limbo_statement.rs b/bindings/java/rs_src/limbo_statement.rs index 6430f2f90..579902d32 100644 --- a/bindings/java/rs_src/limbo_statement.rs +++ b/bindings/java/rs_src/limbo_statement.rs @@ -5,8 +5,9 @@ use crate::utils::set_err_msg_and_throw_exception; use jni::objects::{JByteArray, JObject, JObjectArray, JString, JValue}; use jni::sys::{jdouble, jint, jlong}; use jni::JNIEnv; -use limbo_core::{Statement, StepResult, Value}; +use limbo_core::{OwnedValue, Statement, StepResult}; use std::num::NonZero; +use std::rc::Rc; pub const STEP_RESULT_ID_ROW: i32 = 10; #[allow(dead_code)] @@ -105,17 +106,17 @@ fn row_to_obj_array<'local>( let obj_array = env.new_object_array(row.len() as i32, "java/lang/Object", JObject::null())?; for (i, value) in row.get_values().iter().enumerate() { - let value = value.to_value(); let obj = match value { - limbo_core::Value::Null => JObject::null(), - limbo_core::Value::Integer(i) => { - env.new_object("java/lang/Long", "(J)V", &[JValue::Long(i)])? + limbo_core::OwnedValue::Null => JObject::null(), + limbo_core::OwnedValue::Integer(i) => { + env.new_object("java/lang/Long", "(J)V", &[JValue::Long(*i)])? } - limbo_core::Value::Float(f) => { - env.new_object("java/lang/Double", "(D)V", &[JValue::Double(f)])? + limbo_core::OwnedValue::Float(f) => { + env.new_object("java/lang/Double", "(D)V", &[JValue::Double(*f)])? } - limbo_core::Value::Text(s) => env.new_string(s)?.into(), - limbo_core::Value::Blob(b) => env.byte_array_from_slice(b)?.into(), + limbo_core::OwnedValue::Text(s) => env.new_string(s.as_str())?.into(), + limbo_core::OwnedValue::Blob(b) => env.byte_array_from_slice(&b)?.into(), + _ => unreachable!(), }; if let Err(e) = env.set_object_array_element(&obj_array, i as i32, obj) { eprintln!("Error on parsing row: {:?}", e); @@ -163,7 +164,7 @@ pub extern "system" fn Java_tech_turso_core_LimboStatement_bindNull<'local>( }; stmt.stmt - .bind_at(NonZero::new(position as usize).unwrap(), Value::Null); + .bind_at(NonZero::new(position as usize).unwrap(), OwnedValue::Null); SQLITE_OK } @@ -185,7 +186,7 @@ pub extern "system" fn Java_tech_turso_core_LimboStatement_bindLong<'local>( stmt.stmt.bind_at( NonZero::new(position as usize).unwrap(), - Value::Integer(value), + OwnedValue::Integer(value), ); SQLITE_OK } @@ -208,7 +209,7 @@ pub extern "system" fn Java_tech_turso_core_LimboStatement_bindDouble<'local>( stmt.stmt.bind_at( NonZero::new(position as usize).unwrap(), - Value::Float(value), + OwnedValue::Float(value), ); SQLITE_OK } @@ -236,7 +237,7 @@ pub extern "system" fn Java_tech_turso_core_LimboStatement_bindText<'local>( stmt.stmt.bind_at( NonZero::new(position as usize).unwrap(), - Value::Text(text.as_str()), + OwnedValue::build_text(text.as_str()), ); SQLITE_OK } @@ -264,7 +265,7 @@ pub extern "system" fn Java_tech_turso_core_LimboStatement_bindBlob<'local>( stmt.stmt.bind_at( NonZero::new(position as usize).unwrap(), - Value::Blob(blob.as_ref()), + OwnedValue::Blob(Rc::new(blob)), ); SQLITE_OK } diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs index 31a82389d..31d1fed3b 100644 --- a/bindings/python/src/lib.rs +++ b/bindings/python/src/lib.rs @@ -302,12 +302,13 @@ fn row_to_py(py: Python, row: &limbo_core::Row) -> PyObject { let py_values: Vec = row .get_values() .iter() - .map(|value| match value.to_value() { - limbo_core::Value::Null => py.None(), - limbo_core::Value::Integer(i) => i.to_object(py), - limbo_core::Value::Float(f) => f.to_object(py), - limbo_core::Value::Text(s) => s.to_object(py), - limbo_core::Value::Blob(b) => b.to_object(py), + .map(|value| match value { + limbo_core::OwnedValue::Null => py.None(), + limbo_core::OwnedValue::Integer(i) => i.to_object(py), + limbo_core::OwnedValue::Float(f) => f.to_object(py), + limbo_core::OwnedValue::Text(s) => s.as_str().to_object(py), + limbo_core::OwnedValue::Blob(b) => b.to_object(py), + _ => unreachable!(), }) .collect(); diff --git a/bindings/wasm/lib.rs b/bindings/wasm/lib.rs index aecfecd2e..34213b171 100644 --- a/bindings/wasm/lib.rs +++ b/bindings/wasm/lib.rs @@ -77,7 +77,6 @@ impl RowIterator { let row = stmt.row().unwrap(); let row_array = Array::new(); for value in row.get_values() { - let value = value.to_value(); let value = to_js_value(value); row_array.push(&value); } @@ -118,7 +117,6 @@ impl Statement { let row = stmt.row().unwrap(); let row_array = js_sys::Array::new(); for value in row.get_values() { - let value = value.to_value(); let value = to_js_value(value); row_array.push(&value); } @@ -141,7 +139,6 @@ impl Statement { let row = stmt.row().unwrap(); let row_array = js_sys::Array::new(); for value in row.get_values() { - let value = value.to_value(); let value = to_js_value(value); row_array.push(&value); } @@ -189,19 +186,21 @@ impl Statement { } } -fn to_js_value(value: limbo_core::Value) -> JsValue { +fn to_js_value(value: &limbo_core::OwnedValue) -> JsValue { match value { - limbo_core::Value::Null => JsValue::null(), - limbo_core::Value::Integer(i) => { + limbo_core::OwnedValue::Null => JsValue::null(), + limbo_core::OwnedValue::Integer(i) => { + let i = *i; if i >= i32::MIN as i64 && i <= i32::MAX as i64 { JsValue::from(i as i32) } else { JsValue::from(i) } } - limbo_core::Value::Float(f) => JsValue::from(f), - limbo_core::Value::Text(t) => JsValue::from_str(t), - limbo_core::Value::Blob(b) => js_sys::Uint8Array::from(b).into(), + limbo_core::OwnedValue::Float(f) => JsValue::from(*f), + limbo_core::OwnedValue::Text(t) => JsValue::from_str(t.as_str()), + limbo_core::OwnedValue::Blob(b) => js_sys::Uint8Array::from(b.as_slice()).into(), + _ => unreachable!(), } } diff --git a/cli/app.rs b/cli/app.rs index eb82958c0..3082701d6 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -4,7 +4,7 @@ use crate::{ opcodes_dictionary::OPCODE_DESCRIPTIONS, }; use comfy_table::{Attribute, Cell, CellAlignment, ContentArrangement, Row, Table}; -use limbo_core::{Database, LimboError, Statement, StepResult, Value}; +use limbo_core::{Database, LimboError, OwnedValue, Statement, StepResult}; use clap::{Parser, ValueEnum}; use rustyline::DefaultEditor; @@ -662,19 +662,19 @@ impl<'a> Limbo<'a> { Ok(StepResult::Row) => { let row = rows.row().unwrap(); for (i, value) in row.get_values().iter().enumerate() { - let value = value.to_value(); if i > 0 { let _ = self.writer.write(b"|"); } let _ = self.writer.write( match value { - Value::Null => self.opts.null_value.clone(), - Value::Integer(i) => format!("{}", i), - Value::Float(f) => format!("{:?}", f), - Value::Text(s) => s.to_string(), - Value::Blob(b) => { + OwnedValue::Null => self.opts.null_value.clone(), + OwnedValue::Integer(i) => format!("{}", i), + OwnedValue::Float(f) => format!("{:?}", f), + OwnedValue::Text(s) => s.to_string(), + OwnedValue::Blob(b) => { format!("{}", String::from_utf8_lossy(b)) } + _ => unreachable!(), } .as_bytes(), )?; @@ -724,17 +724,22 @@ impl<'a> Limbo<'a> { let mut row = Row::new(); row.max_height(1); for value in record.get_values() { - let (content, alignment) = match value.to_value() { - Value::Null => { + let (content, alignment) = match value { + OwnedValue::Null => { (self.opts.null_value.clone(), CellAlignment::Left) } - Value::Integer(i) => (i.to_string(), CellAlignment::Right), - Value::Float(f) => (f.to_string(), CellAlignment::Right), - Value::Text(s) => (s.to_string(), CellAlignment::Left), - Value::Blob(b) => ( + OwnedValue::Integer(i) => { + (i.to_string(), CellAlignment::Right) + } + OwnedValue::Float(f) => { + (f.to_string(), CellAlignment::Right) + } + OwnedValue::Text(s) => (s.to_string(), CellAlignment::Left), + OwnedValue::Blob(b) => ( String::from_utf8_lossy(b).to_string(), CellAlignment::Left, ), + _ => unreachable!(), }; row.add_cell(Cell::new(content).set_alignment(alignment)); } @@ -796,10 +801,8 @@ impl<'a> Limbo<'a> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - if let Some(Value::Text(schema)) = - row.get_values().first().map(|v| v.to_value()) - { - let _ = self.write_fmt(format_args!("{};", schema)); + if let Some(OwnedValue::Text(schema)) = row.get_values().first() { + let _ = self.write_fmt(format_args!("{};", schema.as_str())); found = true; } } @@ -856,10 +859,8 @@ impl<'a> Limbo<'a> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - if let Some(Value::Text(table)) = - row.get_values().first().map(|v| v.to_value()) - { - tables.push_str(table); + if let Some(OwnedValue::Text(table)) = row.get_values().first() { + tables.push_str(table.as_str()); tables.push(' '); } } diff --git a/core/lib.rs b/core/lib.rs index 55ea42e18..634823807 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -48,8 +48,7 @@ use storage::sqlite3_ondisk::{DatabaseHeader, DATABASE_HEADER_SIZE}; pub use storage::wal::CheckpointMode; pub use storage::wal::WalFile; pub use storage::wal::WalFileShared; -use types::OwnedValue; -pub use types::Value; +pub use types::OwnedValue; use util::{columns_from_create_table_body, parse_schema_rows}; use vdbe::builder::QueryMode; use vdbe::VTabOpaqueCursor; @@ -497,7 +496,7 @@ impl Statement { self.program.parameters.count() } - pub fn bind_at(&mut self, index: NonZero, value: Value) { + pub fn bind_at(&mut self, index: NonZero, value: OwnedValue) { self.state.bind_at(index, value.into()); } diff --git a/core/types.rs b/core/types.rs index f1dafd31e..170ce7252 100644 --- a/core/types.rs +++ b/core/types.rs @@ -11,27 +11,6 @@ use crate::Result; use std::fmt::Display; use std::rc::Rc; -#[derive(Debug, Clone, PartialEq)] -pub enum Value<'a> { - Null, - Integer(i64), - Float(f64), - Text(&'a str), - Blob(&'a [u8]), -} - -impl Display for Value<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Null => write!(f, "NULL"), - Self::Integer(i) => write!(f, "{}", i), - Self::Float(fl) => write!(f, "{}", fl), - Self::Text(s) => write!(f, "{}", s), - Self::Blob(b) => write!(f, "{:?}", b), - } - } -} - #[derive(Debug, Clone, Copy, PartialEq)] pub enum OwnedValueType { Null, @@ -73,6 +52,10 @@ impl Text { } } + pub fn to_string(&self) -> String { + self.as_str().to_string() + } + pub fn as_str(&self) -> &str { unsafe { std::str::from_utf8_unchecked(self.value.as_ref()) } } @@ -128,46 +111,6 @@ impl OwnedValue { OwnedValue::Record(_) => OwnedValueType::Null, // Map Record to Null for FFI } } - - pub fn to_value(&self) -> Value<'_> { - match self { - OwnedValue::Null => Value::Null, - OwnedValue::Integer(i) => Value::Integer(*i), - OwnedValue::Float(f) => Value::Float(*f), - OwnedValue::Text(s) => Value::Text(s.as_str()), - OwnedValue::Blob(b) => Value::Blob(b), - OwnedValue::Agg(a) => match a.as_ref() { - AggContext::Avg(acc, _count) => match acc { - OwnedValue::Integer(i) => Value::Integer(*i), - OwnedValue::Float(f) => Value::Float(*f), - _ => Value::Float(0.0), - }, - AggContext::Sum(acc) => match acc { - OwnedValue::Integer(i) => Value::Integer(*i), - OwnedValue::Float(f) => Value::Float(*f), - _ => Value::Float(0.0), - }, - AggContext::Count(count) => count.to_value(), - AggContext::Max(max) => match max { - Some(max) => max.to_value(), - None => Value::Null, - }, - AggContext::Min(min) => match min { - Some(min) => min.to_value(), - None => Value::Null, - }, - AggContext::GroupConcat(s) => s.to_value(), - AggContext::External(ext_state) => { - let v = ext_state - .finalized_value - .as_ref() - .unwrap_or(&OwnedValue::Null); - v.to_value() - } - }, - OwnedValue::Record(_) => todo!(), - } - } } #[derive(Debug, Clone, PartialEq)] @@ -478,18 +421,6 @@ impl std::ops::DivAssign for OwnedValue { } } -impl From> for OwnedValue { - fn from(value: Value<'_>) -> Self { - match value { - Value::Null => OwnedValue::Null, - Value::Integer(i) => OwnedValue::Integer(i), - Value::Float(f) => OwnedValue::Float(f), - Value::Text(s) => OwnedValue::Text(Text::from_str(s)), - Value::Blob(b) => OwnedValue::Blob(Rc::new(b.to_owned())), - } - } -} - pub trait FromValue<'a> { fn from_value(value: &'a OwnedValue) -> Result where diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index fdf4a940e..8187c1487 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -1875,14 +1875,58 @@ impl Program { unreachable!(); }; *acc /= count.clone(); + state.registers[*register] = acc.clone(); + } + AggFunc::Sum | AggFunc::Total => { + let AggContext::Sum(acc) = agg.borrow_mut() else { + unreachable!(); + }; + let value = match acc { + OwnedValue::Integer(i) => OwnedValue::Integer(*i), + OwnedValue::Float(f) => OwnedValue::Float(*f), + _ => OwnedValue::Float(0.0), + }; + state.registers[*register] = value; + } + AggFunc::Count | AggFunc::Count0 => { + let AggContext::Count(count) = agg.borrow_mut() else { + unreachable!(); + }; + state.registers[*register] = count.clone(); + } + AggFunc::Max => { + let AggContext::Max(acc) = agg.borrow_mut() else { + unreachable!(); + }; + match acc { + Some(value) => state.registers[*register] = value.clone(), + None => state.registers[*register] = OwnedValue::Null, + } + } + AggFunc::Min => { + let AggContext::Min(acc) = agg.borrow_mut() else { + unreachable!(); + }; + match acc { + Some(value) => state.registers[*register] = value.clone(), + None => state.registers[*register] = OwnedValue::Null, + } + } + AggFunc::GroupConcat | AggFunc::StringAgg => { + let AggContext::GroupConcat(acc) = agg.borrow_mut() else { + unreachable!(); + }; + state.registers[*register] = acc.clone(); } - AggFunc::Sum | AggFunc::Total => {} - AggFunc::Count | AggFunc::Count0 => {} - AggFunc::Max => {} - AggFunc::Min => {} - AggFunc::GroupConcat | AggFunc::StringAgg => {} AggFunc::External(_) => { agg.compute_external()?; + let AggContext::External(agg_state) = agg.borrow_mut() else { + unreachable!(); + }; + match &agg_state.finalized_value { + Some(value) => state.registers[*register] = value.clone(), + None => state.registers[*register] = OwnedValue::Null, + } } }, OwnedValue::Null => { diff --git a/simulator/generation/plan.rs b/simulator/generation/plan.rs index 0b2952f21..74b29ccec 100644 --- a/simulator/generation/plan.rs +++ b/simulator/generation/plan.rs @@ -492,14 +492,16 @@ impl Interaction { StepResult::Row => { let row = rows.row().unwrap(); let mut r = Vec::new(); - for el in row.get_values() { - let v = el.to_value(); + for v in row.get_values() { let v = match v { - limbo_core::Value::Null => Value::Null, - limbo_core::Value::Integer(i) => Value::Integer(i), - limbo_core::Value::Float(f) => Value::Float(f), - limbo_core::Value::Text(t) => Value::Text(t.to_string()), - limbo_core::Value::Blob(b) => Value::Blob(b.to_vec()), + limbo_core::OwnedValue::Null => Value::Null, + limbo_core::OwnedValue::Integer(i) => Value::Integer(*i), + limbo_core::OwnedValue::Float(f) => Value::Float(*f), + limbo_core::OwnedValue::Text(t) => { + Value::Text(t.as_str().to_string()) + } + limbo_core::OwnedValue::Blob(b) => Value::Blob(b.to_vec()), + _ => unreachable!(), }; r.push(v); } diff --git a/sqlite3/src/lib.rs b/sqlite3/src/lib.rs index acace7259..ea11d3472 100644 --- a/sqlite3/src/lib.rs +++ b/sqlite3/src/lib.rs @@ -563,63 +563,64 @@ pub unsafe extern "C" fn sqlite3_column_bytes( #[no_mangle] pub unsafe extern "C" fn sqlite3_value_type(value: *mut ffi::c_void) -> ffi::c_int { - let value = value as *mut limbo_core::Value; + let value = value as *mut limbo_core::OwnedValue; let value = &*value; match value { - limbo_core::Value::Null => 0, - limbo_core::Value::Integer(_) => 1, - limbo_core::Value::Float(_) => 2, - limbo_core::Value::Text(_) => 3, - limbo_core::Value::Blob(_) => 4, + limbo_core::OwnedValue::Null => 0, + limbo_core::OwnedValue::Integer(_) => 1, + limbo_core::OwnedValue::Float(_) => 2, + limbo_core::OwnedValue::Text(_) => 3, + limbo_core::OwnedValue::Blob(_) => 4, + _ => unreachable!(), } } #[no_mangle] pub unsafe extern "C" fn sqlite3_value_int64(value: *mut ffi::c_void) -> i64 { - let value = value as *mut limbo_core::Value; + let value = value as *mut limbo_core::OwnedValue; let value = &*value; match value { - limbo_core::Value::Integer(i) => *i, + limbo_core::OwnedValue::Integer(i) => *i, _ => 0, } } #[no_mangle] pub unsafe extern "C" fn sqlite3_value_double(value: *mut ffi::c_void) -> f64 { - let value = value as *mut limbo_core::Value; + let value = value as *mut limbo_core::OwnedValue; let value = &*value; match value { - limbo_core::Value::Float(f) => *f, + limbo_core::OwnedValue::Float(f) => *f, _ => 0.0, } } #[no_mangle] pub unsafe extern "C" fn sqlite3_value_text(value: *mut ffi::c_void) -> *const ffi::c_uchar { - let value = value as *mut limbo_core::Value; + let value = value as *mut limbo_core::OwnedValue; let value = &*value; match value { - limbo_core::Value::Text(text) => text.as_bytes().as_ptr(), + limbo_core::OwnedValue::Text(text) => text.as_str().as_ptr(), _ => std::ptr::null(), } } #[no_mangle] pub unsafe extern "C" fn sqlite3_value_blob(value: *mut ffi::c_void) -> *const ffi::c_void { - let value = value as *mut limbo_core::Value; + let value = value as *mut limbo_core::OwnedValue; let value = &*value; match value { - limbo_core::Value::Blob(blob) => blob.as_ptr() as *const ffi::c_void, + limbo_core::OwnedValue::Blob(blob) => blob.as_ptr() as *const ffi::c_void, _ => std::ptr::null(), } } #[no_mangle] pub unsafe extern "C" fn sqlite3_value_bytes(value: *mut ffi::c_void) -> ffi::c_int { - let value = value as *mut limbo_core::Value; + let value = value as *mut limbo_core::OwnedValue; let value = &*value; match value { - limbo_core::Value::Blob(blob) => blob.len() as ffi::c_int, + limbo_core::OwnedValue::Blob(blob) => blob.len() as ffi::c_int, _ => 0, } } @@ -635,8 +636,8 @@ pub unsafe extern "C" fn sqlite3_column_text( Some(row) => row, None => return std::ptr::null(), }; - match row.get_values().get(idx as usize).map(|v| v.to_value()) { - Some(limbo_core::Value::Text(text)) => text.as_bytes().as_ptr(), + match row.get_values().get(idx as usize) { + Some(limbo_core::OwnedValue::Text(text)) => text.as_str().as_ptr(), _ => std::ptr::null(), } } diff --git a/tests/integration/functions/test_function_rowid.rs b/tests/integration/functions/test_function_rowid.rs index 3f5b5fdf8..78a77ce9f 100644 --- a/tests/integration/functions/test_function_rowid.rs +++ b/tests/integration/functions/test_function_rowid.rs @@ -1,5 +1,5 @@ use crate::common::{do_flush, TempDatabase}; -use limbo_core::{StepResult, Value}; +use limbo_core::StepResult; #[test] fn test_last_insert_rowid_basic() -> anyhow::Result<()> { @@ -30,8 +30,8 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - if let Value::Integer(id) = row.get_value(0).to_value() { - assert_eq!(id, 1, "First insert should have rowid 1"); + if let limbo_core::OwnedValue::Integer(id) = row.get_value(0) { + assert_eq!(*id, 1, "First insert should have rowid 1"); } } StepResult::IO => { @@ -66,8 +66,8 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - if let Value::Integer(id) = row.get_value(0).to_value() { - last_id = id; + if let limbo_core::OwnedValue::Integer(id) = row.get_value(0) { + last_id = *id; } } StepResult::IO => { @@ -112,8 +112,8 @@ fn test_integer_primary_key() -> anyhow::Result<()> { match select_query.step()? { StepResult::Row => { let row = select_query.row().unwrap(); - if let Value::Integer(id) = row.get_value(0).to_value() { - rowids.push(id); + if let limbo_core::OwnedValue::Integer(id) = row.get_value(0) { + rowids.push(*id); } } StepResult::IO => tmp_db.io.run_once()?, diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index 7360fb863..ad2ed48c0 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -73,12 +73,13 @@ mod tests { let row = row .get_values() .iter() - .map(|x| match x.to_value() { - limbo_core::Value::Null => rusqlite::types::Value::Null, - limbo_core::Value::Integer(x) => rusqlite::types::Value::Integer(x), - limbo_core::Value::Float(x) => rusqlite::types::Value::Real(x), - limbo_core::Value::Text(x) => rusqlite::types::Value::Text(x.to_string()), - limbo_core::Value::Blob(x) => rusqlite::types::Value::Blob(x.to_vec()), + .map(|x| match x { + limbo_core::OwnedValue::Null => rusqlite::types::Value::Null, + limbo_core::OwnedValue::Integer(x) => rusqlite::types::Value::Integer(*x), + limbo_core::OwnedValue::Float(x) => rusqlite::types::Value::Real(*x), + limbo_core::OwnedValue::Text(x) => rusqlite::types::Value::Text(x.to_string()), + limbo_core::OwnedValue::Blob(x) => rusqlite::types::Value::Blob(x.to_vec()), + _ => unreachable!(), }) .collect(); rows.push(row); diff --git a/tests/integration/query_processing/test_read_path.rs b/tests/integration/query_processing/test_read_path.rs index 8f84d4b9c..b90d8f9d4 100644 --- a/tests/integration/query_processing/test_read_path.rs +++ b/tests/integration/query_processing/test_read_path.rs @@ -1,5 +1,5 @@ use crate::common::TempDatabase; -use limbo_core::{StepResult, Value}; +use limbo_core::{OwnedValue, StepResult}; #[test] fn test_statement_reset_bind() -> anyhow::Result<()> { @@ -9,13 +9,13 @@ fn test_statement_reset_bind() -> anyhow::Result<()> { let mut stmt = conn.prepare("select ?")?; - stmt.bind_at(1.try_into()?, Value::Integer(1)); + stmt.bind_at(1.try_into()?, OwnedValue::Integer(1)); loop { match stmt.step()? { StepResult::Row => { let row = stmt.row().unwrap(); - assert_eq!(row.get_value(0).to_value(), Value::Integer(1)); + assert_eq!(*row.get_value(0), limbo_core::OwnedValue::Integer(1)); } StepResult::IO => tmp_db.io.run_once()?, _ => break, @@ -24,13 +24,13 @@ fn test_statement_reset_bind() -> anyhow::Result<()> { stmt.reset(); - stmt.bind_at(1.try_into()?, Value::Integer(2)); + stmt.bind_at(1.try_into()?, OwnedValue::Integer(2)); loop { match stmt.step()? { StepResult::Row => { let row = stmt.row().unwrap(); - assert_eq!(row.get_value(0).to_value(), Value::Integer(2)); + assert_eq!(*row.get_value(0), limbo_core::OwnedValue::Integer(2)); } StepResult::IO => tmp_db.io.run_once()?, _ => break, @@ -48,14 +48,14 @@ fn test_statement_bind() -> anyhow::Result<()> { let mut stmt = conn.prepare("select ?, ?1, :named, ?3, ?4")?; - stmt.bind_at(1.try_into()?, Value::Text("hello")); + stmt.bind_at(1.try_into()?, OwnedValue::build_text("hello")); let i = stmt.parameters().index(":named").unwrap(); - stmt.bind_at(i, Value::Integer(42)); + stmt.bind_at(i, OwnedValue::Integer(42)); - stmt.bind_at(3.try_into()?, Value::Blob(&[0x1, 0x2, 0x3])); + stmt.bind_at(3.try_into()?, OwnedValue::from_blob(vec![0x1, 0x2, 0x3])); - stmt.bind_at(4.try_into()?, Value::Float(0.5)); + stmt.bind_at(4.try_into()?, OwnedValue::Float(0.5)); assert_eq!(stmt.parameters().count(), 4); @@ -63,24 +63,24 @@ fn test_statement_bind() -> anyhow::Result<()> { match stmt.step()? { StepResult::Row => { let row = stmt.row().unwrap(); - if let Value::Text(s) = row.get_value(0).to_value() { - assert_eq!(s, "hello") + if let limbo_core::OwnedValue::Text(s) = row.get_value(0) { + assert_eq!(s.as_str(), "hello") } - if let Value::Text(s) = row.get_value(1).to_value() { - assert_eq!(s, "hello") + if let limbo_core::OwnedValue::Text(s) = row.get_value(1) { + assert_eq!(s.as_str(), "hello") } - if let Value::Integer(i) = row.get_value(2).to_value() { - assert_eq!(i, 42) + if let limbo_core::OwnedValue::Integer(i) = row.get_value(2) { + assert_eq!(*i, 42) } - if let Value::Blob(v) = row.get_value(3).to_value() { - assert_eq!(v, &vec![0x1 as u8, 0x2, 0x3]) + if let limbo_core::OwnedValue::Blob(v) = row.get_value(3) { + assert_eq!(v.as_ref(), &vec![0x1 as u8, 0x2, 0x3]) } - if let Value::Float(f) = row.get_value(4).to_value() { - assert_eq!(f, 0.5) + if let limbo_core::OwnedValue::Float(f) = row.get_value(4) { + assert_eq!(*f, 0.5) } } StepResult::IO => { diff --git a/tests/integration/query_processing/test_write_path.rs b/tests/integration/query_processing/test_write_path.rs index d1b8c50d6..0d9d68b41 100644 --- a/tests/integration/query_processing/test_write_path.rs +++ b/tests/integration/query_processing/test_write_path.rs @@ -1,6 +1,6 @@ use crate::common; use crate::common::{compare_string, do_flush, TempDatabase}; -use limbo_core::{Connection, StepResult, Value}; +use limbo_core::{Connection, StepResult}; use log::debug; use std::rc::Rc; @@ -43,15 +43,15 @@ fn test_simple_overflow_page() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0).to_value(); - let text = row.get_value(1).to_value(); + let first_value = row.get_value(0); + let text = row.get_value(1); let id = match first_value { - Value::Integer(i) => i as i32, - Value::Float(f) => f as i32, + limbo_core::OwnedValue::Integer(i) => *i as i32, + limbo_core::OwnedValue::Float(f) => *f as i32, _ => unreachable!(), }; let text = match text { - Value::Text(t) => t, + limbo_core::OwnedValue::Text(t) => t.as_str(), _ => unreachable!(), }; assert_eq!(1, id); @@ -118,15 +118,15 @@ fn test_sequential_overflow_page() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0).to_value(); - let text = row.get_value(1).to_value(); + let first_value = row.get_value(0); + let text = row.get_value(1); let id = match first_value { - Value::Integer(i) => i as i32, - Value::Float(f) => f as i32, + limbo_core::OwnedValue::Integer(i) => *i as i32, + limbo_core::OwnedValue::Float(f) => *f as i32, _ => unreachable!(), }; let text = match text { - Value::Text(t) => t, + limbo_core::OwnedValue::Text(t) => t.as_str(), _ => unreachable!(), }; let huge_text = &huge_texts[current_index]; @@ -191,9 +191,9 @@ fn test_sequential_write() -> anyhow::Result<()> { StepResult::Row => { let row = rows.row().unwrap(); let first_value = row.get_values().first().expect("missing id"); - let id = match first_value.to_value() { - Value::Integer(i) => i as i32, - Value::Float(f) => f as i32, + let id = match first_value { + limbo_core::OwnedValue::Integer(i) => *i as i32, + limbo_core::OwnedValue::Float(f) => *f as i32, _ => unreachable!(), }; assert_eq!(current_read_index, id); @@ -257,8 +257,8 @@ fn test_regression_multi_row_insert() -> anyhow::Result<()> { StepResult::Row => { let row = rows.row().unwrap(); let first_value = row.get_values().first().expect("missing id"); - let id = match first_value.to_value() { - Value::Float(f) => f as i32, + let id = match first_value { + limbo_core::OwnedValue::Float(f) => *f as i32, _ => panic!("expected float"), }; actual_ids.push(id); @@ -302,7 +302,7 @@ fn test_statement_reset() -> anyhow::Result<()> { match stmt.step()? { StepResult::Row => { let row = stmt.row().unwrap(); - assert_eq!(row.get_value(0).to_value(), Value::Integer(1)); + assert_eq!(*row.get_value(0), limbo_core::OwnedValue::Integer(1)); break; } StepResult::IO => tmp_db.io.run_once()?, @@ -316,7 +316,7 @@ fn test_statement_reset() -> anyhow::Result<()> { match stmt.step()? { StepResult::Row => { let row = stmt.row().unwrap(); - assert_eq!(row.get_value(0).to_value(), Value::Integer(1)); + assert_eq!(*row.get_value(0), limbo_core::OwnedValue::Integer(1)); break; } StepResult::IO => tmp_db.io.run_once()?, @@ -366,10 +366,10 @@ fn test_wal_checkpoint() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0).to_value(); + let first_value = row.get_value(0); let id = match first_value { - Value::Integer(i) => i as i32, - Value::Float(f) => f as i32, + limbo_core::OwnedValue::Integer(i) => *i as i32, + limbo_core::OwnedValue::Float(f) => *f as i32, _ => unreachable!(), }; assert_eq!(current_index, id as usize); @@ -430,13 +430,13 @@ fn test_wal_restart() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0).to_value(); + let first_value = row.get_value(0); let count = match first_value { - Value::Integer(i) => i, + limbo_core::OwnedValue::Integer(i) => i, _ => unreachable!(), }; debug!("counted {}", count); - return Ok(count as usize); + return Ok(*count as usize); } StepResult::IO => { tmp_db.io.run_once()?; diff --git a/tests/integration/wal/test_wal.rs b/tests/integration/wal/test_wal.rs index d3d3005ce..79e9a0cd5 100644 --- a/tests/integration/wal/test_wal.rs +++ b/tests/integration/wal/test_wal.rs @@ -1,5 +1,5 @@ use crate::common::{do_flush, TempDatabase}; -use limbo_core::{Connection, LimboError, Result, StepResult, Value}; +use limbo_core::{Connection, LimboError, Result, StepResult}; use std::cell::RefCell; use std::rc::Rc; @@ -73,16 +73,15 @@ pub(crate) fn execute_and_get_ints( StepResult::Row => { let row = stmt.row().unwrap(); for value in row.get_values() { - let value = value.to_value(); let out = match value { - Value::Integer(i) => i, + limbo_core::OwnedValue::Integer(i) => i, _ => { return Err(LimboError::ConversionError(format!( "cannot convert {value} to int" ))) } }; - result.push(out); + result.push(*out); } } StepResult::Done => break,