Repository: tursodatabase/limbo (language: rust)

Issue:
Zero-copy result row
We now keep even strings as byte vector in `OwnedValue`. Next step is to teach `OwnedValue` about blobs and strings that are managed by someone else and the figure out if we can just pin the btree page somehow and point there. This would be essentially zero-copy to return result row to apps, following SQLite's design.


Discussion / hints:
Hi @penberg. Do you know in what situations SQLite values should be static and not dynamic? Because I was trying to find it in sqlite source code, but it was not obvious to me the flow of data where the static data is referenced and when it will not be used anymore. This would help me understand the lifetime of the value so that I can attempt to do a zerocopy operation.

Fix patch:
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<String>,
-    blobs: Vec<Vec<u8>>,
 }
 
 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<u8>) -> &Vec<u8> {
-        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<PyObject> = 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<usize>, value: Value) {
+    pub fn bind_at(&mut self, index: NonZero<usize>, 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<OwnedValue> for OwnedValue {
     }
 }
 
-impl From<Value<'_>> 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<Self>
     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(),
     }
 }


Test patch:
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,