From 77a412f6afb85da2389510cbd9a39306fcd55b5f Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Sun, 5 Oct 2025 21:31:29 -0300 Subject: [PATCH 1/2] refactor: remove unsafe reference semantics from `RefValue` also renames `RefValue` to `ValueRef`, to align with rusqlite and other crates --- core/incremental/aggregate_operator.rs | 8 +- core/incremental/cursor.rs | 15 +- core/json/mod.rs | 40 +- core/lib.rs | 2 +- .../mvcc/database/checkpoint_state_machine.rs | 6 +- core/mvcc/database/mod.rs | 4 +- core/mvcc/database/tests.rs | 6 +- core/mvcc/persistent_storage/logical_log.rs | 8 +- core/schema.rs | 28 +- core/storage/btree.rs | 10 +- core/storage/sqlite3_ondisk.rs | 37 +- core/types.rs | 378 +++++++++--------- core/vdbe/execute.rs | 34 +- core/vdbe/mod.rs | 20 +- core/vdbe/sorter.rs | 81 ++-- 15 files changed, 329 insertions(+), 348 deletions(-) diff --git a/core/incremental/aggregate_operator.rs b/core/incremental/aggregate_operator.rs index e24b930a9..e577f05e2 100644 --- a/core/incremental/aggregate_operator.rs +++ b/core/incremental/aggregate_operator.rs @@ -7,7 +7,7 @@ use crate::incremental::operator::{ generate_storage_id, ComputationTracker, DbspStateCursors, EvalState, IncrementalOperator, }; use crate::incremental::persistence::{ReadRecord, WriteRow}; -use crate::types::{IOResult, ImmutableRecord, RefValue, SeekKey, SeekOp, SeekResult}; +use crate::types::{IOResult, ImmutableRecord, SeekKey, SeekOp, SeekResult, ValueRef}; use crate::{return_and_restore_if_io, return_if_io, LimboError, Result, Value}; use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt::{self, Display}; @@ -1546,7 +1546,7 @@ impl ScanState { }; // Check if we're still in the same group - if let RefValue::Integer(rec_sid) = rec_storage_id { + if let ValueRef::Integer(rec_sid) = rec_storage_id { if *rec_sid != storage_id { return Ok(IOResult::Done(None)); } @@ -1555,8 +1555,8 @@ impl ScanState { } // Compare zset_hash as blob - if let RefValue::Blob(rec_zset_blob) = rec_zset_hash { - if let Some(rec_hash) = Hash128::from_blob(rec_zset_blob.to_slice()) { + if let ValueRef::Blob(rec_zset_blob) = rec_zset_hash { + if let Some(rec_hash) = Hash128::from_blob(rec_zset_blob) { if rec_hash != zset_hash { return Ok(IOResult::Done(None)); } diff --git a/core/incremental/cursor.rs b/core/incremental/cursor.rs index 67814bd0d..22070f29d 100644 --- a/core/incremental/cursor.rs +++ b/core/incremental/cursor.rs @@ -117,15 +117,12 @@ impl MaterializedViewCursor { Some(rowid) => rowid, }; - let btree_record = return_if_io!(self.btree_cursor.record()); - let btree_ref_values = btree_record - .ok_or_else(|| { - crate::LimboError::InternalError( - "Invalid data in materialized view: found a rowid, but not the row!" - .to_string(), - ) - })? - .get_values(); + let btree_record = return_if_io!(self.btree_cursor.record()).ok_or_else(|| { + crate::LimboError::InternalError( + "Invalid data in materialized view: found a rowid, but not the row!".to_string(), + ) + })?; + let btree_ref_values = btree_record.get_values(); // Convert RefValues to Values (copying for now - can optimize later) let mut btree_values: Vec = diff --git a/core/json/mod.rs b/core/json/mod.rs index 6209b6774..d3b1f4e19 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -11,9 +11,9 @@ pub use crate::json::ops::{ jsonb_replace, }; use crate::json::path::{json_path, JsonPath, PathElement}; -use crate::types::{RawSlice, Text, TextRef, TextSubtype, Value, ValueType}; +use crate::types::{Text, TextSubtype, Value, ValueType}; use crate::vdbe::Register; -use crate::{bail_constraint_error, bail_parse_error, LimboError, RefValue}; +use crate::{bail_constraint_error, bail_parse_error, LimboError, ValueRef}; pub use cache::JsonCacheCell; use jsonb::{ElementType, Jsonb, JsonbHeader, PathOperationMode, SearchOperation, SetOperation}; use std::borrow::Cow; @@ -105,14 +105,12 @@ pub fn json_from_raw_bytes_agg(data: &[u8], raw: bool) -> crate::Result { pub fn convert_dbtype_to_jsonb(val: &Value, strict: Conv) -> crate::Result { convert_ref_dbtype_to_jsonb( - &match val { - Value::Null => RefValue::Null, - Value::Integer(x) => RefValue::Integer(*x), - Value::Float(x) => RefValue::Float(*x), - Value::Text(text) => { - RefValue::Text(TextRef::create_from(text.as_str().as_bytes(), text.subtype)) - } - Value::Blob(items) => RefValue::Blob(RawSlice::create_from(items)), + match val { + Value::Null => ValueRef::Null, + Value::Integer(x) => ValueRef::Integer(*x), + Value::Float(x) => ValueRef::Float(*x), + Value::Text(text) => ValueRef::Text(text.as_str().as_bytes(), text.subtype), + Value::Blob(items) => ValueRef::Blob(items.as_slice()), }, strict, ) @@ -124,14 +122,14 @@ fn parse_as_json_text(slice: &[u8]) -> crate::Result { Jsonb::from_str_with_mode(str, Conv::Strict).map_err(Into::into) } -pub fn convert_ref_dbtype_to_jsonb(val: &RefValue, strict: Conv) -> crate::Result { +pub fn convert_ref_dbtype_to_jsonb(val: ValueRef<'_>, strict: Conv) -> crate::Result { match val { - RefValue::Text(text) => { - let res = if text.subtype == TextSubtype::Json || matches!(strict, Conv::Strict) { - Jsonb::from_str_with_mode(text.as_str(), strict) + ValueRef::Text(text, subtype) => { + let res = if subtype == TextSubtype::Json || matches!(strict, Conv::Strict) { + Jsonb::from_str_with_mode(&String::from_utf8_lossy(text), strict) } else { // Handle as a string literal otherwise - let mut str = text.as_str().replace('"', "\\\""); + let mut str = String::from_utf8_lossy(text).replace('"', "\\\""); // Quote the string to make it a JSON string str.insert(0, '"'); str.push('"'); @@ -139,8 +137,8 @@ pub fn convert_ref_dbtype_to_jsonb(val: &RefValue, strict: Conv) -> crate::Resul }; res.map_err(|_| LimboError::ParseError("malformed JSON".to_string())) } - RefValue::Blob(blob) => { - let bytes = blob.to_slice(); + ValueRef::Blob(blob) => { + let bytes = blob; // Valid JSON can start with these whitespace characters let index = bytes .iter() @@ -177,15 +175,15 @@ pub fn convert_ref_dbtype_to_jsonb(val: &RefValue, strict: Conv) -> crate::Resul json.element_type()?; Ok(json) } - RefValue::Null => Ok(Jsonb::from_raw_data( + ValueRef::Null => Ok(Jsonb::from_raw_data( JsonbHeader::make_null().into_bytes().as_bytes(), )), - RefValue::Float(float) => { + ValueRef::Float(float) => { let mut buff = ryu::Buffer::new(); - Jsonb::from_str(buff.format(*float)) + Jsonb::from_str(buff.format(float)) .map_err(|_| LimboError::ParseError("malformed JSON".to_string())) } - RefValue::Integer(int) => Jsonb::from_str(&int.to_string()) + ValueRef::Integer(int) => Jsonb::from_str(&int.to_string()) .map_err(|_| LimboError::ParseError("malformed JSON".to_string())), } } diff --git a/core/lib.rs b/core/lib.rs index cf51ae283..8145af6e7 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -97,8 +97,8 @@ use turso_macros::match_ignore_ascii_case; use turso_parser::ast::fmt::ToTokens; use turso_parser::{ast, ast::Cmd, parser::Parser}; use types::IOResult; -pub use types::RefValue; pub use types::Value; +pub use types::ValueRef; use util::parse_schema_rows; pub use util::IOExt; pub use vdbe::{builder::QueryMode, explain::EXPLAIN_COLUMNS, explain::EXPLAIN_QUERY_PLAN_COLUMNS}; diff --git a/core/mvcc/database/checkpoint_state_machine.rs b/core/mvcc/database/checkpoint_state_machine.rs index 5a7e6a38f..fbe8b2b84 100644 --- a/core/mvcc/database/checkpoint_state_machine.rs +++ b/core/mvcc/database/checkpoint_state_machine.rs @@ -9,8 +9,8 @@ use crate::storage::pager::CreateBTreeFlags; use crate::storage::wal::{CheckpointMode, TursoRwLock}; use crate::types::{IOCompletions, IOResult, ImmutableRecord, RecordCursor}; use crate::{ - CheckpointResult, Completion, Connection, IOExt, Pager, RefValue, Result, TransactionState, - Value, + CheckpointResult, Completion, Connection, IOExt, Pager, Result, TransactionState, Value, + ValueRef, }; use parking_lot::RwLock; use std::collections::{HashMap, HashSet}; @@ -191,7 +191,7 @@ impl CheckpointStateMachine { let row_data = ImmutableRecord::from_bin_record(row_data.clone()); let mut record_cursor = RecordCursor::new(); record_cursor.parse_full_header(&row_data).unwrap(); - let RefValue::Integer(root_page) = + let ValueRef::Integer(root_page) = record_cursor.get_value(&row_data, 3).unwrap() else { panic!( diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs index 7f1dd573a..95014cbc2 100644 --- a/core/mvcc/database/mod.rs +++ b/core/mvcc/database/mod.rs @@ -18,11 +18,11 @@ use crate::Completion; use crate::File; use crate::IOExt; use crate::LimboError; -use crate::RefValue; use crate::Result; use crate::Statement; use crate::StepResult; use crate::Value; +use crate::ValueRef; use crate::{Connection, Pager}; use crossbeam_skiplist::{SkipMap, SkipSet}; use parking_lot::RwLock; @@ -1978,7 +1978,7 @@ impl MvStore { let record = ImmutableRecord::from_bin_record(row_data); let mut record_cursor = RecordCursor::new(); let record_values = record_cursor.get_values(&record).unwrap(); - let RefValue::Integer(root_page) = record_values[3] else { + let ValueRef::Integer(root_page) = record_values[3] else { panic!( "Expected integer value for root page, got {:?}", record_values[3] diff --git a/core/mvcc/database/tests.rs b/core/mvcc/database/tests.rs index a3aa8db5d..2c8d23106 100644 --- a/core/mvcc/database/tests.rs +++ b/core/mvcc/database/tests.rs @@ -714,7 +714,7 @@ use crate::types::Text; use crate::Value; use crate::{Database, StepResult}; use crate::{MemoryIO, Statement}; -use crate::{RefValue, DATABASE_MANAGER}; +use crate::{ValueRef, DATABASE_MANAGER}; // Simple atomic clock implementation for testing @@ -978,7 +978,7 @@ fn test_cursor_modification_during_scan() { record.start_serialization(&row.data); let value = record.get_value(0).unwrap(); match value { - RefValue::Text(text) => { + ValueRef::Text(text) => { assert_eq!(text.as_str(), "new_row"); } _ => panic!("Expected Text value"), @@ -1210,7 +1210,7 @@ fn test_restart() { .unwrap(); let record = get_record_value(&row); match record.get_value(0).unwrap() { - RefValue::Text(text) => { + ValueRef::Text(text) => { assert_eq!(text.as_str(), "bar"); } _ => panic!("Expected Text value"), diff --git a/core/mvcc/persistent_storage/logical_log.rs b/core/mvcc/persistent_storage/logical_log.rs index a902bac98..52a01bb27 100644 --- a/core/mvcc/persistent_storage/logical_log.rs +++ b/core/mvcc/persistent_storage/logical_log.rs @@ -501,7 +501,7 @@ mod tests { LocalClock, MvStore, }, types::{ImmutableRecord, Text}, - OpenFlags, RefValue, Value, + OpenFlags, Value, ValueRef, }; use super::LogRecordType; @@ -565,7 +565,7 @@ mod tests { let record = ImmutableRecord::from_bin_record(row.data.clone()); let values = record.get_values(); let foo = values.first().unwrap(); - let RefValue::Text(foo) = foo else { + let ValueRef::Text(foo) = foo else { unreachable!() }; assert_eq!(foo.as_str(), "foo"); @@ -637,7 +637,7 @@ mod tests { let record = ImmutableRecord::from_bin_record(row.data.clone()); let values = record.get_values(); let foo = values.first().unwrap(); - let RefValue::Text(foo) = foo else { + let ValueRef::Text(foo) = foo else { unreachable!() }; assert_eq!(foo.as_str(), value.as_str()); @@ -758,7 +758,7 @@ mod tests { let record = ImmutableRecord::from_bin_record(row.data.clone()); let values = record.get_values(); let foo = values.first().unwrap(); - let RefValue::Text(foo) = foo else { + let ValueRef::Text(foo) = foo else { unreachable!() }; diff --git a/core/schema.rs b/core/schema.rs index 9208c5d1d..106dc30c5 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -80,7 +80,7 @@ use crate::util::{ }; use crate::{ bail_parse_error, contains_ignore_ascii_case, eq_ignore_ascii_case, match_ignore_ascii_case, - Connection, LimboError, MvCursor, MvStore, Pager, RefValue, SymbolTable, VirtualTable, + Connection, LimboError, MvCursor, MvStore, Pager, SymbolTable, ValueRef, VirtualTable, }; use crate::{util::normalize_ident, Result}; use core::fmt; @@ -428,36 +428,36 @@ impl Schema { let mut record_cursor = cursor.record_cursor.borrow_mut(); // sqlite schema table has 5 columns: type, name, tbl_name, rootpage, sql let ty_value = record_cursor.get_value(&row, 0)?; - let RefValue::Text(ty) = ty_value else { + let ValueRef::Text(ty, _) = ty_value else { return Err(LimboError::ConversionError("Expected text value".into())); }; - let ty = ty.as_str(); - let RefValue::Text(name) = record_cursor.get_value(&row, 1)? else { + let ty = String::from_utf8_lossy(ty); + let ValueRef::Text(name, _) = record_cursor.get_value(&row, 1)? else { return Err(LimboError::ConversionError("Expected text value".into())); }; - let name = name.as_str(); + let name = String::from_utf8_lossy(name); let table_name_value = record_cursor.get_value(&row, 2)?; - let RefValue::Text(table_name) = table_name_value else { + let ValueRef::Text(table_name, _) = table_name_value else { return Err(LimboError::ConversionError("Expected text value".into())); }; - let table_name = table_name.as_str(); + let table_name = String::from_utf8_lossy(table_name); let root_page_value = record_cursor.get_value(&row, 3)?; - let RefValue::Integer(root_page) = root_page_value else { + let ValueRef::Integer(root_page) = root_page_value else { return Err(LimboError::ConversionError("Expected integer value".into())); }; let sql_value = record_cursor.get_value(&row, 4)?; let sql_textref = match sql_value { - RefValue::Text(sql) => Some(sql), + ValueRef::Text(sql, _) => Some(sql), _ => None, }; - let sql = sql_textref.as_ref().map(|s| s.as_str()); + let sql = sql_textref.map(|s| String::from_utf8_lossy(s)); self.handle_schema_row( - ty, - name, - table_name, + &ty, + &name, + &table_name, root_page, - sql, + sql.as_deref(), syms, &mut from_sql_indexes, &mut automatic_indices, diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 71a63aba6..7935d3595 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -28,7 +28,7 @@ use crate::{ use crate::{ return_corrupt, return_if_io, - types::{compare_immutable, IOResult, ImmutableRecord, RefValue, SeekKey, SeekOp, Value}, + types::{compare_immutable, IOResult, ImmutableRecord, SeekKey, SeekOp, Value, ValueRef}, LimboError, Result, }; @@ -705,7 +705,7 @@ impl BTreeCursor { .unwrap() .last_value(record_cursor) { - Some(Ok(RefValue::Integer(rowid))) => rowid, + Some(Ok(ValueRef::Integer(rowid))) => rowid, _ => unreachable!( "index where has_rowid() is true should have an integer rowid as the last value" ), @@ -2164,7 +2164,7 @@ impl BTreeCursor { fn compare_with_current_record( &self, - key_values: &[RefValue], + key_values: &[ValueRef], seek_op: SeekOp, record_comparer: &RecordCompare, index_info: &IndexInfo, @@ -8930,7 +8930,7 @@ mod tests { let record = record.as_ref().unwrap(); let cur = record.get_values().clone(); let cur = cur.first().unwrap(); - let RefValue::Blob(ref cur) = cur else { + let ValueRef::Blob(ref cur) = cur else { panic!("expected blob, got {cur:?}"); }; assert_eq!( @@ -9473,7 +9473,7 @@ mod tests { let value = record.unwrap().get_value(0)?; assert_eq!( value, - RefValue::Integer(i), + ValueRef::Integer(i), "Unexpected value for record {i}", ); } diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index 6dfe12bd1..08f7addc3 100644 --- a/core/storage/sqlite3_ondisk.rs +++ b/core/storage/sqlite3_ondisk.rs @@ -61,7 +61,7 @@ use crate::storage::buffer_pool::BufferPool; use crate::storage::database::{DatabaseFile, DatabaseStorage, EncryptionOrChecksum}; use crate::storage::pager::Pager; use crate::storage::wal::READMARK_NOT_USED; -use crate::types::{RawSlice, RefValue, SerialType, SerialTypeKind, TextRef, TextSubtype}; +use crate::types::{SerialType, SerialTypeKind, TextSubtype, ValueRef}; use crate::{ bail_corrupt_error, turso_assert, CompletionError, File, IOContext, Result, WalFileShared, }; @@ -1320,22 +1320,22 @@ impl Iterator for SmallVecIter<'_, T, N> { /// Reads a value that might reference the buffer it is reading from. Be sure to store RefValue with the buffer /// always. #[inline(always)] -pub fn read_value(buf: &[u8], serial_type: SerialType) -> Result<(RefValue, usize)> { +pub fn read_value<'a>(buf: &'a [u8], serial_type: SerialType) -> Result<(ValueRef<'a>, usize)> { match serial_type.kind() { - SerialTypeKind::Null => Ok((RefValue::Null, 0)), + SerialTypeKind::Null => Ok((ValueRef::Null, 0)), SerialTypeKind::I8 => { if buf.is_empty() { crate::bail_corrupt_error!("Invalid UInt8 value"); } let val = buf[0] as i8; - Ok((RefValue::Integer(val as i64), 1)) + Ok((ValueRef::Integer(val as i64), 1)) } SerialTypeKind::I16 => { if buf.len() < 2 { crate::bail_corrupt_error!("Invalid BEInt16 value"); } Ok(( - RefValue::Integer(i16::from_be_bytes([buf[0], buf[1]]) as i64), + ValueRef::Integer(i16::from_be_bytes([buf[0], buf[1]]) as i64), 2, )) } @@ -1345,7 +1345,7 @@ pub fn read_value(buf: &[u8], serial_type: SerialType) -> Result<(RefValue, usiz } let sign_extension = if buf[0] <= 127 { 0 } else { 255 }; Ok(( - RefValue::Integer( + ValueRef::Integer( i32::from_be_bytes([sign_extension, buf[0], buf[1], buf[2]]) as i64 ), 3, @@ -1356,7 +1356,7 @@ pub fn read_value(buf: &[u8], serial_type: SerialType) -> Result<(RefValue, usiz crate::bail_corrupt_error!("Invalid BEInt32 value"); } Ok(( - RefValue::Integer(i32::from_be_bytes([buf[0], buf[1], buf[2], buf[3]]) as i64), + ValueRef::Integer(i32::from_be_bytes([buf[0], buf[1], buf[2], buf[3]]) as i64), 4, )) } @@ -1366,7 +1366,7 @@ pub fn read_value(buf: &[u8], serial_type: SerialType) -> Result<(RefValue, usiz } let sign_extension = if buf[0] <= 127 { 0 } else { 255 }; Ok(( - RefValue::Integer(i64::from_be_bytes([ + ValueRef::Integer(i64::from_be_bytes([ sign_extension, sign_extension, buf[0], @@ -1384,7 +1384,7 @@ pub fn read_value(buf: &[u8], serial_type: SerialType) -> Result<(RefValue, usiz crate::bail_corrupt_error!("Invalid BEInt64 value"); } Ok(( - RefValue::Integer(i64::from_be_bytes([ + ValueRef::Integer(i64::from_be_bytes([ buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7], ])), 8, @@ -1395,26 +1395,20 @@ pub fn read_value(buf: &[u8], serial_type: SerialType) -> Result<(RefValue, usiz crate::bail_corrupt_error!("Invalid BEFloat64 value"); } Ok(( - RefValue::Float(f64::from_be_bytes([ + ValueRef::Float(f64::from_be_bytes([ buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7], ])), 8, )) } - SerialTypeKind::ConstInt0 => Ok((RefValue::Integer(0), 0)), - SerialTypeKind::ConstInt1 => Ok((RefValue::Integer(1), 0)), + SerialTypeKind::ConstInt0 => Ok((ValueRef::Integer(0), 0)), + SerialTypeKind::ConstInt1 => Ok((ValueRef::Integer(1), 0)), SerialTypeKind::Blob => { let content_size = serial_type.size(); if buf.len() < content_size { crate::bail_corrupt_error!("Invalid Blob value"); } - if content_size == 0 { - Ok((RefValue::Blob(RawSlice::new(std::ptr::null(), 0)), 0)) - } else { - let ptr = &buf[0] as *const u8; - let slice = RawSlice::new(ptr, content_size); - Ok((RefValue::Blob(slice), content_size)) - } + Ok((ValueRef::Blob(&buf[..content_size]), content_size)) } SerialTypeKind::Text => { let content_size = serial_type.size(); @@ -1427,10 +1421,7 @@ pub fn read_value(buf: &[u8], serial_type: SerialType) -> Result<(RefValue, usiz } Ok(( - RefValue::Text(TextRef::create_from( - &buf[..content_size], - TextSubtype::Text, - )), + ValueRef::Text(&buf[..content_size], TextSubtype::Text), content_size, )) } diff --git a/core/types.rs b/core/types.rs index dc4b2162e..b86c72726 100644 --- a/core/types.rs +++ b/core/types.rs @@ -255,24 +255,24 @@ pub struct RawSlice { len: usize, } -#[derive(PartialEq, Clone)] -pub enum RefValue { +#[derive(PartialEq, Clone, Copy)] +pub enum ValueRef<'a> { Null, Integer(i64), Float(f64), - Text(TextRef), - Blob(RawSlice), + Text(&'a [u8], TextSubtype), + Blob(&'a [u8]), } -impl Debug for RefValue { +impl Debug for ValueRef<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - RefValue::Null => write!(f, "Null"), - RefValue::Integer(i) => f.debug_tuple("Integer").field(i).finish(), - RefValue::Float(float) => f.debug_tuple("Float").field(float).finish(), - RefValue::Text(text_ref) => { + ValueRef::Null => write!(f, "Null"), + ValueRef::Integer(i) => f.debug_tuple("Integer").field(i).finish(), + ValueRef::Float(float) => f.debug_tuple("Float").field(float).finish(), + ValueRef::Text(text_ref, _) => { // truncate string to at most 256 chars - let text = text_ref.as_str(); + let text = String::from_utf8_lossy(text_ref); let max_len = text.len().min(256); f.debug_struct("Text") .field("data", &&text[0..max_len]) @@ -280,9 +280,8 @@ impl Debug for RefValue { .field("truncated", &(text.len() > max_len)) .finish() } - RefValue::Blob(raw_slice) => { + ValueRef::Blob(blob) => { // truncate blob_slice to at most 32 bytes - let blob = raw_slice.to_slice(); let max_len = blob.len().min(32); f.debug_struct("Blob") .field("data", &&blob[0..max_len]) @@ -295,6 +294,16 @@ impl Debug for RefValue { } impl Value { + pub fn as_ref<'a>(&'a self) -> ValueRef<'a> { + match self { + Value::Null => ValueRef::Null, + Value::Integer(v) => ValueRef::Integer(*v), + Value::Float(v) => ValueRef::Float(*v), + Value::Text(v) => ValueRef::Text(v.value.as_slice(), v.subtype), + Value::Blob(v) => ValueRef::Blob(v.as_slice()), + } + } + // A helper function that makes building a text Value easier. pub fn build_text(text: impl AsRef) -> Self { Self::Text(Text::new(text.as_ref())) @@ -833,34 +842,36 @@ impl std::ops::DivAssign for Value { } } -impl<'a> TryFrom<&'a RefValue> for i64 { +impl TryFrom> for i64 { type Error = LimboError; - fn try_from(value: &'a RefValue) -> Result { + fn try_from(value: ValueRef<'_>) -> Result { match value { - RefValue::Integer(i) => Ok(*i), + ValueRef::Integer(i) => Ok(i), _ => Err(LimboError::ConversionError("Expected integer value".into())), } } } -impl<'a> TryFrom<&'a RefValue> for String { +impl TryFrom> for String { type Error = LimboError; - fn try_from(value: &'a RefValue) -> Result { + fn try_from(value: ValueRef<'_>) -> Result { match value { - RefValue::Text(s) => Ok(s.as_str().to_string()), + ValueRef::Text(s, _) => Ok(String::from_utf8_lossy(s).to_string()), _ => Err(LimboError::ConversionError("Expected text value".into())), } } } -impl<'a> TryFrom<&'a RefValue> for &'a str { +impl<'a> TryFrom> for &'a str { type Error = LimboError; - fn try_from(value: &'a RefValue) -> Result { + fn try_from(value: ValueRef<'a>) -> Result { match value { - RefValue::Text(s) => Ok(s.as_str()), + ValueRef::Text(s, _) => Ok(str::from_utf8(s).map_err(|_| { + LimboError::ConversionError("Expected a valid UTF8 string".to_string()) + })?), _ => Err(LimboError::ConversionError("Expected text value".into())), } } @@ -987,7 +998,7 @@ impl ImmutableRecord { // TODO: inline the complete record parsing code here. // Its probably more efficient. - pub fn get_values(&self) -> Vec { + pub fn get_values<'a>(&'a self) -> Vec> { let mut cursor = RecordCursor::new(); cursor.get_values(self).unwrap_or_default() } @@ -1007,7 +1018,6 @@ impl ImmutableRecord { values: impl IntoIterator + Clone, len: usize, ) -> Self { - let mut ref_values = Vec::with_capacity(len); let mut serials = Vec::with_capacity(len); let mut size_header = 0; let mut size_values = 0; @@ -1044,13 +1054,9 @@ impl ImmutableRecord { // write content for value in values { - let start_offset = writer.pos; match value { - Value::Null => { - ref_values.push(RefValue::Null); - } + Value::Null => {} Value::Integer(i) => { - ref_values.push(RefValue::Integer(*i)); let serial_type = SerialType::from(value); match serial_type.kind() { SerialTypeKind::ConstInt0 | SerialTypeKind::ConstInt1 => {} @@ -1065,27 +1071,12 @@ impl ImmutableRecord { other => panic!("Serial type is not an integer: {other:?}"), } } - Value::Float(f) => { - ref_values.push(RefValue::Float(*f)); - writer.extend_from_slice(&f.to_be_bytes()) - } + Value::Float(f) => writer.extend_from_slice(&f.to_be_bytes()), Value::Text(t) => { writer.extend_from_slice(&t.value); - let end_offset = writer.pos; - let len = end_offset - start_offset; - let ptr = unsafe { writer.buf.as_ptr().add(start_offset) }; - let value = RefValue::Text(TextRef { - value: RawSlice::new(ptr, len), - subtype: t.subtype, - }); - ref_values.push(value); } Value::Blob(b) => { writer.extend_from_slice(b); - let end_offset = writer.pos; - let len = end_offset - start_offset; - let ptr = unsafe { writer.buf.as_ptr().add(start_offset) }; - ref_values.push(RefValue::Blob(RawSlice::new(ptr, len))); } }; } @@ -1132,7 +1123,10 @@ impl ImmutableRecord { // TODO: its probably better to not instantiate the RecordCurosr. Instead do the deserialization // inside the function. - pub fn last_value(&self, record_cursor: &mut RecordCursor) -> Option> { + pub fn last_value<'a>( + &'a self, + record_cursor: &mut RecordCursor, + ) -> Option>> { if self.is_invalidated() { return Some(Err(LimboError::InternalError( "Record is invalidated".into(), @@ -1143,12 +1137,12 @@ impl ImmutableRecord { Some(record_cursor.get_value(self, last_idx)) } - pub fn get_value(&self, idx: usize) -> Result { + pub fn get_value<'a>(&'a self, idx: usize) -> Result> { let mut cursor = RecordCursor::new(); cursor.get_value(self, idx) } - pub fn get_value_opt(&self, idx: usize) -> Option { + pub fn get_value_opt<'a>(&'a self, idx: usize) -> Option> { if self.is_invalidated() { return None; } @@ -1314,23 +1308,27 @@ impl RecordCursor { /// # Special Cases /// /// - Returns `RefValue::Null` for out-of-bounds indices - pub fn deserialize_column(&self, record: &ImmutableRecord, idx: usize) -> Result { + pub fn deserialize_column<'a>( + &self, + record: &'a ImmutableRecord, + idx: usize, + ) -> Result> { if idx >= self.serial_types.len() { - return Ok(RefValue::Null); + return Ok(ValueRef::Null); } let serial_type = self.serial_types[idx]; let serial_type_obj = SerialType::try_from(serial_type)?; match serial_type_obj.kind() { - SerialTypeKind::Null => return Ok(RefValue::Null), - SerialTypeKind::ConstInt0 => return Ok(RefValue::Integer(0)), - SerialTypeKind::ConstInt1 => return Ok(RefValue::Integer(1)), + SerialTypeKind::Null => return Ok(ValueRef::Null), + SerialTypeKind::ConstInt0 => return Ok(ValueRef::Integer(0)), + SerialTypeKind::ConstInt1 => return Ok(ValueRef::Integer(1)), _ => {} // continue } if idx + 1 >= self.offsets.len() { - return Ok(RefValue::Null); + return Ok(ValueRef::Null); } let start = self.offsets[idx]; @@ -1358,7 +1356,11 @@ impl RecordCursor { /// * `Err(LimboError)` - Access failed due to invalid record or parsing error /// #[inline(always)] - pub fn get_value(&mut self, record: &ImmutableRecord, idx: usize) -> Result { + pub fn get_value<'a>( + &mut self, + record: &'a ImmutableRecord, + idx: usize, + ) -> Result> { if record.is_invalidated() { return Err(LimboError::InternalError("Record not initialized".into())); } @@ -1380,11 +1382,11 @@ impl RecordCursor { /// * `Some(Err(LimboError))` - Parsing succeeded but deserialization failed /// * `None` - Record is invalid or index is out of bounds /// - pub fn get_value_opt( + pub fn get_value_opt<'a>( &mut self, - record: &ImmutableRecord, + record: &'a ImmutableRecord, idx: usize, - ) -> Option> { + ) -> Option>> { if record.is_invalidated() { return None; } @@ -1443,7 +1445,7 @@ impl RecordCursor { /// * `Ok(Vec)` - All values in column order /// * `Err(LimboError)` - Parsing or deserialization failed /// - pub fn get_values(&mut self, record: &ImmutableRecord) -> Result> { + pub fn get_values<'a>(&mut self, record: &'a ImmutableRecord) -> Result>> { if record.is_invalidated() { return Ok(Vec::new()); } @@ -1459,62 +1461,62 @@ impl RecordCursor { } } -impl RefValue { +impl<'a> ValueRef<'a> { pub fn to_ffi(&self) -> ExtValue { match self { Self::Null => ExtValue::null(), Self::Integer(i) => ExtValue::from_integer(*i), Self::Float(fl) => ExtValue::from_float(*fl), - Self::Text(text) => ExtValue::from_text( - std::str::from_utf8(text.value.to_slice()) - .unwrap() - .to_string(), - ), - Self::Blob(blob) => ExtValue::from_blob(blob.to_slice().to_vec()), + Self::Text(text, _) => { + ExtValue::from_text(std::str::from_utf8(text).unwrap().to_string()) + } + Self::Blob(blob) => ExtValue::from_blob(blob.to_vec()), + } + } + + pub fn to_blob(&self) -> Option<&'a [u8]> { + match self { + Self::Blob(blob) => Some(*blob), + _ => None, } } pub fn to_owned(&self) -> Value { match self { - RefValue::Null => Value::Null, - RefValue::Integer(i) => Value::Integer(*i), - RefValue::Float(f) => Value::Float(*f), - RefValue::Text(text_ref) => Value::Text(Text { - value: text_ref.value.to_slice().to_vec(), - subtype: text_ref.subtype, + ValueRef::Null => Value::Null, + ValueRef::Integer(i) => Value::Integer(*i), + ValueRef::Float(f) => Value::Float(*f), + ValueRef::Text(text, subtype) => Value::Text(Text { + value: text.to_vec(), + subtype: *subtype, }), - RefValue::Blob(b) => Value::Blob(b.to_slice().to_vec()), - } - } - pub fn to_blob(&self) -> Option<&[u8]> { - match self { - Self::Blob(blob) => Some(blob.to_slice()), - _ => None, + ValueRef::Blob(b) => Value::Blob(b.to_vec()), } } } -impl Display for RefValue { +impl Display for ValueRef<'_> { 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.as_str()), - Self::Blob(b) => write!(f, "{}", String::from_utf8_lossy(b.to_slice())), + Self::Text(s, _) => write!(f, "{}", String::from_utf8_lossy(s)), + Self::Blob(b) => write!(f, "{}", String::from_utf8_lossy(b)), } } } -impl Eq for RefValue {} -impl Ord for RefValue { +impl Eq for ValueRef<'_> {} + +impl Ord for ValueRef<'_> { fn cmp(&self, other: &Self) -> std::cmp::Ordering { self.partial_cmp(other).unwrap() } } #[allow(clippy::non_canonical_partial_ord_impl)] -impl PartialOrd for RefValue { +impl<'a> PartialOrd> for ValueRef<'a> { fn partial_cmp(&self, other: &Self) -> Option { match (self, other) { (Self::Integer(int_left), Self::Integer(int_right)) => int_left.partial_cmp(int_right), @@ -1528,24 +1530,21 @@ impl PartialOrd for RefValue { float_left.partial_cmp(float_right) } // Numeric vs Text/Blob - (Self::Integer(_) | Self::Float(_), Self::Text(_) | Self::Blob(_)) => { + (Self::Integer(_) | Self::Float(_), Self::Text(_, _) | Self::Blob(_)) => { Some(std::cmp::Ordering::Less) } - (Self::Text(_) | Self::Blob(_), Self::Integer(_) | Self::Float(_)) => { + (Self::Text(_, _) | Self::Blob(_), Self::Integer(_) | Self::Float(_)) => { Some(std::cmp::Ordering::Greater) } - (Self::Text(text_left), Self::Text(text_right)) => text_left - .value - .to_slice() - .partial_cmp(text_right.value.to_slice()), - // Text vs Blob - (Self::Text(_), Self::Blob(_)) => Some(std::cmp::Ordering::Less), - (Self::Blob(_), Self::Text(_)) => Some(std::cmp::Ordering::Greater), - - (Self::Blob(blob_left), Self::Blob(blob_right)) => { - blob_left.to_slice().partial_cmp(blob_right.to_slice()) + (Self::Text(text_left, _), Self::Text(text_right, _)) => { + text_left.partial_cmp(text_right) } + // Text vs Blob + (Self::Text(_, _), Self::Blob(_)) => Some(std::cmp::Ordering::Less), + (Self::Blob(_), Self::Text(_, _)) => Some(std::cmp::Ordering::Greater), + + (Self::Blob(blob_left), Self::Blob(blob_right)) => blob_left.partial_cmp(blob_right), (Self::Null, Self::Null) => Some(std::cmp::Ordering::Equal), (Self::Null, _) => Some(std::cmp::Ordering::Less), (_, Self::Null) => Some(std::cmp::Ordering::Greater), @@ -1635,8 +1634,8 @@ impl IndexInfo { } pub fn compare_immutable( - l: &[RefValue], - r: &[RefValue], + l: &[ValueRef], + r: &[ValueRef], column_info: &[KeyInfo], ) -> std::cmp::Ordering { assert_eq!(l.len(), r.len()); @@ -1645,9 +1644,10 @@ pub fn compare_immutable( let column_order = column_info[i].sort_order; let collation = column_info[i].collation; let cmp = match (l, r) { - (RefValue::Text(left), RefValue::Text(right)) => { - collation.compare_strings(left.as_str(), right.as_str()) - } + (ValueRef::Text(left, _), ValueRef::Text(right, _)) => collation.compare_strings( + &String::from_utf8_lossy(left), + &String::from_utf8_lossy(right), + ), _ => l.partial_cmp(r).unwrap(), }; if !cmp.is_eq() { @@ -1671,7 +1671,7 @@ impl RecordCompare { pub fn compare( &self, serialized: &ImmutableRecord, - unpacked: &[RefValue], + unpacked: &[ValueRef], index_info: &IndexInfo, skip: usize, tie_breaker: std::cmp::Ordering, @@ -1690,11 +1690,11 @@ impl RecordCompare { } } -pub fn find_compare(unpacked: &[RefValue], index_info: &IndexInfo) -> RecordCompare { +pub fn find_compare(unpacked: &[ValueRef], index_info: &IndexInfo) -> RecordCompare { if !unpacked.is_empty() && index_info.num_cols <= 13 { match &unpacked[0] { - RefValue::Integer(_) => RecordCompare::Int, - RefValue::Text(_) if index_info.key_info[0].collation == CollationSeq::Binary => { + ValueRef::Integer(_) => RecordCompare::Int, + ValueRef::Text(_, _) if index_info.key_info[0].collation == CollationSeq::Binary => { RecordCompare::String } _ => RecordCompare::Generic, @@ -1760,7 +1760,7 @@ pub fn get_tie_breaker_from_seek_op(seek_op: SeekOp) -> std::cmp::Ordering { /// delegates to `compare_records_generic()` with `skip=1` fn compare_records_int( serialized: &ImmutableRecord, - unpacked: &[RefValue], + unpacked: &[ValueRef], index_info: &IndexInfo, tie_breaker: std::cmp::Ordering, ) -> Result { @@ -1794,7 +1794,7 @@ fn compare_records_int( let data_start = header_size; let lhs_int = read_integer(&payload[data_start..], first_serial_type as u8)?; - let RefValue::Integer(rhs_int) = unpacked[0] else { + let ValueRef::Integer(rhs_int) = unpacked[0] else { return compare_records_generic(serialized, unpacked, index_info, 0, tie_breaker); }; let comparison = match index_info.key_info[0].sort_order { @@ -1853,7 +1853,7 @@ fn compare_records_int( /// delegates to `compare_records_generic()` with `skip=1` fn compare_records_string( serialized: &ImmutableRecord, - unpacked: &[RefValue], + unpacked: &[ValueRef], index_info: &IndexInfo, tie_breaker: std::cmp::Ordering, ) -> Result { @@ -1884,7 +1884,7 @@ fn compare_records_string( return compare_records_generic(serialized, unpacked, index_info, 0, tie_breaker); } - let RefValue::Text(rhs_text) = &unpacked[0] else { + let ValueRef::Text(rhs_text, _) = &unpacked[0] else { return compare_records_generic(serialized, unpacked, index_info, 0, tie_breaker); }; @@ -1896,12 +1896,15 @@ fn compare_records_string( let serial_type = SerialType::try_from(first_serial_type)?; let (lhs_value, _) = read_value(&payload[data_start..], serial_type)?; - let RefValue::Text(lhs_text) = lhs_value else { + let ValueRef::Text(lhs_text, _) = lhs_value else { return compare_records_generic(serialized, unpacked, index_info, 0, tie_breaker); }; let collation = index_info.key_info[0].collation; - let comparison = collation.compare_strings(lhs_text.as_str(), rhs_text.as_str()); + let comparison = collation.compare_strings( + &String::from_utf8_lossy(lhs_text), + &String::from_utf8_lossy(rhs_text), + ); let final_comparison = match index_info.key_info[0].sort_order { SortOrder::Asc => comparison, @@ -1910,7 +1913,7 @@ fn compare_records_string( match final_comparison { std::cmp::Ordering::Equal => { - let len_cmp = lhs_text.value.len.cmp(&rhs_text.value.len); + let len_cmp = lhs_text.len().cmp(&rhs_text.len()); if len_cmp != std::cmp::Ordering::Equal { let adjusted = match index_info.key_info[0].sort_order { SortOrder::Asc => len_cmp, @@ -1962,7 +1965,7 @@ fn compare_records_string( /// `tie_breaker` is returned. pub fn compare_records_generic( serialized: &ImmutableRecord, - unpacked: &[RefValue], + unpacked: &[ValueRef], index_info: &IndexInfo, skip: usize, tie_breaker: std::cmp::Ordering, @@ -2009,9 +2012,9 @@ pub fn compare_records_generic( let rhs_value = &unpacked[field_idx]; let lhs_value = match serial_type.kind() { - SerialTypeKind::ConstInt0 => RefValue::Integer(0), - SerialTypeKind::ConstInt1 => RefValue::Integer(1), - SerialTypeKind::Null => RefValue::Null, + SerialTypeKind::ConstInt0 => ValueRef::Integer(0), + SerialTypeKind::ConstInt1 => ValueRef::Integer(1), + SerialTypeKind::Null => ValueRef::Null, _ => { let (value, field_size) = read_value(&payload[data_pos..], serial_type)?; data_pos += field_size; @@ -2020,15 +2023,18 @@ pub fn compare_records_generic( }; let comparison = match (&lhs_value, rhs_value) { - (RefValue::Text(lhs_text), RefValue::Text(rhs_text)) => index_info.key_info[field_idx] - .collation - .compare_strings(lhs_text.as_str(), rhs_text.as_str()), + (ValueRef::Text(lhs_text, _), ValueRef::Text(rhs_text, _)) => { + index_info.key_info[field_idx].collation.compare_strings( + &String::from_utf8_lossy(lhs_text), + &String::from_utf8_lossy(rhs_text), + ) + } - (RefValue::Integer(lhs_int), RefValue::Float(rhs_float)) => { + (ValueRef::Integer(lhs_int), ValueRef::Float(rhs_float)) => { sqlite_int_float_compare(*lhs_int, *rhs_float) } - (RefValue::Float(lhs_float), RefValue::Integer(rhs_int)) => { + (ValueRef::Float(lhs_float), ValueRef::Integer(rhs_int)) => { sqlite_int_float_compare(*rhs_int, *lhs_float).reverse() } @@ -2623,8 +2629,8 @@ mod tests { use crate::translate::collate::CollationSeq; pub fn compare_immutable_for_testing( - l: &[RefValue], - r: &[RefValue], + l: &[ValueRef], + r: &[ValueRef], index_key_info: &[KeyInfo], tie_breaker: std::cmp::Ordering, ) -> std::cmp::Ordering { @@ -2635,7 +2641,7 @@ mod tests { let collation = index_key_info[i].collation; let cmp = match (&l[i], &r[i]) { - (RefValue::Text(left), RefValue::Text(right)) => { + (ValueRef::Text(left), ValueRef::Text(right)) => { collation.compare_strings(left.as_str(), right.as_str()) } _ => l[i].partial_cmp(&r[i]).unwrap_or(std::cmp::Ordering::Equal), @@ -2676,16 +2682,16 @@ mod tests { } } - fn value_to_ref_value(value: &Value) -> RefValue { + fn value_to_ref_value(value: &Value) -> ValueRef { match value { - Value::Null => RefValue::Null, - Value::Integer(i) => RefValue::Integer(*i), - Value::Float(f) => RefValue::Float(*f), - Value::Text(text) => RefValue::Text(TextRef { + Value::Null => ValueRef::Null, + Value::Integer(i) => ValueRef::Integer(*i), + Value::Float(f) => ValueRef::Float(*f), + Value::Text(text) => ValueRef::Text(TextRef { value: RawSlice::from_slice(&text.value), subtype: text.subtype, }), - Value::Blob(blob) => RefValue::Blob(RawSlice::from_slice(blob)), + Value::Blob(blob) => ValueRef::Blob(RawSlice::from_slice(blob)), } } @@ -2709,13 +2715,13 @@ mod tests { fn assert_compare_matches_full_comparison( serialized_values: Vec, - unpacked_values: Vec, + unpacked_values: Vec, index_info: &IndexInfo, test_name: &str, ) { let serialized = create_record(serialized_values.clone()); - let serialized_ref_values: Vec = + let serialized_ref_values: Vec = serialized_values.iter().map(value_to_ref_value).collect(); let tie_breaker = std::cmp::Ordering::Equal; @@ -2815,52 +2821,52 @@ mod tests { let test_cases = vec![ ( vec![Value::Integer(42)], - vec![RefValue::Integer(42)], + vec![ValueRef::Integer(42)], "equal_integers", ), ( vec![Value::Integer(10)], - vec![RefValue::Integer(20)], + vec![ValueRef::Integer(20)], "less_than_integers", ), ( vec![Value::Integer(30)], - vec![RefValue::Integer(20)], + vec![ValueRef::Integer(20)], "greater_than_integers", ), ( vec![Value::Integer(0)], - vec![RefValue::Integer(0)], + vec![ValueRef::Integer(0)], "zero_integers", ), ( vec![Value::Integer(-5)], - vec![RefValue::Integer(-5)], + vec![ValueRef::Integer(-5)], "negative_integers", ), ( vec![Value::Integer(i64::MAX)], - vec![RefValue::Integer(i64::MAX)], + vec![ValueRef::Integer(i64::MAX)], "max_integers", ), ( vec![Value::Integer(i64::MIN)], - vec![RefValue::Integer(i64::MIN)], + vec![ValueRef::Integer(i64::MIN)], "min_integers", ), ( vec![Value::Integer(42), Value::Text(Text::new("hello"))], vec![ - RefValue::Integer(42), - RefValue::Text(TextRef::from_str("hello")), + ValueRef::Integer(42), + ValueRef::Text(TextRef::from_str("hello")), ], "integer_text_equal", ), ( vec![Value::Integer(42), Value::Text(Text::new("hello"))], vec![ - RefValue::Integer(42), - RefValue::Text(TextRef::from_str("world")), + ValueRef::Integer(42), + ValueRef::Text(TextRef::from_str("world")), ], "integer_equal_text_different", ), @@ -2887,43 +2893,43 @@ mod tests { let test_cases = vec![ ( vec![Value::Text(Text::new("hello"))], - vec![RefValue::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(TextRef::from_str("hello"))], "equal_strings", ), ( vec![Value::Text(Text::new("abc"))], - vec![RefValue::Text(TextRef::from_str("def"))], + vec![ValueRef::Text(TextRef::from_str("def"))], "less_than_strings", ), ( vec![Value::Text(Text::new("xyz"))], - vec![RefValue::Text(TextRef::from_str("abc"))], + vec![ValueRef::Text(TextRef::from_str("abc"))], "greater_than_strings", ), ( vec![Value::Text(Text::new(""))], - vec![RefValue::Text(TextRef::from_str(""))], + vec![ValueRef::Text(TextRef::from_str(""))], "empty_strings", ), ( vec![Value::Text(Text::new("a"))], - vec![RefValue::Text(TextRef::from_str("aa"))], + vec![ValueRef::Text(TextRef::from_str("aa"))], "prefix_strings", ), // Multi-field with string first ( vec![Value::Text(Text::new("hello")), Value::Integer(42)], vec![ - RefValue::Text(TextRef::from_str("hello")), - RefValue::Integer(42), + ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Integer(42), ], "string_integer_equal", ), ( vec![Value::Text(Text::new("hello")), Value::Integer(42)], vec![ - RefValue::Text(TextRef::from_str("hello")), - RefValue::Integer(99), + ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Integer(99), ], "string_equal_integer_different", ), @@ -2948,65 +2954,65 @@ mod tests { // NULL vs others ( vec![Value::Null], - vec![RefValue::Integer(42)], + vec![ValueRef::Integer(42)], "null_vs_integer", ), ( vec![Value::Null], - vec![RefValue::Float(64.4)], + vec![ValueRef::Float(64.4)], "null_vs_float", ), ( vec![Value::Null], - vec![RefValue::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(TextRef::from_str("hello"))], "null_vs_text", ), ( vec![Value::Null], - vec![RefValue::Blob(RawSlice::from_slice(b"blob"))], + vec![ValueRef::Blob(RawSlice::from_slice(b"blob"))], "null_vs_blob", ), // Numbers vs Text/Blob ( vec![Value::Integer(42)], - vec![RefValue::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(TextRef::from_str("hello"))], "integer_vs_text", ), ( vec![Value::Float(64.4)], - vec![RefValue::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(TextRef::from_str("hello"))], "float_vs_text", ), ( vec![Value::Integer(42)], - vec![RefValue::Blob(RawSlice::from_slice(b"blob"))], + vec![ValueRef::Blob(RawSlice::from_slice(b"blob"))], "integer_vs_blob", ), ( vec![Value::Float(64.4)], - vec![RefValue::Blob(RawSlice::from_slice(b"blob"))], + vec![ValueRef::Blob(RawSlice::from_slice(b"blob"))], "float_vs_blob", ), // Text vs Blob ( vec![Value::Text(Text::new("hello"))], - vec![RefValue::Blob(RawSlice::from_slice(b"blob"))], + vec![ValueRef::Blob(RawSlice::from_slice(b"blob"))], "text_vs_blob", ), // Integer vs Float (affinity conversion) ( vec![Value::Integer(42)], - vec![RefValue::Float(42.0)], + vec![ValueRef::Float(42.0)], "integer_vs_equal_float", ), ( vec![Value::Integer(42)], - vec![RefValue::Float(42.5)], + vec![ValueRef::Float(42.5)], "integer_vs_different_float", ), ( vec![Value::Float(42.5)], - vec![RefValue::Integer(42)], + vec![ValueRef::Integer(42)], "float_vs_integer", ), ]; @@ -3033,20 +3039,20 @@ mod tests { // DESC order should reverse first field comparison ( vec![Value::Integer(10)], - vec![RefValue::Integer(20)], + vec![ValueRef::Integer(20)], "desc_integer_reversed", ), ( vec![Value::Text(Text::new("abc"))], - vec![RefValue::Text(TextRef::from_str("def"))], + vec![ValueRef::Text(TextRef::from_str("def"))], "desc_string_reversed", ), // Mixed sort orders ( vec![Value::Integer(10), Value::Text(Text::new("hello"))], vec![ - RefValue::Integer(20), - RefValue::Text(TextRef::from_str("hello")), + ValueRef::Integer(20), + ValueRef::Text(TextRef::from_str("hello")), ], "desc_first_asc_second", ), @@ -3071,38 +3077,38 @@ mod tests { ( vec![Value::Integer(42)], vec![ - RefValue::Integer(42), - RefValue::Text(TextRef::from_str("extra")), + ValueRef::Integer(42), + ValueRef::Text(TextRef::from_str("extra")), ], "fewer_serialized_fields", ), ( vec![Value::Integer(42), Value::Text(Text::new("extra"))], - vec![RefValue::Integer(42)], + vec![ValueRef::Integer(42)], "fewer_unpacked_fields", ), (vec![], vec![], "both_empty"), - (vec![], vec![RefValue::Integer(42)], "empty_serialized"), + (vec![], vec![ValueRef::Integer(42)], "empty_serialized"), ( (0..15).map(Value::Integer).collect(), - (0..15).map(RefValue::Integer).collect(), + (0..15).map(ValueRef::Integer).collect(), "large_field_count", ), ( vec![Value::Blob(vec![1, 2, 3])], - vec![RefValue::Blob(RawSlice::from_slice(&[1, 2, 3]))], + vec![ValueRef::Blob(RawSlice::from_slice(&[1, 2, 3]))], "blob_first_field", ), ( vec![Value::Text(Text::new("hello")), Value::Integer(5)], - vec![RefValue::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(TextRef::from_str("hello"))], "equal_text_prefix_but_more_serialized_fields", ), ( vec![Value::Text(Text::new("same")), Value::Integer(5)], vec![ - RefValue::Text(TextRef::from_str("same")), - RefValue::Integer(5), + ValueRef::Text(TextRef::from_str("same")), + ValueRef::Integer(5), ], "equal_text_then_equal_int", ), @@ -3132,9 +3138,9 @@ mod tests { Value::Integer(3), ]); let unpacked = vec![ - RefValue::Integer(1), - RefValue::Integer(99), - RefValue::Integer(3), + ValueRef::Integer(1), + ValueRef::Integer(99), + ValueRef::Integer(3), ]; let tie_breaker = std::cmp::Ordering::Equal; @@ -3160,8 +3166,8 @@ mod tests { let index_info_large = create_index_info(15, vec![SortOrder::Asc; 15], collations_large); let int_values = vec![ - RefValue::Integer(42), - RefValue::Text(TextRef::from_str("hello")), + ValueRef::Integer(42), + ValueRef::Text(TextRef::from_str("hello")), ]; assert!(matches!( find_compare(&int_values, &index_info_small), @@ -3169,21 +3175,21 @@ mod tests { )); let string_values = vec![ - RefValue::Text(TextRef::from_str("hello")), - RefValue::Integer(42), + ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Integer(42), ]; assert!(matches!( find_compare(&string_values, &index_info_small), RecordCompare::String )); - let large_values: Vec = (0..15).map(RefValue::Integer).collect(); + let large_values: Vec = (0..15).map(ValueRef::Integer).collect(); assert!(matches!( find_compare(&large_values, &index_info_large), RecordCompare::Generic )); - let blob_values = vec![RefValue::Blob(RawSlice::from_slice(&[1, 2, 3]))]; + let blob_values = vec![ValueRef::Blob(RawSlice::from_slice(&[1, 2, 3]))]; assert!(matches!( find_compare(&blob_values, &index_info_small), RecordCompare::Generic diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index aa9fe59b4..9a03ca9bc 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -65,7 +65,7 @@ use crate::{ vector::{vector32, vector64, vector_distance_cos, vector_distance_l2, vector_extract}, }; -use crate::{info, turso_assert, OpenFlags, RefValue, Row, TransactionState}; +use crate::{info, turso_assert, OpenFlags, Row, TransactionState, ValueRef}; use super::{ insn::{Cookie, RegisterOrLiteral}, @@ -2705,7 +2705,7 @@ pub fn op_row_id( let record_cursor = record_cursor_ref.deref_mut(); let rowid = record.last_value(record_cursor).unwrap(); match rowid { - Ok(RefValue::Integer(rowid)) => rowid, + Ok(ValueRef::Integer(rowid)) => rowid, _ => unreachable!(), } }; @@ -4275,7 +4275,14 @@ pub fn op_sorter_compare( &record.get_values()[..*num_regs] }; - let cursor = state.get_cursor(*cursor_id); + // Inlined `state.get_cursor` to prevent borrowing conflit with `state.registers` + let cursor = state + .cursors + .get_mut(*cursor_id) + .unwrap_or_else(|| panic!("cursor id {cursor_id} out of bounds")) + .as_mut() + .unwrap_or_else(|| panic!("cursor id {cursor_id} is None")); + let cursor = cursor.as_sorter_mut(); let Some(current_sorter_record) = cursor.record() else { return Err(LimboError::InternalError( @@ -4287,7 +4294,7 @@ pub fn op_sorter_compare( // If the current sorter record has a NULL in any of the significant fields, the comparison is not equal. let is_equal = current_sorter_values .iter() - .all(|v| !matches!(v, RefValue::Null)) + .all(|v| !matches!(v, ValueRef::Null)) && compare_immutable( previous_sorter_values, current_sorter_values, @@ -4953,7 +4960,7 @@ pub fn op_function( } #[cfg(feature = "json")] { - use crate::types::{TextRef, TextSubtype}; + use crate::types::TextSubtype; let table = state.registers[*start_reg].get_value(); let Value::Text(table) = table else { @@ -4978,10 +4985,7 @@ pub fn op_function( for column in table.columns() { let name = column.name.as_ref().unwrap(); let name_json = json::convert_ref_dbtype_to_jsonb( - &RefValue::Text(TextRef::create_from( - name.as_str().as_bytes(), - TextSubtype::Text, - )), + ValueRef::Text(name.as_bytes(), TextSubtype::Text), json::Conv::ToString, )?; json.append_jsonb_to_end(name_json.data()); @@ -5049,13 +5053,13 @@ pub fn op_function( json.append_jsonb_to_end(column_name.data()); let val = record_cursor.get_value(&record, i)?; - if let RefValue::Blob(..) = val { + if let ValueRef::Blob(..) = val { return Err(LimboError::InvalidArgument( "bin_record_json_object: formatting of BLOB values stored in binary record is not supported".to_string() )); } let val_json = - json::convert_ref_dbtype_to_jsonb(&val, json::Conv::NotStrict)?; + json::convert_ref_dbtype_to_jsonb(val, json::Conv::NotStrict)?; json.append_jsonb_to_end(val_json.data()); } json.finalize_unsafe(json::jsonb::ElementType::OBJECT)?; @@ -6249,9 +6253,9 @@ pub fn op_idx_insert( // UNIQUE indexes disallow duplicates like (a=1,b=2,rowid=1) and (a=1,b=2,rowid=2). let existing_key = if cursor.has_rowid() { let count = cursor.record_cursor.borrow_mut().count(record); - record.get_values()[..count.saturating_sub(1)].to_vec() + &record.get_values()[..count.saturating_sub(1)] } else { - record.get_values().to_vec() + &record.get_values()[..] }; let inserted_key_vals = &record_to_insert.get_values(); if existing_key.len() != inserted_key_vals.len() { @@ -6259,7 +6263,7 @@ pub fn op_idx_insert( } let conflict = compare_immutable( - existing_key.as_slice(), + existing_key, inserted_key_vals, &cursor.index_info.as_ref().unwrap().key_info, ) == std::cmp::Ordering::Equal; @@ -6550,7 +6554,7 @@ pub fn op_no_conflict( record .get_values() .iter() - .any(|val| matches!(val, RefValue::Null)) + .any(|val| matches!(val, ValueRef::Null)) } RecordSource::Unpacked { start_reg, diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 90b14225f..16695bd0f 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -32,7 +32,7 @@ use crate::{ state_machine::StateMachine, storage::{pager::PagerCommitResult, sqlite3_ondisk::SmallVec}, translate::{collate::CollationSeq, plan::TableReferences}, - types::{IOCompletions, IOResult, RawSlice, TextRef}, + types::{IOCompletions, IOResult}, vdbe::{ execute::{ OpCheckpointState, OpColumnState, OpDeleteState, OpDeleteSubState, OpDestroyState, @@ -41,7 +41,7 @@ use crate::{ }, metrics::StatementMetrics, }, - RefValue, + ValueRef, }; use crate::{ @@ -1001,22 +1001,10 @@ fn make_record(registers: &[Register], start_reg: &usize, count: &usize) -> Immu ImmutableRecord::from_registers(regs, regs.len()) } -pub fn registers_to_ref_values(registers: &[Register]) -> Vec { +pub fn registers_to_ref_values<'a>(registers: &'a [Register]) -> Vec> { registers .iter() - .map(|reg| { - let value = reg.get_value(); - match value { - Value::Null => RefValue::Null, - Value::Integer(i) => RefValue::Integer(*i), - Value::Float(f) => RefValue::Float(*f), - Value::Text(t) => RefValue::Text(TextRef { - value: RawSlice::new(t.value.as_ptr(), t.value.len()), - subtype: t.subtype, - }), - Value::Blob(b) => RefValue::Blob(RawSlice::new(b.as_ptr(), b.len())), - } - }) + .map(|reg| reg.get_value().as_ref()) .collect() } diff --git a/core/vdbe/sorter.rs b/core/vdbe/sorter.rs index f2ef80c69..bedf6de18 100644 --- a/core/vdbe/sorter.rs +++ b/core/vdbe/sorter.rs @@ -1,6 +1,6 @@ use turso_parser::ast::SortOrder; -use std::cell::RefCell; +use std::cell::{RefCell, UnsafeCell}; use std::cmp::{Eq, Ord, Ordering, PartialEq, PartialOrd, Reverse}; use std::collections::BinaryHeap; use std::rc::Rc; @@ -15,7 +15,7 @@ use crate::{ storage::sqlite3_ondisk::{read_varint, varint_len, write_varint}, translate::collate::CollationSeq, turso_assert, - types::{IOResult, ImmutableRecord, KeyInfo, RecordCursor, RefValue}, + types::{IOResult, ImmutableRecord, KeyInfo, RecordCursor, ValueRef}, Result, }; use crate::{io_yield_many, io_yield_one, return_if_io, CompletionError}; @@ -614,7 +614,8 @@ impl SortedChunk { struct SortableImmutableRecord { record: ImmutableRecord, cursor: RecordCursor, - key_values: RefCell>, + // SAFETY: borrows from 'self + key_values: UnsafeCell>>, key_len: usize, index_key_info: Rc>, /// The key deserialization error, if any. @@ -636,29 +637,34 @@ impl SortableImmutableRecord { Ok(Self { record, cursor, - key_values: RefCell::new(Vec::with_capacity(key_len)), + key_values: UnsafeCell::new(Vec::with_capacity(key_len)), index_key_info, deserialization_error: RefCell::new(None), key_len, }) } - /// Attempts to deserialize the key value at the given index. - /// If the key value has already been deserialized, this does nothing. - /// The deserialized key value is stored in the `key_values` field. - /// In case of an error, the error is stored in the `deserialization_error` field. - fn try_deserialize_key(&self, idx: usize) { - let mut key_values = self.key_values.borrow_mut(); - if idx < key_values.len() { - // The key value with this index has already been deserialized. - return; - } - match self.cursor.deserialize_column(&self.record, idx) { - Ok(value) => key_values.push(value), - Err(error) => { - self.deserialization_error.replace(Some(error)); - } + fn key_value<'a>(&'a self, i: usize) -> Option> { + // SAFETY: there are no other active references + let key_values = unsafe { &mut *self.key_values.get() }; + + if i >= key_values.len() { + assert_eq!(key_values.len(), i, "access must be sequential"); + + let value = match self.cursor.deserialize_column(&self.record, i) { + Ok(value) => value, + Err(err) => { + self.deserialization_error.replace(Some(err)); + return None; + } + }; + + // SAFETY: no 'static lifetime is exposed, all references are bound to 'self + let value: ValueRef<'static> = unsafe { std::mem::transmute(value) }; + key_values.push(value); } + + Some(key_values[i]) } } @@ -674,34 +680,25 @@ impl Ord for SortableImmutableRecord { self.cursor.serial_types.len(), other.cursor.serial_types.len() ); - let this_key_values_len = self.key_values.borrow().len(); - let other_key_values_len = other.key_values.borrow().len(); for i in 0..self.key_len { - // Lazily deserialize the key values if they haven't been deserialized already. - if i >= this_key_values_len { - self.try_deserialize_key(i); - if self.deserialization_error.borrow().is_some() { - return Ordering::Equal; - } - } - if i >= other_key_values_len { - other.try_deserialize_key(i); - if other.deserialization_error.borrow().is_some() { - return Ordering::Equal; - } - } + let Some(this_key_value) = self.key_value(i) else { + return Ordering::Equal; + }; + + let Some(other_key_value) = other.key_value(i) else { + return Ordering::Equal; + }; - let this_key_value = &self.key_values.borrow()[i]; - let other_key_value = &other.key_values.borrow()[i]; let column_order = self.index_key_info[i].sort_order; let collation = self.index_key_info[i].collation; let cmp = match (this_key_value, other_key_value) { - (RefValue::Text(left), RefValue::Text(right)) => { - collation.compare_strings(left.as_str(), right.as_str()) - } - _ => this_key_value.partial_cmp(other_key_value).unwrap(), + (ValueRef::Text(left, _), ValueRef::Text(right, _)) => collation.compare_strings( + &String::from_utf8_lossy(left), + &String::from_utf8_lossy(right), + ), + _ => this_key_value.partial_cmp(&other_key_value).unwrap(), }; if !cmp.is_eq() { return match column_order { @@ -742,7 +739,7 @@ enum SortedChunkIOState { mod tests { use super::*; use crate::translate::collate::CollationSeq; - use crate::types::{ImmutableRecord, RefValue, Value, ValueType}; + use crate::types::{ImmutableRecord, Value, ValueRef, ValueType}; use crate::util::IOExt; use crate::PlatformIO; use rand_chacha::{ @@ -806,7 +803,7 @@ mod tests { for i in 0..num_records { assert!(sorter.has_more()); let record = sorter.record().unwrap(); - assert_eq!(record.get_values()[0], RefValue::Integer(i)); + assert_eq!(record.get_values()[0], ValueRef::Integer(i)); // Check that the record remained unchanged after sorting. assert_eq!(record, &initial_records[(num_records - i - 1) as usize]); From cf53ecb7e345a996bbc174d6fb34b9c606daff98 Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Sun, 5 Oct 2025 23:43:12 -0300 Subject: [PATCH 2/2] refactor: remove `TextRef` and `RawSlice` and fix tests --- core/mvcc/database/tests.rs | 8 +- core/mvcc/persistent_storage/logical_log.rs | 15 +- core/storage/btree.rs | 16 +- core/types.rs | 157 ++++---------------- 4 files changed, 51 insertions(+), 145 deletions(-) diff --git a/core/mvcc/database/tests.rs b/core/mvcc/database/tests.rs index 2c8d23106..3cc76db23 100644 --- a/core/mvcc/database/tests.rs +++ b/core/mvcc/database/tests.rs @@ -978,8 +978,8 @@ fn test_cursor_modification_during_scan() { record.start_serialization(&row.data); let value = record.get_value(0).unwrap(); match value { - ValueRef::Text(text) => { - assert_eq!(text.as_str(), "new_row"); + ValueRef::Text(text, _) => { + assert_eq!(text, b"new_row"); } _ => panic!("Expected Text value"), } @@ -1210,8 +1210,8 @@ fn test_restart() { .unwrap(); let record = get_record_value(&row); match record.get_value(0).unwrap() { - ValueRef::Text(text) => { - assert_eq!(text.as_str(), "bar"); + ValueRef::Text(text, _) => { + assert_eq!(text, b"bar"); } _ => panic!("Expected Text value"), } diff --git a/core/mvcc/persistent_storage/logical_log.rs b/core/mvcc/persistent_storage/logical_log.rs index 52a01bb27..a340d6c43 100644 --- a/core/mvcc/persistent_storage/logical_log.rs +++ b/core/mvcc/persistent_storage/logical_log.rs @@ -565,10 +565,10 @@ mod tests { let record = ImmutableRecord::from_bin_record(row.data.clone()); let values = record.get_values(); let foo = values.first().unwrap(); - let ValueRef::Text(foo) = foo else { + let ValueRef::Text(foo, _) = foo else { unreachable!() }; - assert_eq!(foo.as_str(), "foo"); + assert_eq!(foo, b"foo"); } #[test] @@ -637,10 +637,10 @@ mod tests { let record = ImmutableRecord::from_bin_record(row.data.clone()); let values = record.get_values(); let foo = values.first().unwrap(); - let ValueRef::Text(foo) = foo else { + let ValueRef::Text(foo, _) = foo else { unreachable!() }; - assert_eq!(foo.as_str(), value.as_str()); + assert_eq!(*foo, value.as_bytes()); } } @@ -758,11 +758,14 @@ mod tests { let record = ImmutableRecord::from_bin_record(row.data.clone()); let values = record.get_values(); let foo = values.first().unwrap(); - let ValueRef::Text(foo) = foo else { + let ValueRef::Text(foo, _) = foo else { unreachable!() }; - assert_eq!(foo.as_str(), format!("row_{}", present_rowid.row_id as u64)); + assert_eq!( + String::from_utf8_lossy(foo), + format!("row_{}", present_rowid.row_id as u64) + ); } // Check rowids that were deleted diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 7935d3595..80b831231 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -8690,7 +8690,11 @@ mod tests { run_until_done(|| cursor.next(), pager.deref()).unwrap(); let record = run_until_done(|| cursor.record(), &pager).unwrap(); let record = record.as_ref().unwrap(); - let cur = record.get_values().clone(); + let cur = record + .get_values() + .iter() + .map(ValueRef::to_owned) + .collect::>(); if let Some(prev) = prev { if prev >= cur { println!("Seed: {seed}"); @@ -8933,11 +8937,7 @@ mod tests { let ValueRef::Blob(ref cur) = cur else { panic!("expected blob, got {cur:?}"); }; - assert_eq!( - cur.to_slice(), - key, - "key {key:?} is not found, seed: {seed}" - ); + assert_eq!(cur, key, "key {key:?} is not found, seed: {seed}"); } pager.end_read_tx(); } @@ -9469,8 +9469,8 @@ mod tests { let exists = run_until_done(|| cursor.next(), &pager)?; assert!(exists, "Record {i} not found"); - let record = run_until_done(|| cursor.record(), &pager)?; - let value = record.unwrap().get_value(0)?; + let record = run_until_done(|| cursor.record(), &pager)?.unwrap(); + let value = record.get_value(0)?; assert_eq!( value, ValueRef::Integer(i), diff --git a/core/types.rs b/core/types.rs index b86c72726..6de047f9b 100644 --- a/core/types.rs +++ b/core/types.rs @@ -68,12 +68,6 @@ impl Display for Text { } } -#[derive(Debug, Clone, PartialEq)] -pub struct TextRef { - pub value: RawSlice, - pub subtype: TextSubtype, -} - impl Text { pub fn new(value: &str) -> Self { Self { @@ -119,24 +113,12 @@ pub trait AnyText: AsRef { fn subtype(&self) -> TextSubtype; } -impl AsRef for TextRef { - fn as_ref(&self) -> &str { - self.as_str() - } -} - impl AnyText for Text { fn subtype(&self) -> TextSubtype { self.subtype } } -impl AnyText for TextRef { - fn subtype(&self) -> TextSubtype { - self.subtype - } -} - impl AnyText for &str { fn subtype(&self) -> TextSubtype { TextSubtype::Text @@ -147,12 +129,6 @@ pub trait AnyBlob { fn as_slice(&self) -> &[u8]; } -impl AnyBlob for RawSlice { - fn as_slice(&self) -> &[u8] { - self.to_slice() - } -} - impl AnyBlob for Vec { fn as_slice(&self) -> &[u8] { self.as_slice() @@ -195,22 +171,6 @@ impl From for String { } } -impl Display for TextRef { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.as_str()) - } -} - -impl TextRef { - pub fn create_from(value: &[u8], subtype: TextSubtype) -> Self { - let value = RawSlice::create_from(value); - Self { value, subtype } - } - pub fn as_str(&self) -> &str { - unsafe { std::str::from_utf8_unchecked(self.value.to_slice()) } - } -} - #[cfg(feature = "serde")] fn float_to_string(float: &f64, serializer: S) -> Result where @@ -249,12 +209,6 @@ pub enum Value { Blob(Vec), } -#[derive(Debug, Clone, PartialEq)] -pub struct RawSlice { - data: *const u8, - len: usize, -} - #[derive(PartialEq, Clone, Copy)] pub enum ValueRef<'a> { Null, @@ -2559,27 +2513,6 @@ pub enum SeekKey<'a> { IndexKey(&'a ImmutableRecord), } -impl RawSlice { - pub fn create_from(value: &[u8]) -> Self { - if value.is_empty() { - RawSlice::new(std::ptr::null(), 0) - } else { - let ptr = &value[0] as *const u8; - RawSlice::new(ptr, value.len()) - } - } - pub fn new(data: *const u8, len: usize) -> Self { - Self { data, len } - } - pub fn to_slice(&self) -> &[u8] { - if self.data.is_null() { - &[] - } else { - unsafe { std::slice::from_raw_parts(self.data, self.len) } - } - } -} - #[derive(Debug)] pub enum DatabaseChangeType { Delete, @@ -2641,9 +2574,10 @@ mod tests { let collation = index_key_info[i].collation; let cmp = match (&l[i], &r[i]) { - (ValueRef::Text(left), ValueRef::Text(right)) => { - collation.compare_strings(left.as_str(), right.as_str()) - } + (ValueRef::Text(left, _), ValueRef::Text(right, _)) => collation.compare_strings( + &String::from_utf8_lossy(left), + &String::from_utf8_lossy(right), + ), _ => l[i].partial_cmp(&r[i]).unwrap_or(std::cmp::Ordering::Equal), }; @@ -2682,37 +2616,6 @@ mod tests { } } - fn value_to_ref_value(value: &Value) -> ValueRef { - match value { - Value::Null => ValueRef::Null, - Value::Integer(i) => ValueRef::Integer(*i), - Value::Float(f) => ValueRef::Float(*f), - Value::Text(text) => ValueRef::Text(TextRef { - value: RawSlice::from_slice(&text.value), - subtype: text.subtype, - }), - Value::Blob(blob) => ValueRef::Blob(RawSlice::from_slice(blob)), - } - } - - impl TextRef { - fn from_str(s: &str) -> Self { - TextRef { - value: RawSlice::from_slice(s.as_bytes()), - subtype: crate::types::TextSubtype::Text, - } - } - } - - impl RawSlice { - fn from_slice(data: &[u8]) -> Self { - Self { - data: data.as_ptr(), - len: data.len(), - } - } - } - fn assert_compare_matches_full_comparison( serialized_values: Vec, unpacked_values: Vec, @@ -2722,7 +2625,7 @@ mod tests { let serialized = create_record(serialized_values.clone()); let serialized_ref_values: Vec = - serialized_values.iter().map(value_to_ref_value).collect(); + serialized_values.iter().map(Value::as_ref).collect(); let tie_breaker = std::cmp::Ordering::Equal; @@ -2858,7 +2761,7 @@ mod tests { vec![Value::Integer(42), Value::Text(Text::new("hello"))], vec![ ValueRef::Integer(42), - ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Text(b"hello", TextSubtype::Text), ], "integer_text_equal", ), @@ -2866,7 +2769,7 @@ mod tests { vec![Value::Integer(42), Value::Text(Text::new("hello"))], vec![ ValueRef::Integer(42), - ValueRef::Text(TextRef::from_str("world")), + ValueRef::Text(b"world", TextSubtype::Text), ], "integer_equal_text_different", ), @@ -2893,34 +2796,34 @@ mod tests { let test_cases = vec![ ( vec![Value::Text(Text::new("hello"))], - vec![ValueRef::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(b"hello", TextSubtype::Text)], "equal_strings", ), ( vec![Value::Text(Text::new("abc"))], - vec![ValueRef::Text(TextRef::from_str("def"))], + vec![ValueRef::Text(b"def", TextSubtype::Text)], "less_than_strings", ), ( vec![Value::Text(Text::new("xyz"))], - vec![ValueRef::Text(TextRef::from_str("abc"))], + vec![ValueRef::Text(b"abc", TextSubtype::Text)], "greater_than_strings", ), ( vec![Value::Text(Text::new(""))], - vec![ValueRef::Text(TextRef::from_str(""))], + vec![ValueRef::Text(b"", TextSubtype::Text)], "empty_strings", ), ( vec![Value::Text(Text::new("a"))], - vec![ValueRef::Text(TextRef::from_str("aa"))], + vec![ValueRef::Text(b"aa", TextSubtype::Text)], "prefix_strings", ), // Multi-field with string first ( vec![Value::Text(Text::new("hello")), Value::Integer(42)], vec![ - ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Text(b"hello", TextSubtype::Text), ValueRef::Integer(42), ], "string_integer_equal", @@ -2928,7 +2831,7 @@ mod tests { ( vec![Value::Text(Text::new("hello")), Value::Integer(42)], vec![ - ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Text(b"hello", TextSubtype::Text), ValueRef::Integer(99), ], "string_equal_integer_different", @@ -2964,39 +2867,39 @@ mod tests { ), ( vec![Value::Null], - vec![ValueRef::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(b"hello", TextSubtype::Text)], "null_vs_text", ), ( vec![Value::Null], - vec![ValueRef::Blob(RawSlice::from_slice(b"blob"))], + vec![ValueRef::Blob(b"blob")], "null_vs_blob", ), // Numbers vs Text/Blob ( vec![Value::Integer(42)], - vec![ValueRef::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(b"hello", TextSubtype::Text)], "integer_vs_text", ), ( vec![Value::Float(64.4)], - vec![ValueRef::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(b"hello", TextSubtype::Text)], "float_vs_text", ), ( vec![Value::Integer(42)], - vec![ValueRef::Blob(RawSlice::from_slice(b"blob"))], + vec![ValueRef::Blob(b"blob")], "integer_vs_blob", ), ( vec![Value::Float(64.4)], - vec![ValueRef::Blob(RawSlice::from_slice(b"blob"))], + vec![ValueRef::Blob(b"blob")], "float_vs_blob", ), // Text vs Blob ( vec![Value::Text(Text::new("hello"))], - vec![ValueRef::Blob(RawSlice::from_slice(b"blob"))], + vec![ValueRef::Blob(b"blob")], "text_vs_blob", ), // Integer vs Float (affinity conversion) @@ -3044,7 +2947,7 @@ mod tests { ), ( vec![Value::Text(Text::new("abc"))], - vec![ValueRef::Text(TextRef::from_str("def"))], + vec![ValueRef::Text(b"def", TextSubtype::Text)], "desc_string_reversed", ), // Mixed sort orders @@ -3052,7 +2955,7 @@ mod tests { vec![Value::Integer(10), Value::Text(Text::new("hello"))], vec![ ValueRef::Integer(20), - ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Text(b"hello", TextSubtype::Text), ], "desc_first_asc_second", ), @@ -3078,7 +2981,7 @@ mod tests { vec![Value::Integer(42)], vec![ ValueRef::Integer(42), - ValueRef::Text(TextRef::from_str("extra")), + ValueRef::Text(b"extra", TextSubtype::Text), ], "fewer_serialized_fields", ), @@ -3096,18 +2999,18 @@ mod tests { ), ( vec![Value::Blob(vec![1, 2, 3])], - vec![ValueRef::Blob(RawSlice::from_slice(&[1, 2, 3]))], + vec![ValueRef::Blob(&[1, 2, 3])], "blob_first_field", ), ( vec![Value::Text(Text::new("hello")), Value::Integer(5)], - vec![ValueRef::Text(TextRef::from_str("hello"))], + vec![ValueRef::Text(b"hello", TextSubtype::Text)], "equal_text_prefix_but_more_serialized_fields", ), ( vec![Value::Text(Text::new("same")), Value::Integer(5)], vec![ - ValueRef::Text(TextRef::from_str("same")), + ValueRef::Text(b"same", TextSubtype::Text), ValueRef::Integer(5), ], "equal_text_then_equal_int", @@ -3167,7 +3070,7 @@ mod tests { let int_values = vec![ ValueRef::Integer(42), - ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Text(b"hello", TextSubtype::Text), ]; assert!(matches!( find_compare(&int_values, &index_info_small), @@ -3175,7 +3078,7 @@ mod tests { )); let string_values = vec![ - ValueRef::Text(TextRef::from_str("hello")), + ValueRef::Text(b"hello", TextSubtype::Text), ValueRef::Integer(42), ]; assert!(matches!( @@ -3189,7 +3092,7 @@ mod tests { RecordCompare::Generic )); - let blob_values = vec![ValueRef::Blob(RawSlice::from_slice(&[1, 2, 3]))]; + let blob_values = vec![ValueRef::Blob(&[1, 2, 3])]; assert!(matches!( find_compare(&blob_values, &index_info_small), RecordCompare::Generic