diff --git a/bindings/java/rs_src/limbo_connection.rs b/bindings/java/rs_src/limbo_connection.rs index 02b2a0eb8..46431ed2b 100644 --- a/bindings/java/rs_src/limbo_connection.rs +++ b/bindings/java/rs_src/limbo_connection.rs @@ -21,6 +21,7 @@ impl LimboConnection { LimboConnection { conn, io } } + #[allow(clippy::wrong_self_convention)] pub fn to_ptr(self) -> jlong { Box::into_raw(Box::new(self)) as jlong } diff --git a/bindings/java/rs_src/limbo_db.rs b/bindings/java/rs_src/limbo_db.rs index 189ed090b..2136a237f 100644 --- a/bindings/java/rs_src/limbo_db.rs +++ b/bindings/java/rs_src/limbo_db.rs @@ -17,6 +17,7 @@ impl LimboDB { LimboDB { db, io } } + #[allow(clippy::wrong_self_convention)] pub fn to_ptr(self) -> jlong { Box::into_raw(Box::new(self)) as jlong } diff --git a/bindings/java/rs_src/limbo_statement.rs b/bindings/java/rs_src/limbo_statement.rs index f86fd1fc0..12f9e583d 100644 --- a/bindings/java/rs_src/limbo_statement.rs +++ b/bindings/java/rs_src/limbo_statement.rs @@ -26,6 +26,7 @@ impl LimboStatement { LimboStatement { stmt, connection } } + #[allow(clippy::wrong_self_convention)] pub fn to_ptr(self) -> jlong { Box::into_raw(Box::new(self)) as jlong } @@ -66,7 +67,7 @@ pub extern "system" fn Java_tech_turso_core_LimboStatement_step<'local>( match step_result { StepResult::Row => { let row = stmt.stmt.row().unwrap(); - return match row_to_obj_array(&mut env, &row) { + return match row_to_obj_array(&mut env, row) { Ok(row) => to_limbo_step_result(&mut env, STEP_RESULT_ID_ROW, Some(row)), Err(e) => { set_err_msg_and_throw_exception(&mut env, obj, LIMBO_ETC, e.to_string()); @@ -114,7 +115,7 @@ fn row_to_obj_array<'local>( env.new_object("java/lang/Double", "(D)V", &[JValue::Double(*f)])? } limbo_core::Value::Text(s) => env.new_string(s.as_str())?.into(), - limbo_core::Value::Blob(b) => env.byte_array_from_slice(&b.as_slice())?.into(), + limbo_core::Value::Blob(b) => env.byte_array_from_slice(b.as_slice())?.into(), }; if let Err(e) = env.set_object_array_element(&obj_array, i as i32, obj) { eprintln!("Error on parsing row: {:?}", e); diff --git a/bindings/javascript/src/lib.rs b/bindings/javascript/src/lib.rs index ffa9f966e..8dda27dd0 100644 --- a/bindings/javascript/src/lib.rs +++ b/bindings/javascript/src/lib.rs @@ -6,7 +6,6 @@ use std::num::NonZeroUsize; use std::rc::Rc; use std::sync::Arc; -use limbo_core::types::Text; use limbo_core::{maybe_init_database_file, LimboError, StepResult}; use napi::iterator::Generator; use napi::{bindgen_prelude::ObjectFinalize, Env, JsUnknown}; @@ -529,9 +528,7 @@ fn from_js_value(value: JsUnknown) -> napi::Result { } napi::ValueType::String => { let s = value.coerce_to_string()?; - Ok(limbo_core::Value::Text(Text::from_str( - s.into_utf8()?.as_str()?, - ))) + Ok(limbo_core::Value::Text(s.into_utf8()?.as_str()?.into())) } napi::ValueType::Symbol | napi::ValueType::Object diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs index 5cf3a9f60..234ff7219 100644 --- a/bindings/python/src/lib.rs +++ b/bindings/python/src/lib.rs @@ -1,6 +1,5 @@ use anyhow::Result; use errors::*; -use limbo_core::types::Text; use limbo_core::Value; use pyo3::prelude::*; use pyo3::types::{PyBytes, PyList, PyTuple}; @@ -96,17 +95,15 @@ impl Cursor { // For DDL and DML statements, // we need to execute the statement immediately if stmt_is_ddl || stmt_is_dml { - loop { - match stmt.borrow_mut().step().map_err(|e| { - PyErr::new::(format!("Step error: {:?}", e)) - })? { - limbo_core::StepResult::IO => { - self.conn.io.run_once().map_err(|e| { - PyErr::new::(format!("IO error: {:?}", e)) - })?; - } - _ => break, - } + while let limbo_core::StepResult::IO = stmt + .borrow_mut() + .step() + .map_err(|e| PyErr::new::(format!("Step error: {:?}", e)))? + { + self.conn + .io + .run_once() + .map_err(|e| PyErr::new::(format!("IO error: {:?}", e)))?; } } @@ -130,7 +127,7 @@ impl Cursor { })? { limbo_core::StepResult::Row => { let row = stmt.row().unwrap(); - let py_row = row_to_py(py, &row)?; + let py_row = row_to_py(py, row)?; return Ok(Some(py_row)); } limbo_core::StepResult::IO => { @@ -166,7 +163,7 @@ impl Cursor { })? { limbo_core::StepResult::Row => { let row = stmt.row().unwrap(); - let py_row = row_to_py(py, &row)?; + let py_row = row_to_py(py, row)?; results.push(py_row); } limbo_core::StepResult::IO => { @@ -342,13 +339,13 @@ fn row_to_py(py: Python, row: &limbo_core::Row) -> Result { /// Converts a Python object to a Limbo Value fn py_to_owned_value(obj: &Bound) -> Result { if obj.is_none() { - return Ok(Value::Null); + Ok(Value::Null) } else if let Ok(integer) = obj.extract::() { return Ok(Value::Integer(integer)); } else if let Ok(float) = obj.extract::() { return Ok(Value::Float(float)); } else if let Ok(string) = obj.extract::() { - return Ok(Value::Text(Text::from_str(string))); + return Ok(Value::Text(string.into())); } else if let Ok(bytes) = obj.downcast::() { return Ok(Value::Blob(bytes.as_bytes().to_vec())); } else { diff --git a/bindings/rust/src/lib.rs b/bindings/rust/src/lib.rs index d7925a0e3..91a1ab7d2 100644 --- a/bindings/rust/src/lib.rs +++ b/bindings/rust/src/lib.rs @@ -445,14 +445,14 @@ mod tests { .query("SELECT data FROM test_large_persistence ORDER BY id;", ()) .await?; - for i in 0..NUM_INSERTS { + for (i, value) in original_data.iter().enumerate().take(NUM_INSERTS) { let row = rows .next() .await? .unwrap_or_else(|| panic!("Expected row {} but found None", i)); assert_eq!( row.get_value(0)?, - Value::Text(original_data[i].clone()), + Value::Text(value.clone()), "Mismatch in retrieved data for row {}", i ); diff --git a/bindings/rust/src/value.rs b/bindings/rust/src/value.rs index de2fae99e..64dc31ce8 100644 --- a/bindings/rust/src/value.rs +++ b/bindings/rust/src/value.rs @@ -110,9 +110,9 @@ impl Value { } } -impl Into for Value { - fn into(self) -> limbo_core::Value { - match self { +impl From for limbo_core::Value { + fn from(val: Value) -> Self { + match val { Value::Null => limbo_core::Value::Null, Value::Integer(n) => limbo_core::Value::Integer(n), Value::Real(n) => limbo_core::Value::Float(n), diff --git a/bindings/wasm/lib.rs b/bindings/wasm/lib.rs index bbb56b9c7..8f65a3d01 100644 --- a/bindings/wasm/lib.rs +++ b/bindings/wasm/lib.rs @@ -1,8 +1,12 @@ +#[cfg(all(feature = "web", feature = "nodejs"))] +compile_error!("Features 'web' and 'nodejs' cannot be enabled at the same time"); + use js_sys::{Array, Object}; use limbo_core::{maybe_init_database_file, Clock, Instant, OpenFlags, Result}; use std::cell::RefCell; use std::sync::Arc; use wasm_bindgen::prelude::*; + #[allow(dead_code)] #[wasm_bindgen] pub struct Database { @@ -48,6 +52,7 @@ impl RowIterator { } #[wasm_bindgen] + #[allow(clippy::should_implement_trait)] pub fn next(&mut self) -> JsValue { let mut stmt = self.inner.borrow_mut(); match stmt.step() { @@ -364,10 +369,7 @@ impl limbo_core::DatabaseStorage for DatabaseFile { } } -#[cfg(all(feature = "web", feature = "nodejs"))] -compile_error!("Features 'web' and 'nodejs' cannot be enabled at the same time"); - -#[cfg(feature = "web")] +#[cfg(all(feature = "web", not(feature = "nodejs")))] #[wasm_bindgen(module = "/web/src/web-vfs.js")] extern "C" { type VFS; @@ -393,7 +395,7 @@ extern "C" { fn sync(this: &VFS, fd: i32); } -#[cfg(feature = "nodejs")] +#[cfg(all(feature = "nodejs", not(feature = "web")))] #[wasm_bindgen(module = "/node/src/vfs.cjs")] extern "C" { type VFS; diff --git a/cli/app.rs b/cli/app.rs index c2ea8b70d..b2e6ba388 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -509,8 +509,8 @@ impl Limbo { if line.is_empty() { return Ok(()); } - if line.starts_with('.') { - self.handle_dot_command(&line[1..]); + if let Some(command) = line.strip_prefix('.') { + self.handle_dot_command(command); let _ = self.reset_line(line); return Ok(()); } @@ -747,7 +747,7 @@ impl Limbo { let name = rows.get_column_name(i); Cell::new(name) .add_attribute(Attribute::Bold) - .fg(config.table.header_color.into_comfy_table_color()) + .fg(config.table.header_color.as_comfy_table_color()) }) .collect::>(); table.set_header(header); @@ -785,7 +785,7 @@ impl Limbo { .set_alignment(alignment) .fg(config.table.column_colors [idx % config.table.column_colors.len()] - .into_comfy_table_color()), + .as_comfy_table_color()), ); } table.add_row(row); @@ -1060,10 +1060,9 @@ impl Limbo { Ok(rl.readline(&self.prompt)?) } else { let mut input = String::new(); - println!(""); let mut reader = std::io::stdin().lock(); if reader.read_line(&mut input)? == 0 { - return Err(ReadlineError::Eof.into()); + return Err(ReadlineError::Eof); } // Remove trailing newline if input.ends_with('\n') { diff --git a/cli/config/palette.rs b/cli/config/palette.rs index 3b6962270..62a171a40 100644 --- a/cli/config/palette.rs +++ b/cli/config/palette.rs @@ -174,7 +174,7 @@ impl<'de> Deserialize<'de> for LimboColor { { struct LimboColorVisitor; - impl<'de> Visitor<'de> for LimboColorVisitor { + impl Visitor<'_> for LimboColorVisitor { type Value = LimboColor; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { @@ -228,7 +228,7 @@ impl Validate for LimboColor { } impl LimboColor { - pub fn into_comfy_table_color(&self) -> comfy_table::Color { + pub fn as_comfy_table_color(&self) -> comfy_table::Color { match self.0 { Color::Black => comfy_table::Color::Black, Color::Red => comfy_table::Color::Red, @@ -247,7 +247,7 @@ impl LimboColor { Color::Fixed(7) => comfy_table::Color::Grey, Color::Fixed(8) => comfy_table::Color::DarkGrey, Color::DarkGray => comfy_table::Color::AnsiValue(241), - Color::LightRed => comfy_table::Color::AnsiValue(09), + Color::LightRed => comfy_table::Color::AnsiValue(9), Color::LightGreen => comfy_table::Color::AnsiValue(10), Color::LightYellow => comfy_table::Color::AnsiValue(11), Color::LightBlue => comfy_table::Color::AnsiValue(12), diff --git a/cli/helper.rs b/cli/helper.rs index 141e8f8d5..c11e5b535 100644 --- a/cli/helper.rs +++ b/cli/helper.rs @@ -154,7 +154,7 @@ impl SqlCompleter { conn, io, cmd: C::command().into(), - _cmd_phantom: PhantomData::default(), + _cmd_phantom: PhantomData, } } @@ -165,7 +165,7 @@ impl SqlCompleter { ) -> rustyline::Result<(usize, Vec)> { // TODO maybe check to see if the line is empty and then just output the command names line = &line[1..]; - pos = pos - 1; + pos -= 1; let (prefix_pos, _) = extract_word(line, pos, ESCAPE_CHAR, default_break_chars); diff --git a/core/benches/tpc_h_benchmark.rs b/core/benches/tpc_h_benchmark.rs index fe29f8ced..39bb048e1 100644 --- a/core/benches/tpc_h_benchmark.rs +++ b/core/benches/tpc_h_benchmark.rs @@ -4,7 +4,7 @@ use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criteri use limbo_core::{Database, PlatformIO, IO as _}; use pprof::criterion::{Output, PProfProfiler}; -const TPC_H_PATH: &'static str = "../perf/tpc-h/TPC-H.db"; +const TPC_H_PATH: &str = "../perf/tpc-h/TPC-H.db"; macro_rules! tpc_query { ($num:literal) => { diff --git a/core/ext/vtab_xconnect.rs b/core/ext/vtab_xconnect.rs index 07a7596e1..06c548104 100644 --- a/core/ext/vtab_xconnect.rs +++ b/core/ext/vtab_xconnect.rs @@ -61,7 +61,7 @@ pub unsafe extern "C" fn execute( return ResultCode::Error; } Ok(StepResult::Done) => { - *last_insert_rowid = conn.last_insert_rowid() as i64; + *last_insert_rowid = conn.last_insert_rowid(); return ResultCode::OK; } Ok(StepResult::IO) => { diff --git a/core/fast_lock.rs b/core/fast_lock.rs index 55d766222..96a823699 100644 --- a/core/fast_lock.rs +++ b/core/fast_lock.rs @@ -14,7 +14,7 @@ pub struct SpinLockGuard<'a, T> { lock: &'a SpinLock, } -impl<'a, T> Drop for SpinLockGuard<'a, T> { +impl Drop for SpinLockGuard<'_, T> { fn drop(&mut self) { self.lock.locked.store(false, Ordering::Release); } diff --git a/core/functions/datetime.rs b/core/functions/datetime.rs index 4edd3b8dd..2792fdcdd 100644 --- a/core/functions/datetime.rs +++ b/core/functions/datetime.rs @@ -664,11 +664,7 @@ pub fn exec_timediff(values: &[Register]) -> Value { fn format_time_duration(duration: &chrono::Duration) -> Value { let is_negative = duration.num_seconds() < 0; - let abs_duration = if is_negative { - -duration.clone() - } else { - duration.clone() - }; + let abs_duration = if is_negative { -*duration } else { *duration }; let total_seconds = abs_duration.num_seconds(); let hours = (total_seconds % 86400) / 3600; @@ -695,7 +691,7 @@ fn format_time_duration(duration: &chrono::Duration) -> Value { millis ); - Value::build_text(&result) + Value::build_text(result) } #[cfg(test)] @@ -839,7 +835,7 @@ mod tests { Value::Float(f64::NAN), // NaN Value::Float(f64::INFINITY), // Infinity Value::Null, // Null value - Value::Blob(vec![1, 2, 3].into()), // Blob (unsupported type) + Value::Blob(vec![1, 2, 3]), // Blob (unsupported type) // Invalid timezone tests Value::build_text("2024-07-21T12:00:00+24:00"), // Invalid timezone offset (too large) Value::build_text("2024-07-21T12:00:00-24:00"), // Invalid timezone offset (too small) @@ -974,7 +970,7 @@ mod tests { Value::Float(f64::NAN), // NaN Value::Float(f64::INFINITY), // Infinity Value::Null, // Null value - Value::Blob(vec![1, 2, 3].into()), // Blob (unsupported type) + Value::Blob(vec![1, 2, 3]), // Blob (unsupported type) // Invalid timezone tests Value::build_text("2024-07-21T12:00:00+24:00"), // Invalid timezone offset (too large) Value::build_text("2024-07-21T12:00:00-24:00"), // Invalid timezone offset (too small) diff --git a/core/functions/printf.rs b/core/functions/printf.rs index 836b1c734..5924ca561 100644 --- a/core/functions/printf.rs +++ b/core/functions/printf.rs @@ -36,7 +36,7 @@ pub fn exec_printf(values: &[Register]) -> crate::Result { match value { Value::Integer(_) => result.push_str(&format!("{}", value)), Value::Float(_) => result.push_str(&format!("{}", value)), - _ => result.push_str("0".into()), + _ => result.push('0'), } args_index += 1; } @@ -59,7 +59,7 @@ pub fn exec_printf(values: &[Register]) -> crate::Result { match value { Value::Float(f) => result.push_str(&format!("{:.6}", f)), Value::Integer(i) => result.push_str(&format!("{:.6}", *i as f64)), - _ => result.push_str("0.0".into()), + _ => result.push_str("0.0"), } args_index += 1; } @@ -75,7 +75,7 @@ pub fn exec_printf(values: &[Register]) -> crate::Result { } } } - Ok(Value::build_text(&result)) + Ok(Value::build_text(result)) } #[cfg(test)] diff --git a/core/json/cache.rs b/core/json/cache.rs index 5f0408e0c..271049944 100644 --- a/core/json/cache.rs +++ b/core/json/cache.rs @@ -90,7 +90,7 @@ impl JsonCacheCell { #[cfg(test)] pub fn lookup(&self, key: &Value) -> Option { - assert_eq!(self.accessed.get(), false); + assert!(!self.accessed.get()); self.accessed.set(true); @@ -116,7 +116,7 @@ impl JsonCacheCell { key: &Value, value: impl Fn(&Value) -> crate::Result, ) -> crate::Result { - assert_eq!(self.accessed.get(), false); + assert!(!self.accessed.get()); self.accessed.set(true); let result = unsafe { @@ -139,8 +139,7 @@ impl JsonCacheCell { } } } else { - let result = value(key); - result + value(key) } }; self.accessed.set(false); @@ -149,7 +148,7 @@ impl JsonCacheCell { } pub fn clear(&mut self) { - assert_eq!(self.accessed.get(), false); + assert!(!self.accessed.get()); self.accessed.set(true); unsafe { let cache_ptr = self.inner.get(); @@ -325,7 +324,7 @@ mod tests { let cache_cell = JsonCacheCell::new(); // Access flag should be false initially - assert_eq!(cache_cell.accessed.get(), false); + assert!(!cache_cell.accessed.get()); // Inner cache should be None initially unsafe { @@ -350,7 +349,7 @@ mod tests { } // Access flag should be reset to false - assert_eq!(cache_cell.accessed.get(), false); + assert!(!cache_cell.accessed.get()); // Insert the value using get_or_insert_with let insert_result = cache_cell.get_or_insert_with(&key, |k| { @@ -363,7 +362,7 @@ mod tests { assert_eq!(insert_result.unwrap(), value); // Access flag should be reset to false - assert_eq!(cache_cell.accessed.get(), false); + assert!(!cache_cell.accessed.get()); // Lookup should now return the value let lookup_result = cache_cell.lookup(&key); @@ -426,7 +425,7 @@ mod tests { assert!(error_result.is_err()); // Access flag should be reset to false - assert_eq!(cache_cell.accessed.get(), false); + assert!(!cache_cell.accessed.get()); // The entry should not be cached let lookup_result = cache_cell.lookup(&key); diff --git a/core/json/jsonb.rs b/core/json/jsonb.rs index ee869cee2..4c4a2de04 100644 --- a/core/json/jsonb.rs +++ b/core/json/jsonb.rs @@ -179,6 +179,7 @@ pub struct Jsonb { } #[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[allow(clippy::enum_variant_names, clippy::upper_case_acronyms)] pub enum ElementType { NULL = 0, TRUE = 1, @@ -203,7 +204,7 @@ pub enum JsonIndentation<'a> { None, } -impl<'a> JsonIndentation<'a> { +impl JsonIndentation<'_> { pub fn is_pretty(&self) -> bool { match self { Self::Indentation(_) => true, @@ -2067,10 +2068,10 @@ impl Jsonb { return Ok(pos); } - return Err(PError::Message { + Err(PError::Message { msg: "Expected null or nan".to_string(), location: Some(pos), - }); + }) } fn write_element_header( @@ -2108,13 +2109,17 @@ impl Jsonb { let new_len = header_bytes.len(); - if new_len > old_len { - self.data.splice( - cursor + old_len..cursor + old_len, - std::iter::repeat(0).take(new_len - old_len), - ); - } else if new_len < old_len { - self.data.drain(cursor + new_len..cursor + old_len); + match new_len.cmp(&old_len) { + std::cmp::Ordering::Greater => { + self.data.splice( + cursor + old_len..cursor + old_len, + std::iter::repeat(0).take(new_len - old_len), + ); + } + std::cmp::Ordering::Less => { + self.data.drain(cursor + new_len..cursor + old_len); + } + std::cmp::Ordering::Equal => {} } for (i, &byte) in header_bytes.iter().enumerate() { @@ -2365,7 +2370,7 @@ impl Jsonb { } else { if root_type == ElementType::OBJECT && root_size == 0 - && (*idx == Some(0) || *idx == None) + && (*idx == Some(0) || idx.is_none()) && mode.allows_insert() { let array = JsonbHeader::new(ElementType::ARRAY, 0).into_bytes(); @@ -3093,7 +3098,7 @@ mod tests { // Test round-trip let reparsed = Jsonb::from_str("null").unwrap(); - assert_eq!(reparsed.data[0] as u8, ElementType::NULL as u8); + assert_eq!(reparsed.data[0], ElementType::NULL as u8); } #[test] @@ -3110,10 +3115,10 @@ mod tests { // Round-trip let true_parsed = Jsonb::from_str("true").unwrap(); - assert_eq!(true_parsed.data[0] as u8, ElementType::TRUE as u8); + assert_eq!(true_parsed.data[0], ElementType::TRUE as u8); let false_parsed = Jsonb::from_str("false").unwrap(); - assert_eq!(false_parsed.data[0] as u8, ElementType::FALSE as u8); + assert_eq!(false_parsed.data[0], ElementType::FALSE as u8); } #[test] @@ -3398,12 +3403,12 @@ world""#, // Create a JSON string that exceeds MAX_JSON_DEPTH let mut deep_json = String::from("["); for _ in 0..MAX_JSON_DEPTH + 1 { - deep_json.push_str("["); + deep_json.push('['); } for _ in 0..MAX_JSON_DEPTH + 1 { - deep_json.push_str("]"); + deep_json.push(']'); } - deep_json.push_str("]"); + deep_json.push(']'); // Should fail due to exceeding depth limit assert!(Jsonb::from_str(&deep_json).is_err()); @@ -3559,10 +3564,10 @@ world""#, for i in 0..1000 { large_array.push_str(&format!("{}", i)); if i < 999 { - large_array.push_str(","); + large_array.push(','); } } - large_array.push_str("]"); + large_array.push(']'); let parsed = Jsonb::from_str(&large_array).unwrap(); assert!(parsed.to_string().unwrap().starts_with("[0,1,2,")); diff --git a/core/json/mod.rs b/core/json/mod.rs index 3ddb03151..b76f671d8 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -208,13 +208,13 @@ pub fn json_set(args: &[Register], json_cache: &JsonCacheCell) -> crate::Result< } let make_jsonb_fn = curry_convert_dbtype_to_jsonb(Conv::Strict); - let mut json = json_cache.get_or_insert_with(&args[0].get_owned_value(), make_jsonb_fn)?; + let mut json = json_cache.get_or_insert_with(args[0].get_owned_value(), make_jsonb_fn)?; let other = args[1..].chunks_exact(2); for chunk in other { - let path = json_path_from_owned_value(&chunk[0].get_owned_value(), true)?; + let path = json_path_from_owned_value(chunk[0].get_owned_value(), true)?; - let value = convert_dbtype_to_jsonb(&chunk[1].get_owned_value(), Conv::NotStrict)?; + let value = convert_dbtype_to_jsonb(chunk[1].get_owned_value(), Conv::NotStrict)?; let mut op = SetOperation::new(value); if let Some(path) = path { let _ = json.operate_on_path(&path, &mut op); @@ -232,13 +232,13 @@ pub fn jsonb_set(args: &[Register], json_cache: &JsonCacheCell) -> crate::Result } let make_jsonb_fn = curry_convert_dbtype_to_jsonb(Conv::Strict); - let mut json = json_cache.get_or_insert_with(&args[0].get_owned_value(), make_jsonb_fn)?; + let mut json = json_cache.get_or_insert_with(args[0].get_owned_value(), make_jsonb_fn)?; let other = args[1..].chunks_exact(2); for chunk in other { - let path = json_path_from_owned_value(&chunk[0].get_owned_value(), true)?; + let path = json_path_from_owned_value(chunk[0].get_owned_value(), true)?; - let value = convert_dbtype_to_jsonb(&chunk[1].get_owned_value(), Conv::NotStrict)?; + let value = convert_dbtype_to_jsonb(chunk[1].get_owned_value(), Conv::NotStrict)?; let mut op = SetOperation::new(value); if let Some(path) = path { let _ = json.operate_on_path(&path, &mut op); @@ -360,7 +360,7 @@ pub fn jsonb_extract( fn jsonb_extract_internal(value: Jsonb, paths: &[Register]) -> crate::Result<(Jsonb, ElementType)> { let null = Jsonb::from_raw_data(JsonbHeader::make_null().into_bytes().as_bytes()); if paths.len() == 1 { - if let Some(path) = json_path_from_owned_value(&paths[0].get_owned_value(), true)? { + if let Some(path) = json_path_from_owned_value(paths[0].get_owned_value(), true)? { let mut json = value; let mut op = SearchOperation::new(json.len()); @@ -559,9 +559,9 @@ pub fn json_object(values: &[Register]) -> crate::Result { if chunk[0].get_owned_value().value_type() != ValueType::Text { bail_constraint_error!("json_object() labels must be TEXT") } - let key = convert_dbtype_to_jsonb(&chunk[0].get_owned_value(), Conv::ToString)?; + let key = convert_dbtype_to_jsonb(chunk[0].get_owned_value(), Conv::ToString)?; json.append_jsonb_to_end(key.data()); - let value = convert_dbtype_to_jsonb(&chunk[1].get_owned_value(), Conv::NotStrict)?; + let value = convert_dbtype_to_jsonb(chunk[1].get_owned_value(), Conv::NotStrict)?; json.append_jsonb_to_end(value.data()); } @@ -580,9 +580,9 @@ pub fn jsonb_object(values: &[Register]) -> crate::Result { if chunk[0].get_owned_value().value_type() != ValueType::Text { bail_constraint_error!("json_object() labels must be TEXT") } - let key = convert_dbtype_to_jsonb(&chunk[0].get_owned_value(), Conv::ToString)?; + let key = convert_dbtype_to_jsonb(chunk[0].get_owned_value(), Conv::ToString)?; json.append_jsonb_to_end(key.data()); - let value = convert_dbtype_to_jsonb(&chunk[1].get_owned_value(), Conv::NotStrict)?; + let value = convert_dbtype_to_jsonb(chunk[1].get_owned_value(), Conv::NotStrict)?; json.append_jsonb_to_end(value.data()); } @@ -624,7 +624,7 @@ pub fn json_quote(value: &Value) -> crate::Result { } escaped_value.push('"'); - Ok(Value::build_text(&escaped_value)) + Ok(Value::build_text(escaped_value)) } // Numbers are unquoted in json Value::Integer(ref int) => Ok(Value::Integer(int.to_owned())), diff --git a/core/json/ops.rs b/core/json/ops.rs index ef5242465..eb2785c52 100644 --- a/core/json/ops.rs +++ b/core/json/ops.rs @@ -54,7 +54,7 @@ pub fn json_remove(args: &[Register], json_cache: &JsonCacheCell) -> crate::Resu } let make_jsonb_fn = curry_convert_dbtype_to_jsonb(Conv::Strict); - let mut json = json_cache.get_or_insert_with(&args[0].get_owned_value(), make_jsonb_fn)?; + let mut json = json_cache.get_or_insert_with(args[0].get_owned_value(), make_jsonb_fn)?; for arg in &args[1..] { if let Some(path) = json_path_from_owned_value(arg.get_owned_value(), true)? { let mut op = DeleteOperation::new(); @@ -73,7 +73,7 @@ pub fn jsonb_remove(args: &[Register], json_cache: &JsonCacheCell) -> crate::Res } let make_jsonb_fn = curry_convert_dbtype_to_jsonb(Conv::Strict); - let mut json = json_cache.get_or_insert_with(&args[0].get_owned_value(), make_jsonb_fn)?; + let mut json = json_cache.get_or_insert_with(args[0].get_owned_value(), make_jsonb_fn)?; for arg in &args[1..] { if let Some(path) = json_path_from_owned_value(arg.get_owned_value(), true)? { let mut op = DeleteOperation::new(); @@ -90,12 +90,12 @@ pub fn json_replace(args: &[Register], json_cache: &JsonCacheCell) -> crate::Res } let make_jsonb_fn = curry_convert_dbtype_to_jsonb(Conv::Strict); - let mut json = json_cache.get_or_insert_with(&args[0].get_owned_value(), make_jsonb_fn)?; + let mut json = json_cache.get_or_insert_with(args[0].get_owned_value(), make_jsonb_fn)?; let other = args[1..].chunks_exact(2); for chunk in other { - let path = json_path_from_owned_value(&chunk[0].get_owned_value(), true)?; + let path = json_path_from_owned_value(chunk[0].get_owned_value(), true)?; - let value = convert_dbtype_to_jsonb(&chunk[1].get_owned_value(), Conv::NotStrict)?; + let value = convert_dbtype_to_jsonb(chunk[1].get_owned_value(), Conv::NotStrict)?; if let Some(path) = path { let mut op = ReplaceOperation::new(value); @@ -114,11 +114,11 @@ pub fn jsonb_replace(args: &[Register], json_cache: &JsonCacheCell) -> crate::Re } let make_jsonb_fn = curry_convert_dbtype_to_jsonb(Conv::Strict); - let mut json = json_cache.get_or_insert_with(&args[0].get_owned_value(), make_jsonb_fn)?; + let mut json = json_cache.get_or_insert_with(args[0].get_owned_value(), make_jsonb_fn)?; let other = args[1..].chunks_exact(2); for chunk in other { - let path = json_path_from_owned_value(&chunk[0].get_owned_value(), true)?; - let value = convert_dbtype_to_jsonb(&chunk[1].get_owned_value(), Conv::NotStrict)?; + let path = json_path_from_owned_value(chunk[0].get_owned_value(), true)?; + let value = convert_dbtype_to_jsonb(chunk[1].get_owned_value(), Conv::NotStrict)?; if let Some(path) = path { let mut op = ReplaceOperation::new(value); @@ -137,11 +137,11 @@ pub fn json_insert(args: &[Register], json_cache: &JsonCacheCell) -> crate::Resu } let make_jsonb_fn = curry_convert_dbtype_to_jsonb(Conv::Strict); - let mut json = json_cache.get_or_insert_with(&args[0].get_owned_value(), make_jsonb_fn)?; + let mut json = json_cache.get_or_insert_with(args[0].get_owned_value(), make_jsonb_fn)?; let other = args[1..].chunks_exact(2); for chunk in other { - let path = json_path_from_owned_value(&chunk[0].get_owned_value(), true)?; - let value = convert_dbtype_to_jsonb(&chunk[1].get_owned_value(), Conv::NotStrict)?; + let path = json_path_from_owned_value(chunk[0].get_owned_value(), true)?; + let value = convert_dbtype_to_jsonb(chunk[1].get_owned_value(), Conv::NotStrict)?; if let Some(path) = path { let mut op = InsertOperation::new(value); @@ -160,11 +160,11 @@ pub fn jsonb_insert(args: &[Register], json_cache: &JsonCacheCell) -> crate::Res } let make_jsonb_fn = curry_convert_dbtype_to_jsonb(Conv::Strict); - let mut json = json_cache.get_or_insert_with(&args[0].get_owned_value(), make_jsonb_fn)?; + let mut json = json_cache.get_or_insert_with(args[0].get_owned_value(), make_jsonb_fn)?; let other = args[1..].chunks_exact(2); for chunk in other { - let path = json_path_from_owned_value(&chunk[0].get_owned_value(), true)?; - let value = convert_dbtype_to_jsonb(&chunk[1].get_owned_value(), Conv::NotStrict)?; + let path = json_path_from_owned_value(chunk[0].get_owned_value(), true)?; + let value = convert_dbtype_to_jsonb(chunk[1].get_owned_value(), Conv::NotStrict)?; if let Some(path) = path { let mut op = InsertOperation::new(value); @@ -184,7 +184,7 @@ mod tests { use super::*; fn create_text(s: &str) -> Value { - Value::Text(Text::from_str(s)) + Value::Text(s.into()) } fn create_json(s: &str) -> Value { diff --git a/core/json/path.rs b/core/json/path.rs index 1412ada9e..5dcca118f 100644 --- a/core/json/path.rs +++ b/core/json/path.rs @@ -168,6 +168,7 @@ fn handle_after_root( } } +#[allow(clippy::too_many_arguments)] fn handle_in_key<'a>( ch: (usize, char), parser_state: &mut PPState, diff --git a/core/lib.rs b/core/lib.rs index 679ff78e7..a4636e89b 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -237,7 +237,7 @@ impl Database { mv_transactions: RefCell::new(Vec::new()), transaction_state: Cell::new(TransactionState::None), last_change: Cell::new(0), - syms: RefCell::new(SymbolTable::new()), + syms: RefCell::new(SymbolTable::default()), total_changes: Cell::new(0), _shared_cache: false, cache_size: Cell::new(self.header.lock().default_page_cache_size), @@ -375,7 +375,7 @@ impl Connection { self.clone(), &syms, QueryMode::Normal, - &input, + input, )?); Ok(Statement::new( program, @@ -494,7 +494,7 @@ impl Connection { self.clone(), &syms, QueryMode::Explain, - &input, + input, )?; let _ = std::io::stdout().write_all(program.explain().as_bytes()); } @@ -511,7 +511,7 @@ impl Connection { self.clone(), &syms, QueryMode::Normal, - &input, + input, )?; let mut state = @@ -648,12 +648,7 @@ impl Connection { loop { match stmt.step()? { vdbe::StepResult::Row => { - let row: Vec = stmt - .row() - .unwrap() - .get_values() - .map(|v| v.clone()) - .collect(); + let row: Vec = stmt.row().unwrap().get_values().cloned().collect(); results.push(row); } vdbe::StepResult::Interrupt | vdbe::StepResult::Busy => { @@ -681,12 +676,7 @@ impl Connection { loop { match stmt.step()? { vdbe::StepResult::Row => { - let row: Vec = stmt - .row() - .unwrap() - .get_values() - .map(|v| v.clone()) - .collect(); + let row: Vec = stmt.row().unwrap().get_values().cloned().collect(); results.push(row); } vdbe::StepResult::Interrupt | vdbe::StepResult::Busy => { @@ -716,12 +706,7 @@ impl Connection { loop { match stmt.step()? { vdbe::StepResult::Row => { - let row: Vec = stmt - .row() - .unwrap() - .get_values() - .map(|v| v.clone()) - .collect(); + let row: Vec = stmt.row().unwrap().get_values().cloned().collect(); results.push(row); } vdbe::StepResult::Interrupt | vdbe::StepResult::Busy => { @@ -815,6 +800,7 @@ pub type Row = vdbe::Row; pub type StepResult = vdbe::StepResult; +#[derive(Default)] pub struct SymbolTable { pub functions: HashMap>, pub vtabs: HashMap>, @@ -857,14 +843,6 @@ pub fn resolve_ext_path(extpath: &str) -> Result { } impl SymbolTable { - pub fn new() -> Self { - Self { - functions: HashMap::new(), - vtabs: HashMap::new(), - vtab_modules: HashMap::new(), - } - } - pub fn resolve_function( &self, name: &str, @@ -903,7 +881,7 @@ impl Iterator for QueryRunner<'_> { .unwrap() .trim(); self.last_offset = byte_offset_end; - Some(self.conn.run_cmd(cmd, &input)) + Some(self.conn.run_cmd(cmd, input)) } Ok(None) => None, Err(err) => { diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs index e3ed96738..14b497ff3 100644 --- a/core/mvcc/database/mod.rs +++ b/core/mvcc/database/mod.rs @@ -387,8 +387,7 @@ impl MvStore { if let Some(rv) = row_versions .iter() .rev() - .filter(|rv| rv.is_visible_to(&tx, &self.txs)) - .next() + .find(|rv| rv.is_visible_to(&tx, &self.txs)) { tx.insert_to_read_set(id); return Ok(Some(rv.row.clone())); diff --git a/core/mvcc/database/tests.rs b/core/mvcc/database/tests.rs index b1c93a7ac..17ffcff83 100644 --- a/core/mvcc/database/tests.rs +++ b/core/mvcc/database/tests.rs @@ -721,7 +721,7 @@ fn test_lazy_scan_cursor_basic() { assert_eq!(count, 5); // After the last row, is_empty should return true - assert!(cursor.forward() == false); + assert!(!cursor.forward()); assert!(cursor.is_empty()); } @@ -840,7 +840,7 @@ fn test_scan_cursor_basic() { assert_eq!(count, 5); // After the last row, is_empty should return true - assert!(cursor.forward() == false); + assert!(!cursor.forward()); assert!(cursor.is_empty()); } diff --git a/core/numeric/mod.rs b/core/numeric/mod.rs index ba1127f66..7f7beac10 100644 --- a/core/numeric/mod.rs +++ b/core/numeric/mod.rs @@ -413,7 +413,7 @@ impl std::ops::Mul for DoubleDouble { impl std::ops::MulAssign for DoubleDouble { fn mul_assign(&mut self, rhs: Self) { - *self = self.clone() * rhs; + *self = *self * rhs; } } @@ -539,7 +539,7 @@ pub fn str_to_f64(input: impl AsRef) -> Option { if exponent > 0 { while exponent >= 100 { exponent -= 100; - result *= DoubleDouble(1.0e+100, -1.5902891109759918046e+83); + result *= DoubleDouble(1.0e+100, -1.590_289_110_975_991_8e83); } while exponent >= 10 { exponent -= 10; @@ -552,15 +552,15 @@ pub fn str_to_f64(input: impl AsRef) -> Option { } else { while exponent <= -100 { exponent += 100; - result *= DoubleDouble(1.0e-100, -1.99918998026028836196e-117); + result *= DoubleDouble(1.0e-100, -1.999_189_980_260_288_3e-117); } while exponent <= -10 { exponent += 10; - result *= DoubleDouble(1.0e-10, -3.6432197315497741579e-27); + result *= DoubleDouble(1.0e-10, -3.643_219_731_549_774e-27); } while exponent <= -1 { exponent += 1; - result *= DoubleDouble(1.0e-01, -5.5511151231257827021e-18); + result *= DoubleDouble(1.0e-01, -5.551_115_123_125_783e-18); } } diff --git a/core/pseudo.rs b/core/pseudo.rs index c4bb1ee40..d55ba0e98 100644 --- a/core/pseudo.rs +++ b/core/pseudo.rs @@ -1,14 +1,11 @@ use crate::types::ImmutableRecord; +#[derive(Default)] pub struct PseudoCursor { current: Option, } impl PseudoCursor { - pub fn new() -> Self { - Self { current: None } - } - pub fn record(&self) -> Option<&ImmutableRecord> { self.current.as_ref() } diff --git a/core/schema.rs b/core/schema.rs index c24fe5ef9..ca61cd892 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -67,7 +67,7 @@ impl Schema { pub fn get_table(&self, name: &str) -> Option> { let name = normalize_ident(name); - let name = if name.eq_ignore_ascii_case(&SCHEMA_TABLE_NAME_ALT) { + let name = if name.eq_ignore_ascii_case(SCHEMA_TABLE_NAME_ALT) { SCHEMA_TABLE_NAME } else { &name @@ -500,7 +500,7 @@ fn create_table( } => { primary_key = true; if let Some(o) = o { - order = o.clone(); + order = *o; } } limbo_sqlite3_parser::ast::ColumnConstraint::NotNull { .. } => { @@ -859,7 +859,7 @@ impl Affinity { } } - pub fn to_char_code(&self) -> u8 { + pub fn as_char_code(&self) -> u8 { self.aff_mask() as u8 } @@ -1168,7 +1168,7 @@ impl Index { .all(|col| set.contains(col)) { // skip unique columns that are satisfied with pk constraint - return false; + false } else { true } @@ -1463,7 +1463,7 @@ mod tests { let sql = r#"CREATE TABLE t1 (a INTEGER NOT NULL);"#; let table = BTreeTable::from_sql(sql, 0)?; let column = table.get_column("a").unwrap().1; - assert_eq!(column.notnull, true); + assert!(column.notnull); Ok(()) } @@ -1472,7 +1472,7 @@ mod tests { let sql = r#"CREATE TABLE t1 (a INTEGER);"#; let table = BTreeTable::from_sql(sql, 0)?; let column = table.get_column("a").unwrap().1; - assert_eq!(column.notnull, false); + assert!(!column.notnull); Ok(()) } diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 890b8b420..cec3766c6 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -580,7 +580,7 @@ impl BTreeCursor { return None; } let rowid = match self.get_immutable_record().as_ref().unwrap().last_value() { - Some(RefValue::Integer(rowid)) => *rowid as i64, + Some(RefValue::Integer(rowid)) => *rowid, _ => unreachable!( "index where has_rowid() is true should have an integer rowid as the last value" ), @@ -929,8 +929,8 @@ impl BTreeCursor { CursorState::ReadWritePayload(PayloadOverflowWithOffset::SkipOverflowPages { next_page: first_overflow_page.unwrap(), pages_left_to_skip: pages_to_skip, - page_offset: page_offset, - amount: amount, + page_offset, + amount, buffer_offset: bytes_processed as usize, is_write, }); @@ -964,7 +964,7 @@ impl BTreeCursor { CursorState::ReadWritePayload(PayloadOverflowWithOffset::ProcessPage { next_page: *next_page, remaining_to_read: *amount, - page: page, + page, current_offset: *page_offset as usize, buffer_offset: *buffer_offset, is_write: *is_write, @@ -1102,7 +1102,7 @@ impl BTreeCursor { payload_offset: u32, num_bytes: u32, payload: &[u8], - buffer: &mut Vec, + buffer: &mut [u8], page: BTreePage, ) { page.get().set_dirty(); @@ -1353,9 +1353,8 @@ impl BTreeCursor { let max = max_cell_idx.get(); if min > max { if let Some(nearest_matching_cell) = nearest_matching_cell.get() { - let left_child_page = contents.cell_table_interior_read_left_child_page( - nearest_matching_cell as usize, - )?; + let left_child_page = contents + .cell_table_interior_read_left_child_page(nearest_matching_cell)?; self.stack.set_cell_index(nearest_matching_cell as i32); let mem_page = self.read_page(left_child_page as usize)?; self.stack.push(mem_page); @@ -1737,19 +1736,17 @@ impl BTreeCursor { min_cell_idx.set(cur_cell_idx + 1); } } + } else if cmp.is_gt() { + max_cell_idx.set(cur_cell_idx - 1); + } else if cmp.is_lt() { + min_cell_idx.set(cur_cell_idx + 1); } else { - if cmp.is_gt() { - max_cell_idx.set(cur_cell_idx - 1); - } else if cmp.is_lt() { - min_cell_idx.set(cur_cell_idx + 1); - } else { - match iter_dir { - IterationDirection::Forwards => { - min_cell_idx.set(cur_cell_idx + 1); - } - IterationDirection::Backwards => { - max_cell_idx.set(cur_cell_idx - 1); - } + match iter_dir { + IterationDirection::Forwards => { + min_cell_idx.set(cur_cell_idx + 1); + } + IterationDirection::Backwards => { + max_cell_idx.set(cur_cell_idx - 1); } } } @@ -1964,19 +1961,17 @@ impl BTreeCursor { min_cell_idx.set(cur_cell_idx + 1); } } + } else if cmp.is_gt() { + max_cell_idx.set(cur_cell_idx - 1); + } else if cmp.is_lt() { + min_cell_idx.set(cur_cell_idx + 1); } else { - if cmp.is_gt() { - max_cell_idx.set(cur_cell_idx - 1); - } else if cmp.is_lt() { - min_cell_idx.set(cur_cell_idx + 1); - } else { - match iter_dir { - IterationDirection::Forwards => { - min_cell_idx.set(cur_cell_idx + 1); - } - IterationDirection::Backwards => { - max_cell_idx.set(cur_cell_idx - 1); - } + match iter_dir { + IterationDirection::Forwards => { + min_cell_idx.set(cur_cell_idx + 1); + } + IterationDirection::Backwards => { + max_cell_idx.set(cur_cell_idx - 1); } } } @@ -2744,7 +2739,7 @@ impl BTreeCursor { } else { size_of_cell_to_remove_from_left }; - new_page_sizes[i + 1] += size_of_cell_to_move_right as i64; + new_page_sizes[i + 1] += size_of_cell_to_move_right; cell_array.number_of_cells_per_page[i] -= 1; } @@ -2967,7 +2962,7 @@ impl BTreeCursor { if !is_leaf_page { // Interior // Make this page's rightmost pointer point to pointer of divider cell before modification - let previous_pointer_divider = read_u32(÷r_cell, 0); + let previous_pointer_divider = read_u32(divider_cell, 0); page.get() .get_contents() .write_u32(offset::BTREE_RIGHTMOST_PTR, previous_pointer_divider); @@ -2992,7 +2987,7 @@ impl BTreeCursor { let (rowid, _) = read_varint(÷r_cell[n_bytes_payload..])?; new_divider_cell .extend_from_slice(&(page.get().get().id as u32).to_be_bytes()); - write_varint_to_vec(rowid as u64, &mut new_divider_cell); + write_varint_to_vec(rowid, &mut new_divider_cell); } else { // Leaf index new_divider_cell @@ -3225,7 +3220,7 @@ impl BTreeCursor { i: usize, page: &std::sync::Arc, ) { - let left_pointer = if parent_contents.overflow_cells.len() == 0 { + let left_pointer = if parent_contents.overflow_cells.is_empty() { let (cell_start, cell_len) = parent_contents.cell_get_raw_region( balance_info.first_divider_cell + i, payload_overflow_threshold_max( @@ -3265,6 +3260,7 @@ impl BTreeCursor { } #[cfg(debug_assertions)] + #[allow(clippy::too_many_arguments)] fn post_balance_non_root_validation( &self, parent_page: &BTreePage, @@ -3420,7 +3416,7 @@ impl BTreeCursor { let rightmost = read_u32(rightmost_pointer, 0); debug_validate_cells!(parent_contents, self.usable_space() as u16); - if !pages_to_balance_new[0].is_some() { + if pages_to_balance_new[0].is_none() { tracing::error!( "balance_non_root(balance_shallower_incorrect_page, page_idx={})", 0 @@ -3428,8 +3424,13 @@ impl BTreeCursor { valid = false; } - for i in 1..sibling_count_new { - if pages_to_balance_new[i].is_some() { + for (i, value) in pages_to_balance_new + .iter() + .enumerate() + .take(sibling_count_new) + .skip(1) + { + if value.is_some() { tracing::error!( "balance_non_root(balance_shallower_incorrect_page, page_idx={})", i @@ -3490,7 +3491,9 @@ impl BTreeCursor { valid = false } - for parent_cell_idx in 0..contents.cell_count() { + for (parent_cell_idx, cell_buf_in_array) in + cells_debug.iter().enumerate().take(contents.cell_count()) + { let (parent_cell_start, parent_cell_len) = parent_contents.cell_get_raw_region( parent_cell_idx, payload_overflow_threshold_max( @@ -3522,7 +3525,6 @@ impl BTreeCursor { let parent_cell_buf = to_static_buf( &mut parent_buf[parent_cell_start..parent_cell_start + parent_cell_len], ); - let cell_buf_in_array = &cells_debug[parent_cell_idx]; if cell_buf != cell_buf_in_array || cell_buf != parent_cell_buf { tracing::error!("balance_non_root(balance_shallower_cell_not_found_debug, page_id={}, cell_in_cell_array_idx={})", @@ -4225,11 +4227,9 @@ impl BTreeCursor { return Ok(CursorResult::Ok(())); } }; - } else { - if self.reusable_immutable_record.borrow().is_none() { - self.state = CursorState::None; - return Ok(CursorResult::Ok(())); - } + } else if self.reusable_immutable_record.borrow().is_none() { + self.state = CursorState::None; + return Ok(CursorResult::Ok(())); } let delete_info = self.state.mut_delete_info().unwrap(); @@ -4934,7 +4934,7 @@ impl BTreeCursor { return_if_locked!(page_ref.get()); let page_ref = page_ref.get(); let buf = page_ref.get().contents.as_mut().unwrap().as_ptr(); - buf[dest_offset..dest_offset + new_payload.len()].copy_from_slice(&new_payload); + buf[dest_offset..dest_offset + new_payload.len()].copy_from_slice(new_payload); Ok(CursorResult::Ok(())) } @@ -4952,10 +4952,7 @@ impl BTreeCursor { } pub fn is_write_in_progress(&self) -> bool { - match self.state { - CursorState::Write(_) => true, - _ => false, - } + matches!(self.state, CursorState::Write(_)) } /// Count the number of entries in the b-tree @@ -6503,7 +6500,6 @@ mod tests { storage::{ database::DatabaseFile, page_cache::DumbLruPageCache, - pager::CreateBTreeFlags, sqlite3_ondisk::{self, DatabaseHeader}, }, types::Text, @@ -7098,6 +7094,8 @@ mod tests { #[cfg(feature = "index_experimental")] fn btree_index_insert_fuzz_run(attempts: usize, inserts: usize) { + use crate::storage::pager::CreateBTreeFlags; + let (mut rng, seed) = if std::env::var("SEED").is_ok() { let seed = std::env::var("SEED").unwrap(); let seed = seed.parse::().unwrap(); @@ -7825,7 +7823,7 @@ mod tests { // allow appends with extra place to insert let cell_idx = rng.next_u64() as usize % (page.cell_count() + 1); let free = compute_free_space(page, usable_space); - let regs = &[Register::Value(Value::Integer(i as i64))]; + let regs = &[Register::Value(Value::Integer(i))]; let record = ImmutableRecord::from_registers(regs, regs.len()); let mut payload: Vec = Vec::new(); fill_cell_payload( @@ -8339,9 +8337,8 @@ mod tests { .unwrap(); } - match validate_btree(pager.clone(), root_page) { - (_, false) => panic!("Invalid B-tree after insertion"), - _ => {} + if let (_, false) = validate_btree(pager.clone(), root_page) { + panic!("Invalid B-tree after insertion"); } // Delete records with 500 <= key <= 3500 @@ -8362,7 +8359,7 @@ mod tests { // Verify that records with key < 500 and key > 3500 still exist in the BTree. for i in 1..=10000 { - if i >= 500 && i <= 3500 { + if (500..=3500).contains(&i) { continue; } @@ -8395,11 +8392,11 @@ mod tests { let (pager, root_page) = empty_btree(); - for i in 0..iterations { + for (i, huge_text) in huge_texts.iter().enumerate().take(iterations) { let mut cursor = BTreeCursor::new_table(None, pager.clone(), root_page); tracing::info!("INSERT INTO t VALUES ({});", i,); let regs = &[Register::Value(Value::Text(Text { - value: huge_texts[i].as_bytes().to_vec(), + value: huge_text.as_bytes().to_vec(), subtype: crate::types::TextSubtype::Text, }))]; let value = ImmutableRecord::from_registers(regs, regs.len()); @@ -8674,7 +8671,7 @@ mod tests { let removed = page_free_array( contents, start, - size as usize, + size, &cell_array, pager.usable_space() as u16, ) diff --git a/core/storage/page_cache.rs b/core/storage/page_cache.rs index 344280fe1..fd5d25bc4 100644 --- a/core/storage/page_cache.rs +++ b/core/storage/page_cache.rs @@ -514,12 +514,12 @@ impl PageHashMap { } pub fn contains_key(&self, key: &PageCacheKey) -> bool { - let bucket = self.hash(&key); + let bucket = self.hash(key); self.buckets[bucket].iter().any(|node| node.key == *key) } pub fn get(&self, key: &PageCacheKey) -> Option<&NonNull> { - let bucket = self.hash(&key); + let bucket = self.hash(key); let bucket = &self.buckets[bucket]; let mut idx = 0; while let Some(node) = bucket.get(idx) { @@ -532,7 +532,7 @@ impl PageHashMap { } pub fn remove(&mut self, key: &PageCacheKey) -> Option> { - let bucket = self.hash(&key); + let bucket = self.hash(key); let bucket = &mut self.buckets[bucket]; let mut idx = 0; while let Some(node) = bucket.get(idx) { @@ -996,7 +996,7 @@ mod tests { let key = PageCacheKey::new(id_page as usize); #[allow(clippy::arc_with_non_send_sync)] let page = Arc::new(Page::new(id_page as usize)); - if let Some(_) = cache.peek(&key, false) { + if cache.peek(&key, false).is_some() { continue; // skip duplicate page ids } tracing::debug!("inserting page {:?}", key); @@ -1017,11 +1017,11 @@ mod tests { let random = rng.next_u64() % 2 == 0; let key = if random || lru.is_empty() { let id_page: u64 = rng.next_u64() % max_pages; - let key = PageCacheKey::new(id_page as usize); - key + + PageCacheKey::new(id_page as usize) } else { let i = rng.next_u64() as usize % lru.len(); - let key: PageCacheKey = lru.iter().skip(i).next().unwrap().0.clone(); + let key: PageCacheKey = lru.iter().nth(i).unwrap().0.clone(); key }; tracing::debug!("removing page {:?}", key); @@ -1038,7 +1038,7 @@ mod tests { cache.verify_list_integrity(); for (key, page) in &lru { println!("getting page {:?}", key); - cache.peek(&key, false).unwrap(); + cache.peek(key, false).unwrap(); assert_eq!(page.get().id, key.pgno); } } diff --git a/core/storage/pager.rs b/core/storage/pager.rs index ce266b250..4c71d93aa 100644 --- a/core/storage/pager.rs +++ b/core/storage/pager.rs @@ -449,7 +449,7 @@ impl Pager { { let page = self.do_allocate_page(page_type, 0, BtreePageAllocMode::Any); let page_id = page.get().get().id; - return Ok(CursorResult::Ok(page_id as u32)); + Ok(CursorResult::Ok(page_id as u32)) } // If autovacuum is enabled, we need to allocate a new page number that is greater than the largest root page number @@ -460,7 +460,7 @@ impl Pager { AutoVacuumMode::None => { let page = self.do_allocate_page(page_type, 0, BtreePageAllocMode::Any); let page_id = page.get().get().id; - return Ok(CursorResult::Ok(page_id as u32)); + Ok(CursorResult::Ok(page_id as u32)) } AutoVacuumMode::Full => { let mut root_page_num = self.db_header.lock().vacuum_mode_largest_root_page; @@ -560,14 +560,14 @@ impl Pager { pub fn end_tx(&self) -> Result { let cacheflush_status = self.cacheflush()?; - return match cacheflush_status { + match cacheflush_status { PagerCacheflushStatus::IO => Ok(PagerCacheflushStatus::IO), PagerCacheflushStatus::Done(_) => { self.wal.borrow().end_write_tx()?; self.wal.borrow().end_read_tx()?; Ok(cacheflush_status) } - }; + } } pub fn end_read_tx(&self) -> Result<()> { @@ -645,7 +645,7 @@ impl Pager { self.add_dirty(DATABASE_HEADER_PAGE_ID); let contents = header_page.get().contents.as_ref().unwrap(); - contents.write_database_header(&header); + contents.write_database_header(header); Ok(()) } @@ -756,12 +756,12 @@ impl Pager { frame_len: u32, ) -> Result> { let wal = self.wal.borrow(); - return wal.read_frame_raw( + wal.read_frame_raw( frame_no.into(), self.buffer_pool.clone(), p_frame, frame_len, - ); + ) } pub fn checkpoint(&self) -> Result { @@ -978,7 +978,7 @@ impl Pager { } // update database size - self.write_database_header(&mut header)?; + self.write_database_header(&header)?; // FIXME: should reserve page cache entry before modifying the database let page = allocate_page(header.database_size as usize, &self.buffer_pool, 0); @@ -1190,7 +1190,7 @@ mod ptrmap { if db_page_no == FIRST_PTRMAP_PAGE_NO { return true; } - return get_ptrmap_page_no_for_db_page(db_page_no, page_size) == db_page_no; + get_ptrmap_page_no_for_db_page(db_page_no, page_size) == db_page_no } /// Calculates which pointer map page (1-indexed) contains the entry for `db_page_no_to_query` (1-indexed). diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index 750ecf25c..d2e7fa27a 100644 --- a/core/storage/sqlite3_ondisk.rs +++ b/core/storage/sqlite3_ondisk.rs @@ -1095,14 +1095,13 @@ pub struct SmallVecIter<'a, T, const N: usize> { pos: usize, } -impl<'a, T: Default + Copy, const N: usize> Iterator for SmallVecIter<'a, T, N> { +impl Iterator for SmallVecIter<'_, T, N> { type Item = T; fn next(&mut self) -> Option { - self.vec.get(self.pos).map(|item| { - self.pos += 1; - item - }) + let next = self.vec.get(self.pos)?; + self.pos += 1; + Some(next) } } @@ -1401,7 +1400,7 @@ pub fn read_entire_wal_dumb(file: &Arc) -> Result) -> Result MAX_PAGE_SIZE + if !(MIN_PAGE_SIZE..=MAX_PAGE_SIZE).contains(&page_size_u32) || page_size_u32.count_ones() != 1 { panic!("Invalid page size in WAL header: {}", page_size_u32); @@ -1462,13 +1460,13 @@ pub fn read_entire_wal_dumb(file: &Arc) -> Result, offset: usize, buffer_pool: Rc, - complete: Box>) -> ()>, + complete: Box>)>, ) -> Result> { tracing::trace!("begin_read_wal_frame(offset={})", offset); let buf = buffer_pool.get(); @@ -1532,6 +1530,7 @@ pub fn begin_read_wal_frame( } #[instrument(skip(io, page, write_counter, wal_header, checksums), level = Level::TRACE)] +#[allow(clippy::too_many_arguments)] pub fn begin_write_wal_frame( io: &Arc, offset: usize, @@ -1752,8 +1751,8 @@ mod tests { #[case(&[0x40, 0x09, 0x21, 0xFB, 0x54, 0x44, 0x2D, 0x18], SerialType::f64(), Value::Float(std::f64::consts::PI))] #[case(&[1, 2], SerialType::const_int0(), Value::Integer(0))] #[case(&[65, 66], SerialType::const_int1(), Value::Integer(1))] - #[case(&[1, 2, 3], SerialType::blob(3), Value::Blob(vec![1, 2, 3].into()))] - #[case(&[], SerialType::blob(0), Value::Blob(vec![].into()))] // empty blob + #[case(&[1, 2, 3], SerialType::blob(3), Value::Blob(vec![1, 2, 3]))] + #[case(&[], SerialType::blob(0), Value::Blob(vec![]))] // empty blob #[case(&[65, 66, 67], SerialType::text(3), Value::build_text("ABC"))] #[case(&[0x80], SerialType::i8(), Value::Integer(-128))] #[case(&[0x80, 0], SerialType::i16(), Value::Integer(-32768))] diff --git a/core/translate/aggregation.rs b/core/translate/aggregation.rs index 3fb1ff83f..b558ec5fd 100644 --- a/core/translate/aggregation.rs +++ b/core/translate/aggregation.rs @@ -86,7 +86,7 @@ pub fn handle_distinct(program: &mut ProgramBuilder, agg: &Aggregate, agg_arg_re }); program.emit_insn(Insn::IdxInsert { cursor_id: distinct_ctx.cursor_id, - record_reg: record_reg, + record_reg, unpacked_start: None, unpacked_count: None, flags: IdxInsertFlags::new(), diff --git a/core/translate/delete.rs b/core/translate/delete.rs index 484b46527..a30730077 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -69,11 +69,7 @@ pub fn prepare_delete_plan( crate::bail_parse_error!("Table is neither a virtual table nor a btree table"); }; let name = tbl_name.name.0.as_str().to_string(); - let indexes = schema - .get_indices(table.get_name()) - .iter() - .cloned() - .collect(); + let indexes = schema.get_indices(table.get_name()).to_vec(); let joined_tables = vec![JoinedTable { table, identifier: name, @@ -83,7 +79,7 @@ pub fn prepare_delete_plan( index: None, }, join_info: None, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }]; let mut table_references = TableReferences::new(joined_tables, vec![]); diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index c054e9d0e..2e95c317a 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -262,7 +262,7 @@ pub fn emit_query<'a>( t_ctx: &mut TranslateCtx<'a>, ) -> Result { if !plan.values.is_empty() { - let reg_result_cols_start = emit_values(program, &plan, &t_ctx.resolver)?; + let reg_result_cols_start = emit_values(program, plan, &t_ctx.resolver)?; return Ok(reg_result_cols_start); } @@ -309,7 +309,7 @@ pub fn emit_query<'a>( program, t_ctx, group_by, - &plan, + plan, &plan.result_columns, &plan.order_by, )?; @@ -348,7 +348,7 @@ pub fn emit_query<'a>( t_ctx, &plan.table_references, &plan.join_order, - &mut plan.where_clause, + &plan.where_clause, None, )?; @@ -398,7 +398,7 @@ pub fn emit_query<'a>( #[instrument(skip_all, level = Level::TRACE)] fn emit_program_for_delete( program: &mut ProgramBuilder, - mut plan: DeletePlan, + plan: DeletePlan, schema: &Schema, syms: &SymbolTable, ) -> Result<()> { @@ -446,7 +446,7 @@ fn emit_program_for_delete( &mut t_ctx, &plan.table_references, &[JoinOrderMember::default()], - &mut plan.where_clause, + &plan.where_clause, None, )?; @@ -501,7 +501,7 @@ fn emit_delete_insns( dest: key_reg, }); - if let Some(_) = table_reference.virtual_table() { + if table_reference.virtual_table().is_some() { let conflict_action = 0u16; let start_reg = key_reg; @@ -672,7 +672,7 @@ fn emit_program_for_update( &mut t_ctx, &plan.table_references, &[JoinOrderMember::default()], - &mut plan.where_clause, + &plan.where_clause, temp_cursor_id, )?; @@ -935,7 +935,7 @@ fn emit_update_insns( if idx > 0 { accum.push_str(", "); } - accum.push_str(&table_ref.table.get_name()); + accum.push_str(table_ref.table.get_name()); accum.push('.'); accum.push_str(&col.name); @@ -1060,7 +1060,7 @@ fn emit_update_insns( // Insert new index key (filled further above with values from set_clauses) program.emit_insn(Insn::IdxInsert { cursor_id: idx_cursor_id, - record_reg: record_reg, + record_reg, unpacked_start: Some(start), unpacked_count: Some((index.columns.len() + 1) as u16), flags: IdxInsertFlags::new(), @@ -1074,7 +1074,7 @@ fn emit_update_insns( flag: InsertFlags::new().update(true), table_name: table_ref.identifier.clone(), }); - } else if let Some(_) = table_ref.virtual_table() { + } else if table_ref.virtual_table().is_some() { let arg_count = table_ref.columns().len() + 2; program.emit_insn(Insn::VUpdate { cursor_id, diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 851a26b5f..c8ee0430f 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -2211,6 +2211,7 @@ pub fn translate_expr( Ok(target_register) } +#[allow(clippy::too_many_arguments)] fn emit_binary_insn( program: &mut ProgramBuilder, op: &ast::Operator, @@ -2490,7 +2491,7 @@ fn translate_like_base( }; match op { ast::LikeOperator::Like | ast::LikeOperator::Glob => { - let arg_count = if matches!(escape, Some(_)) { 3 } else { 2 }; + let arg_count = if escape.is_some() { 3 } else { 2 }; let start_reg = program.alloc_registers(arg_count); let mut constant_mask = 0; translate_expr(program, referenced_tables, lhs, start_reg + 1, resolver)?; diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index 23bf54c1d..f744b8051 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -86,7 +86,7 @@ pub fn init_group_by<'a>( t_ctx: &mut TranslateCtx<'a>, group_by: &'a GroupBy, plan: &SelectPlan, - result_columns: &'a Vec, + result_columns: &'a [ResultSetColumn], order_by: &'a Option>, ) -> Result<()> { collect_non_aggregate_expressions( @@ -239,7 +239,7 @@ fn collect_non_aggregate_expressions<'a>( non_aggregate_expressions: &mut Vec<(&'a ast::Expr, bool)>, group_by: &'a GroupBy, plan: &SelectPlan, - root_result_columns: &'a Vec, + root_result_columns: &'a [ResultSetColumn], order_by: &'a Option>, ) -> Result<()> { let mut result_columns = Vec::new(); @@ -512,11 +512,11 @@ impl<'a> GroupByAggArgumentSource<'a> { } /// Emits bytecode for processing a single GROUP BY group. -pub fn group_by_process_single_group<'a>( +pub fn group_by_process_single_group( program: &mut ProgramBuilder, group_by: &GroupBy, plan: &SelectPlan, - t_ctx: &mut TranslateCtx<'a>, + t_ctx: &mut TranslateCtx, ) -> Result<()> { let GroupByMetadata { registers, @@ -663,16 +663,16 @@ pub fn group_by_process_single_group<'a>( start_reg_dest, .. } => { - let mut sorter_column_index = 0; let mut next_reg = *start_reg_dest; - for (expr, in_result) in t_ctx.non_aggregate_expressions.iter() { + for (sorter_column_index, (expr, in_result)) in + t_ctx.non_aggregate_expressions.iter().enumerate() + { if *in_result { program.emit_column(*pseudo_cursor, sorter_column_index, next_reg); t_ctx.resolver.expr_to_reg_cache.push((expr, next_reg)); next_reg += 1; } - sorter_column_index += 1; } } GroupByRowSource::MainLoop { start_reg_dest, .. } => { @@ -712,12 +712,12 @@ pub fn group_by_process_single_group<'a>( /// Emits the bytecode for processing the aggregation phase of a GROUP BY clause. /// This is called either when: /// 1. the main query execution loop has finished processing, -/// and we now have data in the GROUP BY sorter. +/// and we now have data in the GROUP BY sorter. /// 2. the rows are already sorted in the order that the GROUP BY keys are defined, -/// and we can start aggregating inside the main loop. -pub fn group_by_agg_phase<'a>( +/// and we can start aggregating inside the main loop. +pub fn group_by_agg_phase( program: &mut ProgramBuilder, - t_ctx: &mut TranslateCtx<'a>, + t_ctx: &mut TranslateCtx, plan: &SelectPlan, ) -> Result<()> { let GroupByMetadata { diff --git a/core/translate/index.rs b/core/translate/index.rs index 55d8f4a18..dec87c232 100644 --- a/core/translate/index.rs +++ b/core/translate/index.rs @@ -110,7 +110,7 @@ pub fn translate_create_index( ); // determine the order of the columns in the index for the sorter - let order = idx.columns.iter().map(|c| c.order.clone()).collect(); + let order = idx.columns.iter().map(|c| c.order).collect(); // open the sorter and the pseudo table program.emit_insn(Insn::SorterOpen { cursor_id: sorter_cursor_id, diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index 9700a69bb..955181934 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -67,7 +67,7 @@ impl LoopLabels { } pub fn init_distinct(program: &mut ProgramBuilder, plan: &SelectPlan) -> DistinctCtx { - let index_name = format!("distinct_{}", program.offset().to_offset_int()); // we don't really care about the name that much, just enough that we don't get name collisions + let index_name = format!("distinct_{}", program.offset().as_offset_int()); // we don't really care about the name that much, just enough that we don't get name collisions let index = Arc::new(Index { name: index_name.clone(), table_name: String::new(), @@ -100,7 +100,7 @@ pub fn init_distinct(program: &mut ProgramBuilder, plan: &SelectPlan) -> Distinc is_table: false, }); - return ctx; + ctx } /// Initialize resources needed for the source operators (tables, joins, etc) @@ -345,7 +345,7 @@ pub fn init_loop( jump_target_when_true: jump_target, jump_target_when_false: t_ctx.label_main_loop_end.unwrap(), }; - translate_condition_expr(program, &tables, &cond.expr, meta, &t_ctx.resolver)?; + translate_condition_expr(program, tables, &cond.expr, meta, &t_ctx.resolver)?; program.preassign_label_to_next_insn(jump_target); } @@ -631,7 +631,7 @@ pub fn open_loop( }; Some(emit_autoindex( program, - &index, + index, table_cursor_id .expect("an ephemeral index must have a source table cursor"), index_cursor_id @@ -747,9 +747,9 @@ enum LoopEmitTarget { /// Emits the bytecode for the inner loop of a query. /// At this point the cursors for all tables have been opened and rewound. -pub fn emit_loop<'a>( +pub fn emit_loop( program: &mut ProgramBuilder, - t_ctx: &mut TranslateCtx<'a>, + t_ctx: &mut TranslateCtx, plan: &SelectPlan, ) -> Result<()> { // if we have a group by, we emit a record into the group by sorter, @@ -773,9 +773,9 @@ pub fn emit_loop<'a>( /// This is a helper function for inner_loop_emit, /// which does a different thing depending on the emit target. /// See the InnerLoopEmitTarget enum for more details. -fn emit_loop_source<'a>( +fn emit_loop_source( program: &mut ProgramBuilder, - t_ctx: &mut TranslateCtx<'a>, + t_ctx: &mut TranslateCtx, plan: &SelectPlan, emit_target: LoopEmitTarget, ) -> Result<()> { @@ -1182,7 +1182,7 @@ fn emit_seek( translate_expr_no_constant_opt( program, Some(tables), - &expr, + expr, reg, &t_ctx.resolver, NoConstantOptReason::RegisterReuse, diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 9448c25cc..c6823576a 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -54,6 +54,7 @@ use transaction::{translate_tx_begin, translate_tx_commit}; use update::translate_update; #[instrument(skip_all, level = Level::TRACE)] +#[allow(clippy::too_many_arguments)] pub fn translate( schema: &Schema, stmt: ast::Stmt, @@ -151,7 +152,7 @@ pub fn translate_inner( ast::Stmt::CreateTrigger { .. } => bail_parse_error!("CREATE TRIGGER not supported yet"), ast::Stmt::CreateView { .. } => bail_parse_error!("CREATE VIEW not supported yet"), ast::Stmt::CreateVirtualTable(vtab) => { - translate_create_virtual_table(*vtab, schema, query_mode, &syms, program)? + translate_create_virtual_table(*vtab, schema, query_mode, syms, program)? } ast::Stmt::Delete(delete) => { let Delete { diff --git a/core/translate/optimizer/access_method.rs b/core/translate/optimizer/access_method.rs index 9fafe1fc6..947a724f0 100644 --- a/core/translate/optimizer/access_method.rs +++ b/core/translate/optimizer/access_method.rs @@ -31,7 +31,7 @@ pub struct AccessMethod<'a> { pub constraint_refs: &'a [ConstraintRef], } -impl<'a> AccessMethod<'a> { +impl AccessMethod<'_> { pub fn is_scan(&self) -> bool { self.constraint_refs.is_empty() } @@ -81,7 +81,7 @@ pub fn find_best_access_method_for_join_order<'a>( let cost = estimate_cost_for_scan_or_seek( Some(index_info), &rhs_constraints.constraints, - &usable_constraint_refs, + usable_constraint_refs, input_cardinality, ); @@ -139,7 +139,7 @@ pub fn find_best_access_method_for_join_order<'a>( cost, index: candidate.index.clone(), iter_dir, - constraint_refs: &usable_constraint_refs, + constraint_refs: usable_constraint_refs, }; } } diff --git a/core/translate/optimizer/cost.rs b/core/translate/optimizer/cost.rs index 862592b6a..460fa9b0a 100644 --- a/core/translate/optimizer/cost.rs +++ b/core/translate/optimizer/cost.rs @@ -33,7 +33,7 @@ pub const ESTIMATED_HARDCODED_ROWS_PER_TABLE: usize = 1000000; pub const ESTIMATED_HARDCODED_ROWS_PER_PAGE: usize = 50; // roughly 80 bytes per 4096 byte page pub fn estimate_page_io_cost(rowcount: f64) -> Cost { - Cost((rowcount as f64 / ESTIMATED_HARDCODED_ROWS_PER_PAGE as f64).ceil()) + Cost((rowcount / ESTIMATED_HARDCODED_ROWS_PER_PAGE as f64).ceil()) } /// Estimate the cost of a scan or seek operation. diff --git a/core/translate/optimizer/join.rs b/core/translate/optimizer/join.rs index 11ebbd109..2a35b2bf8 100644 --- a/core/translate/optimizer/join.rs +++ b/core/translate/optimizer/join.rs @@ -60,7 +60,7 @@ pub fn join_lhs_and_rhs<'a>( let best_access_method = find_best_access_method_for_join_order( rhs_table_reference, rhs_constraints, - &join_order, + join_order, maybe_order_target, input_cardinality as f64, )?; @@ -135,17 +135,17 @@ pub fn compute_best_join_order<'a>( joined_tables, maybe_order_target, access_methods_arena, - &constraints, + constraints, )?; // Keep track of both 1. the best plan overall (not considering sorting), and 2. the best ordered plan (which might not be the same). // We assign Some Cost (tm) to any required sort operation, so the best ordered plan may end up being // the one we choose, if the cost reduction from avoiding sorting brings it below the cost of the overall best one. let mut best_ordered_plan: Option = None; - let mut best_plan_is_also_ordered = if let Some(ref order_target) = maybe_order_target { + let mut best_plan_is_also_ordered = if let Some(order_target) = maybe_order_target { plan_satisfies_order_target( &naive_plan, - &access_methods_arena, + access_methods_arena, joined_tables, order_target, ) @@ -226,12 +226,8 @@ pub fn compute_best_join_order<'a>( let mut left_join_illegal_map: HashMap = HashMap::with_capacity(left_join_count); for (i, _) in joined_tables.iter().enumerate() { - for j in i + 1..joined_tables.len() { - if joined_tables[j] - .join_info - .as_ref() - .map_or(false, |j| j.outer) - { + for (j, joined_table) in joined_tables.iter().enumerate().skip(i + 1) { + if joined_table.join_info.as_ref().map_or(false, |j| j.outer) { // bitwise OR the masks if let Some(illegal_lhs) = left_join_illegal_map.get_mut(&i) { illegal_lhs.add_table(j); @@ -329,10 +325,10 @@ pub fn compute_best_join_order<'a>( continue; }; - let satisfies_order_target = if let Some(ref order_target) = maybe_order_target { + let satisfies_order_target = if let Some(order_target) = maybe_order_target { plan_satisfies_order_target( &rel, - &access_methods_arena, + access_methods_arena, joined_tables, order_target, ) @@ -1039,7 +1035,7 @@ mod tests { .unwrap(); // Verify that t2 is chosen first due to its equality filter - assert_eq!(best_plan.table_numbers().nth(0).unwrap(), 1); + assert_eq!(best_plan.table_numbers().next().unwrap(), 1); // Verify table scan is used since there are no indexes let access_method = &access_methods_arena.borrow()[best_plan.data[0].1]; assert!(access_method.is_scan()); @@ -1148,11 +1144,11 @@ mod tests { // Expected optimal order: fact table as outer, with rowid seeks in any order on each dimension table // Verify fact table is selected as the outer table as all the other tables can use SeekRowid assert_eq!( - best_plan.table_numbers().nth(0).unwrap(), + best_plan.table_numbers().next().unwrap(), FACT_TABLE_IDX, "First table should be fact (table {}) due to available index, got table {} instead", FACT_TABLE_IDX, - best_plan.table_numbers().nth(0).unwrap() + best_plan.table_numbers().next().unwrap() ); // Verify access methods @@ -1187,7 +1183,7 @@ mod tests { for i in 0..NUM_TABLES { let mut columns = vec![_create_column_rowid_alias("id")]; if i < NUM_TABLES - 1 { - columns.push(_create_column_of_type(&format!("next_id"), Type::Integer)); + columns.push(_create_column_of_type("next_id", Type::Integer)); } tables.push(_create_btree_table(&format!("t{}", i + 1), columns)); } @@ -1250,14 +1246,19 @@ mod tests { assert!(access_method.constraint_refs.is_empty()); // all of the rest should use rowid equality - for i in 1..NUM_TABLES { + for (i, table_constraints) in table_constraints + .iter() + .enumerate() + .take(NUM_TABLES) + .skip(1) + { let access_method = &access_methods_arena.borrow()[best_plan.data[i].1]; assert!(!access_method.is_scan()); assert!(access_method.iter_dir == IterationDirection::Forwards); assert!(access_method.index.is_none()); assert!(access_method.constraint_refs.len() == 1); - let constraint = &table_constraints[i].constraints - [access_method.constraint_refs[0].constraint_vec_pos]; + let constraint = + &table_constraints.constraints[access_method.constraint_refs[0].constraint_vec_pos]; assert!(constraint.lhs_mask.contains_table(i - 1)); assert!(constraint.operator == ast::Operator::Equals); } @@ -1311,7 +1312,7 @@ mod tests { }, identifier: "t1".to_string(), join_info: None, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }); // Create where clause that only references second column @@ -1402,7 +1403,7 @@ mod tests { }, identifier: "t1".to_string(), join_info: None, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }); // Create where clause that references first and third columns @@ -1518,7 +1519,7 @@ mod tests { }, identifier: "t1".to_string(), join_info: None, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }); // Create where clause: c1 = 5 AND c2 > 10 AND c3 = 7 @@ -1666,7 +1667,7 @@ mod tests { identifier: name, internal_id, join_info, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), } } diff --git a/core/translate/optimizer/lift_common_subexpressions.rs b/core/translate/optimizer/lift_common_subexpressions.rs index af06f0c37..8c6190da6 100644 --- a/core/translate/optimizer/lift_common_subexpressions.rs +++ b/core/translate/optimizer/lift_common_subexpressions.rs @@ -20,6 +20,7 @@ use crate::{ /// 1. (c AND d) OR (e AND f) /// 2. a, /// 3. b, +/// /// where `a` and `b` become separate WhereTerms, and the original WhereTerm /// is updated to `(c AND d) OR (e AND f)`. /// diff --git a/core/translate/optimizer/mod.rs b/core/translate/optimizer/mod.rs index 57d586862..2b9d9b911 100644 --- a/core/translate/optimizer/mod.rs +++ b/core/translate/optimizer/mod.rs @@ -10,7 +10,7 @@ use limbo_sqlite3_parser::{ ast::{self, Expr, SortOrder}, to_sql_string::ToSqlString as _, }; -use order::{compute_order_target, plan_satisfies_order_target, EliminatesSort}; +use order::{compute_order_target, plan_satisfies_order_target, EliminatesSortBy}; use crate::{ parameters::PARAM_PREFIX, @@ -152,14 +152,14 @@ fn optimize_subqueries(plan: &mut SelectPlan, schema: &Schema) -> Result<()> { fn optimize_table_access( table_references: &mut TableReferences, available_indexes: &HashMap>>, - where_clause: &mut Vec, + where_clause: &mut [WhereTerm], order_by: &mut Option>, group_by: &mut Option, ) -> Result>> { let access_methods_arena = RefCell::new(Vec::new()); let maybe_order_target = compute_order_target(order_by, group_by.as_mut()); let constraints_per_table = - constraints_from_where_clause(where_clause, &table_references, available_indexes)?; + constraints_from_where_clause(where_clause, table_references, available_indexes)?; let Some(best_join_order_result) = compute_best_join_order( table_references.joined_tables_mut(), maybe_order_target.as_ref(), @@ -204,13 +204,13 @@ fn optimize_table_access( ); if satisfies_order_target { match order_target.1 { - EliminatesSort::GroupBy => { + EliminatesSortBy::Group => { let _ = group_by.as_mut().and_then(|g| g.sort_order.take()); } - EliminatesSort::OrderBy => { + EliminatesSortBy::Order => { let _ = order_by.take(); } - EliminatesSort::GroupByAndOrderBy => { + EliminatesSortBy::GroupByAndOrder => { let _ = group_by.as_mut().and_then(|g| g.sort_order.take()); let _ = order_by.take(); } @@ -294,14 +294,14 @@ fn optimize_table_access( let ephemeral_index = ephemeral_index_build( &joined_tables[table_idx], &table_constraints.constraints, - &usable_constraint_refs, + usable_constraint_refs, ); let ephemeral_index = Arc::new(ephemeral_index); joined_tables[table_idx].op = Operation::Search(Search::Seek { index: Some(ephemeral_index), seek_def: build_seek_def_from_constraints( &table_constraints.constraints, - &usable_constraint_refs, + usable_constraint_refs, access_method.iter_dir, where_clause, )?, @@ -326,7 +326,7 @@ fn optimize_table_access( index: Some(index.clone()), seek_def: build_seek_def_from_constraints( &constraints_per_table[table_idx].constraints, - &constraint_refs, + constraint_refs, access_method.iter_dir, where_clause, )?, @@ -348,7 +348,7 @@ fn optimize_table_access( index: None, seek_def: build_seek_def_from_constraints( &constraints_per_table[table_idx].constraints, - &constraint_refs, + constraint_refs, access_method.iter_dir, where_clause, )?, @@ -370,7 +370,7 @@ enum ConstantConditionEliminationResult { /// Returns a ConstantEliminationResult indicating whether any predicates are always false. /// This is used to determine whether the query can be aborted early. fn eliminate_constant_conditions( - where_clause: &mut Vec, + where_clause: &mut [WhereTerm], ) -> Result { let mut i = 0; while i < where_clause.len() { @@ -530,7 +530,7 @@ impl Optimizable for ast::Expr { let table_ref = tables.find_joined_table_by_internal_id(*table).unwrap(); let columns = table_ref.columns(); let column = &columns[*column]; - return column.primary_key || column.notnull; + column.primary_key || column.notnull } Expr::RowId { .. } => true, Expr::InList { lhs, rhs, .. } => { @@ -864,11 +864,11 @@ pub fn build_seek_def_from_constraints( /// But to illustrate the general idea, consider the following examples: /// /// 1. For example, having two conditions like (x>10 AND y>20) cannot be used as a valid [SeekKey] GT(x:10, y:20) -/// because the first row greater than (x:10, y:20) might be (x:10, y:21), which does not satisfy the where clause. -/// In this case, only GT(x:10) must be used as the [SeekKey], and rows with y <= 20 must be filtered as a regular condition expression for each value of x. +/// because the first row greater than (x:10, y:20) might be (x:10, y:21), which does not satisfy the where clause. +/// In this case, only GT(x:10) must be used as the [SeekKey], and rows with y <= 20 must be filtered as a regular condition expression for each value of x. /// /// 2. In contrast, having (x=10 AND y>20) forms a valid index key GT(x:10, y:20) because after the seek, we can simply terminate as soon as x > 10, -/// i.e. use GT(x:10, y:20) as the [SeekKey] and GT(x:10) as the [TerminationKey]. +/// i.e. use GT(x:10, y:20) as the [SeekKey] and GT(x:10) as the [TerminationKey]. /// /// The preceding examples are for an ascending index. The logic is similar for descending indexes, but an important distinction is that /// since a descending index is laid out in reverse order, the comparison operators are reversed, e.g. LT becomes GT, LE becomes GE, etc. diff --git a/core/translate/optimizer/order.rs b/core/translate/optimizer/order.rs index ea1692a9e..4caa67d05 100644 --- a/core/translate/optimizer/order.rs +++ b/core/translate/optimizer/order.rs @@ -19,22 +19,22 @@ pub struct ColumnOrder { #[derive(Debug, PartialEq, Clone)] /// If an [OrderTarget] is satisfied, then [EliminatesSort] describes which part of the query no longer requires sorting. -pub enum EliminatesSort { - GroupBy, - OrderBy, - GroupByAndOrderBy, +pub enum EliminatesSortBy { + Group, + Order, + GroupByAndOrder, } #[derive(Debug, PartialEq, Clone)] /// An [OrderTarget] is considered in join optimization and index selection, /// so that if a given join ordering and its access methods satisfy the [OrderTarget], /// then the join ordering and its access methods are preferred, all other things being equal. -pub struct OrderTarget(pub Vec, pub EliminatesSort); +pub struct OrderTarget(pub Vec, pub EliminatesSortBy); impl OrderTarget { fn maybe_from_iterator<'a>( list: impl Iterator + Clone, - eliminates_sort: EliminatesSort, + eliminates_sort: EliminatesSortBy, ) -> Option { if list.clone().count() == 0 { return None; @@ -79,12 +79,12 @@ pub fn compute_order_target( // Only ORDER BY - we would like the joined result rows to be in the order specified by the ORDER BY (Some(order_by), None) => OrderTarget::maybe_from_iterator( order_by.iter().map(|(expr, order)| (expr, *order)), - EliminatesSort::OrderBy, + EliminatesSortBy::Order, ), // Only GROUP BY - we would like the joined result rows to be in the order specified by the GROUP BY (None, Some(group_by)) => OrderTarget::maybe_from_iterator( group_by.exprs.iter().map(|expr| (expr, SortOrder::Asc)), - EliminatesSort::GroupBy, + EliminatesSortBy::Group, ), // Both ORDER BY and GROUP BY: // If the GROUP BY does not contain all the expressions in the ORDER BY, @@ -107,7 +107,7 @@ pub fn compute_order_target( if !group_by_contains_all { return OrderTarget::maybe_from_iterator( group_by.exprs.iter().map(|expr| (expr, SortOrder::Asc)), - EliminatesSort::GroupBy, + EliminatesSortBy::Group, ); } // If yes, let's try to target an ordering that matches the GROUP BY columns, @@ -146,7 +146,7 @@ pub fn compute_order_target( .iter(), ) .map(|(expr, dir)| (expr, *dir)), - EliminatesSort::GroupByAndOrderBy, + EliminatesSortBy::GroupByAndOrder, ) } } diff --git a/core/translate/plan.rs b/core/translate/plan.rs index 697f49911..71b5a1dcc 100644 --- a/core/translate/plan.rs +++ b/core/translate/plan.rs @@ -114,7 +114,7 @@ impl WhereTerm { } fn eval_at(&self, join_order: &[JoinOrderMember]) -> Result { - determine_where_to_eval_term(&self, join_order) + determine_where_to_eval_term(self, join_order) } } @@ -335,7 +335,7 @@ pub enum QueryDestination { }, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub struct JoinOrderMember { /// The internal ID of the[TableReference] pub table_id: TableInternalId, @@ -346,16 +346,6 @@ pub struct JoinOrderMember { pub is_outer: bool, } -impl Default for JoinOrderMember { - fn default() -> Self { - Self { - table_id: TableInternalId::default(), - original_idx: 0, - is_outer: false, - } - } -} - #[derive(Debug, Clone, PartialEq)] /// Whether a column is DISTINCT or not. @@ -414,7 +404,7 @@ impl DistinctCtx { }); program.emit_insn(Insn::IdxInsert { cursor_id: self.cursor_id, - record_reg: record_reg, + record_reg, unpacked_start: None, unpacked_count: None, flags: IdxInsertFlags::new(), @@ -472,7 +462,7 @@ impl SelectPlan { QueryDestination::CoroutineYield { .. } ) || self.table_references.joined_tables().len() != 1 - || self.table_references.outer_query_refs().len() != 0 + || self.table_references.outer_query_refs().is_empty() || self.result_columns.len() != 1 || self.group_by.is_some() || self.contains_constant_false_condition @@ -837,15 +827,11 @@ impl TableReferences { } } -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] #[repr(transparent)] pub struct ColumnUsedMask(u128); impl ColumnUsedMask { - pub fn new() -> Self { - Self(0) - } - pub fn set(&mut self, index: usize) { assert!( index < 128, @@ -950,7 +936,7 @@ impl JoinedTable { identifier, internal_id, join_info, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), } } @@ -987,14 +973,12 @@ impl JoinedTable { CursorType::BTreeTable(btree.clone()), )) }; - let index_cursor_id = if let Some(index) = index { - Some(program.alloc_cursor_id_keyed( + let index_cursor_id = index.map(|index| { + program.alloc_cursor_id_keyed( CursorKey::index(self.internal_id, index.clone()), CursorType::BTreeIndex(index.clone()), - )) - } else { - None - }; + ) + }); Ok((table_cursor_id, index_cursor_id)) } Table::Virtual(virtual_table) => { @@ -1031,7 +1015,7 @@ impl JoinedTable { if self.col_used_mask.is_empty() { return false; } - let mut index_cols_mask = ColumnUsedMask::new(); + let mut index_cols_mask = ColumnUsedMask::default(); for col in index.columns.iter() { index_cols_mask.set(col.pos_in_table); } @@ -1040,7 +1024,7 @@ impl JoinedTable { if btree.has_rowid { if let Some(pos_of_rowid_alias_col) = btree.get_rowid_alias_column().map(|(pos, _)| pos) { - let mut empty_mask = ColumnUsedMask::new(); + let mut empty_mask = ColumnUsedMask::default(); empty_mask.set(pos_of_rowid_alias_col); if self.col_used_mask == empty_mask { // However if the index would be ONLY used for the rowid, then let's not bother using it to cover the query. @@ -1079,6 +1063,7 @@ pub struct SeekDef { /// For example, given: /// - CREATE INDEX i ON t (x, y desc) /// - SELECT * FROM t WHERE x = 1 AND y >= 30 + /// /// The key is [(1, ASC), (30, DESC)] pub key: Vec<(ast::Expr, SortOrder)>, /// The condition to use when seeking. See [SeekKey] for more details. @@ -1100,6 +1085,7 @@ pub struct SeekKey { /// For example, given: /// - CREATE INDEX i ON t (x, y) /// - SELECT * FROM t WHERE x = 1 AND y < 30 + /// /// We want to seek to the first row where x = 1, and then iterate forwards. /// In this case, the seek key is GT(1, NULL) since NULL is always LT in index key comparisons. /// We can't use just GT(1) because in index key comparisons, only the given number of columns are compared, diff --git a/core/translate/planner.rs b/core/translate/planner.rs index 1bf6a7452..09b9d9059 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -240,7 +240,7 @@ pub fn bind_column_references( }) } -fn parse_from_clause_table<'a>( +fn parse_from_clause_table( schema: &Schema, table: ast::SelectTable, table_references: &mut TableReferences, @@ -288,7 +288,7 @@ fn parse_from_clause_table<'a>( identifier: alias.unwrap_or(normalized_qualified_name), internal_id: table_ref_counter.next(), join_info: None, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }); return Ok(()); }; @@ -313,7 +313,7 @@ fn parse_from_clause_table<'a>( identifier: outer_ref.identifier.clone(), internal_id: table_ref_counter.next(), join_info: None, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }); return Ok(()); } @@ -371,7 +371,7 @@ fn parse_from_clause_table<'a>( table: Table::Virtual(vtab), identifier: alias, internal_id: table_ref_counter.next(), - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }); Ok(()) @@ -380,7 +380,7 @@ fn parse_from_clause_table<'a>( } } -pub fn parse_from<'a>( +pub fn parse_from( schema: &Schema, mut from: Option, syms: &SymbolTable, @@ -435,7 +435,7 @@ pub fn parse_from<'a>( identifier: t.identifier.clone(), internal_id: t.internal_id, table: t.table.clone(), - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), } })); @@ -535,7 +535,7 @@ pub fn determine_where_to_eval_term( )); } - return determine_where_to_eval_expr(&term.expr, join_order); + determine_where_to_eval_expr(&term.expr, join_order) } /// A bitmask representing a set of tables in a query plan. @@ -667,8 +667,8 @@ pub fn table_mask_from_expr( Ok(mask) } -pub fn determine_where_to_eval_expr<'a>( - top_level_expr: &'a Expr, +pub fn determine_where_to_eval_expr( + top_level_expr: &Expr, join_order: &[JoinOrderMember], ) -> Result { let mut eval_at: EvalAt = EvalAt::BeforeLoop; @@ -689,7 +689,7 @@ pub fn determine_where_to_eval_expr<'a>( Ok(eval_at) } -fn parse_join<'a>( +fn parse_join( schema: &Schema, join: ast::JoinedSelectTable, syms: &SymbolTable, diff --git a/core/translate/pragma.rs b/core/translate/pragma.rs index afd929725..05d1a87c2 100644 --- a/core/translate/pragma.rs +++ b/core/translate/pragma.rs @@ -29,6 +29,7 @@ fn list_pragmas(program: &mut ProgramBuilder) { program.epilogue(crate::translate::emitter::TransactionMode::None); } +#[allow(clippy::too_many_arguments)] pub fn translate_pragma( query_mode: QueryMode, schema: &Schema, diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index 6c7b85c21..74b071b44 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -17,6 +17,7 @@ use super::{ /// - all result columns /// - result row (or if a subquery, yields to the parent query) /// - limit +#[allow(clippy::too_many_arguments)] pub fn emit_select_result( program: &mut ProgramBuilder, resolver: &Resolver, diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 4770ed976..ffeed36fc 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -800,7 +800,7 @@ pub fn translate_drop_table( }); program.emit_insn(Insn::OpenRead { cursor_id: sqlite_schema_cursor_id_1, - root_page: 1usize.into(), + root_page: 1usize, }); let schema_column_0_register = program.alloc_register(); diff --git a/core/translate/select.rs b/core/translate/select.rs index d3e8c354d..1a194df55 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -88,7 +88,7 @@ pub fn translate_select( }) } -pub fn prepare_select_plan<'a>( +pub fn prepare_select_plan( schema: &Schema, mut select: ast::Select, syms: &SymbolTable, @@ -181,7 +181,8 @@ pub fn prepare_select_plan<'a>( } } -fn prepare_one_select_plan<'a>( +#[allow(clippy::too_many_arguments)] +fn prepare_one_select_plan( schema: &Schema, select: ast::OneSelect, limit: Option<&ast::Limit>, @@ -284,7 +285,7 @@ fn prepare_one_select_plan<'a>( match column { ResultColumn::Star => { select_star( - &plan.table_references.joined_tables(), + plan.table_references.joined_tables(), &mut plan.result_columns, ); for table in plan.table_references.joined_tables_mut() { @@ -574,7 +575,7 @@ fn prepare_one_select_plan<'a>( } // Parse the LIMIT/OFFSET clause - (plan.limit, plan.offset) = limit.map_or(Ok((None, None)), |l| parse_limit(l))?; + (plan.limit, plan.offset) = limit.map_or(Ok((None, None)), parse_limit)?; // Return the unoptimized query plan Ok(plan) @@ -676,13 +677,7 @@ fn estimate_num_instructions(select: &SelectPlan) -> usize { let order_by_instructions = select.order_by.is_some() as usize * 10; let condition_instructions = select.where_clause.len() * 3; - let num_instructions = 20 - + table_instructions - + group_by_instructions - + order_by_instructions - + condition_instructions; - - num_instructions + 20 + table_instructions + group_by_instructions + order_by_instructions + condition_instructions } fn estimate_num_labels(select: &SelectPlan) -> usize { @@ -706,20 +701,17 @@ fn estimate_num_labels(select: &SelectPlan) -> usize { let order_by_labels = select.order_by.is_some() as usize * 10; let condition_labels = select.where_clause.len() * 2; - let num_labels = - init_halt_labels + table_labels + group_by_labels + order_by_labels + condition_labels; - - num_labels + init_halt_labels + table_labels + group_by_labels + order_by_labels + condition_labels } -pub fn emit_simple_count<'a>( +pub fn emit_simple_count( program: &mut ProgramBuilder, - _t_ctx: &mut TranslateCtx<'a>, + _t_ctx: &mut TranslateCtx, plan: &SelectPlan, ) -> Result<()> { let cursors = plan .joined_tables() - .get(0) + .first() .unwrap() .resolve_cursors(program)?; diff --git a/core/translate/subquery.rs b/core/translate/subquery.rs index f6b526922..4004a7cd1 100644 --- a/core/translate/subquery.rs +++ b/core/translate/subquery.rs @@ -44,10 +44,10 @@ pub fn emit_subqueries( /// /// Since a subquery has its own SelectPlan, it can contain nested subqueries, /// which can contain even more nested subqueries, etc. -pub fn emit_subquery<'a>( +pub fn emit_subquery( program: &mut ProgramBuilder, plan: &mut SelectPlan, - t_ctx: &mut TranslateCtx<'a>, + t_ctx: &mut TranslateCtx, ) -> Result { let yield_reg = program.alloc_register(); let coroutine_implementation_start_offset = program.allocate_label(); diff --git a/core/translate/update.rs b/core/translate/update.rs index 822689411..233667a4f 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -144,7 +144,7 @@ pub fn prepare_update_plan( index: None, }, join_info: None, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }]; let mut table_references = TableReferences::new(joined_tables, vec![]); let set_clauses = body @@ -229,7 +229,7 @@ pub fn prepare_update_plan( index: None, }, join_info: None, - col_used_mask: ColumnUsedMask::new(), + col_used_mask: ColumnUsedMask::default(), }]; let mut table_references = TableReferences::new(joined_tables, vec![]); diff --git a/core/translate/values.rs b/core/translate/values.rs index 824738752..a5e0f4142 100644 --- a/core/translate/values.rs +++ b/core/translate/values.rs @@ -39,7 +39,7 @@ fn emit_values_when_single_row( translate_expr_no_constant_opt( program, None, - &v, + v, start_reg + i, resolver, NoConstantOptReason::RegisterReuse, @@ -131,7 +131,7 @@ fn emit_values_in_subquery( translate_expr_no_constant_opt( program, None, - &v, + v, start_reg + i, resolver, NoConstantOptReason::RegisterReuse, diff --git a/core/types.rs b/core/types.rs index d20ec0ac2..81eaed959 100644 --- a/core/types.rs +++ b/core/types.rs @@ -58,6 +58,12 @@ pub struct Text { pub subtype: TextSubtype, } +impl Display for Text { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.as_str()) + } +} + #[derive(Debug, Clone, PartialEq)] pub struct TextRef { pub value: RawSlice, @@ -65,10 +71,6 @@ pub struct TextRef { } impl Text { - pub fn from_str>(value: S) -> Self { - Self::new(&value.into()) - } - pub fn new(value: &str) -> Self { Self { value: value.as_bytes().to_vec(), @@ -84,10 +86,6 @@ 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()) } } @@ -99,6 +97,15 @@ impl AsRef for Text { } } +impl From<&str> for Text { + fn from(value: &str) -> Self { + Text { + value: value.as_bytes().to_vec(), + subtype: TextSubtype::Text, + } + } +} + impl From for Text { fn from(value: String) -> Self { Text { @@ -108,14 +115,16 @@ impl From for Text { } } +impl Display for TextRef { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.as_str()) + } +} + impl TextRef { pub fn as_str(&self) -> &str { unsafe { std::str::from_utf8_unchecked(self.value.to_slice()) } } - - pub fn to_string(&self) -> String { - self.as_str().to_string() - } } #[cfg(feature = "serde")] @@ -331,11 +340,12 @@ impl Display for Value { } else { format!("{}.{}", whole, fraction) }; - let (prefix, exponent) = if exponent.starts_with('-') { - ("-0", &exponent[1..]) - } else { - ("+", exponent) - }; + let (prefix, exponent) = + if let Some(stripped_exponent) = exponent.strip_prefix('-') { + ("-0", &stripped_exponent[1..]) + } else { + ("+", exponent) + }; return write!(f, "{}e{}{}", trimmed_mantissa, prefix, exponent); } } @@ -514,10 +524,6 @@ impl PartialEq for Value { _ => false, } } - - fn ne(&self, other: &Value) -> bool { - !self.eq(other) - } } #[allow(clippy::non_canonical_partial_ord_impl)] @@ -782,6 +788,10 @@ impl Record { pub fn len(&self) -> usize { self.values.len() } + + pub fn is_empty(&self) -> bool { + self.values.is_empty() + } } struct AppendWriter<'a> { buf: &'a mut Vec, @@ -859,6 +869,10 @@ impl ImmutableRecord { self.values.len() } + pub fn is_empty(&self) -> bool { + self.values.is_empty() + } + pub fn from_registers<'a>( registers: impl IntoIterator + Copy, len: usize, @@ -1152,7 +1166,7 @@ impl PartialOrd for RefValue { /// A bitfield that represents the comparison spec for index keys. /// Since indexed columns can individually specify ASC/DESC, each key must /// be compared differently. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] #[repr(transparent)] pub struct IndexKeySortOrder(u64); @@ -1180,16 +1194,6 @@ impl IndexKeySortOrder { } IndexKeySortOrder(spec) } - - pub fn default() -> Self { - Self(0) - } -} - -impl Default for IndexKeySortOrder { - fn default() -> Self { - Self::default() - } } #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] @@ -1715,8 +1719,6 @@ mod tests { assert_eq!( buf.len(), header_length // 9 bytes (header size + 8 serial types) - + 0 // ConstInt0: 0 bytes - + 0 // ConstInt1: 0 bytes + size_of::() // I8: 1 byte + size_of::() // I16: 2 bytes + (size_of::() - 1) // I24: 3 bytes diff --git a/core/util.rs b/core/util.rs index 49c13a9f4..6537017f6 100644 --- a/core/util.rs +++ b/core/util.rs @@ -147,7 +147,7 @@ pub fn parse_schema_rows( .unwrap(); let index = schema::Index::from_sql( &unparsed_sql_from_index.sql, - unparsed_sql_from_index.root_page as usize, + unparsed_sql_from_index.root_page, table.as_ref(), )?; schema.add_index(Arc::new(index)); @@ -432,7 +432,7 @@ pub fn exprs_are_equivalent(expr1: &Expr, expr2: &Expr) -> bool { } // Variables that are not bound to a specific value, are treated as NULL // https://sqlite.org/lang_expr.html#varparam - (Expr::Variable(var), Expr::Variable(var2)) if var == "" && var2 == "" => false, + (Expr::Variable(var), Expr::Variable(var2)) if var.is_empty() && var2.is_empty() => false, // Named variables can be compared by their name (Expr::Variable(val), Expr::Variable(val2)) => val == val2, (Expr::Parenthesized(exprs1), Expr::Parenthesized(exprs2)) => { @@ -1366,7 +1366,7 @@ pub mod tests { assert_eq!(opts.path, "/home/user/db.sqlite"); assert_eq!(opts.vfs, Some("unix".to_string())); assert_eq!(opts.mode, OpenMode::ReadOnly); - assert_eq!(opts.immutable, true); + assert!(opts.immutable); } #[test] @@ -1448,7 +1448,7 @@ pub mod tests { assert_eq!(opts.vfs, Some("unix".to_string())); assert_eq!(opts.mode, OpenMode::ReadWrite); assert_eq!(opts.cache, CacheMode::Private); - assert_eq!(opts.immutable, false); + assert!(!opts.immutable); } #[test] @@ -2012,12 +2012,12 @@ pub mod tests { // > i64::MAX, convert to float assert_eq!( parse_numeric_literal("9223372036854775808").unwrap(), - Value::Float(9.223372036854775808e+18) + Value::Float(9.223_372_036_854_776e18) ); // < i64::MIN, convert to float assert_eq!( parse_numeric_literal("-9223372036854775809").unwrap(), - Value::Float(-9.223372036854775809e+18) + Value::Float(-9.223_372_036_854_776e18) ); } } diff --git a/core/vdbe/builder.rs b/core/vdbe/builder.rs index e90f6d736..a87a5d982 100644 --- a/core/vdbe/builder.rs +++ b/core/vdbe/builder.rs @@ -14,9 +14,10 @@ use crate::{ emitter::TransactionMode, plan::{ResultSetColumn, TableReferences}, }, - types::Text, Connection, Value, VirtualTable, }; + +#[derive(Default)] pub struct TableRefIdCounter { next_free: TableInternalId, } @@ -356,7 +357,7 @@ impl ProgramBuilder { pub fn add_comment(&mut self, insn_index: BranchOffset, comment: &'static str) { if let Some(comments) = &mut self.comments { - comments.push((insn_index.to_offset_int(), comment)); + comments.push((insn_index.as_offset_int(), comment)); } } @@ -387,8 +388,8 @@ impl ProgramBuilder { .constant_spans .iter() .find(|span| span.0 <= *index_b && span.1 >= *index_b); - if a_span.is_some() && b_span.is_some() { - a_span.unwrap().0.cmp(&b_span.unwrap().0) + if let (Some(a_span), Some(b_span)) = (a_span, b_span) { + a_span.0.cmp(&b_span.0) } else if a_span.is_some() { Ordering::Greater } else if b_span.is_some() { @@ -466,7 +467,7 @@ impl ProgramBuilder { unreachable!("Label is not a label"); }; self.label_to_resolved_offset[label_number as usize] = - Some((to_offset.to_offset_int(), target)); + Some((to_offset.as_offset_int(), target)); } /// Resolve unresolved labels to a specific offset in the instruction list. @@ -730,7 +731,7 @@ impl ProgramBuilder { } /// Initialize the program with basic setup and return initial metadata and labels - pub fn prologue<'a>(&mut self) { + pub fn prologue(&mut self) { if self.nested_level == 0 { self.init_label = self.allocate_label(); @@ -823,7 +824,7 @@ impl ProgramBuilder { Numeric::Float(v) => Value::Float(v.into()), }, ast::Literal::Null => Value::Null, - ast::Literal::String(s) => Value::Text(Text::from_str(sanitize_string(s))), + ast::Literal::String(s) => Value::Text(sanitize_string(s).into()), ast::Literal::Blob(s) => Value::Blob( // Taken from `translate_expr` s.as_bytes() diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 8baad9624..857386450 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -125,7 +125,7 @@ pub fn op_init( unreachable!("unexpected Insn {:?}", insn) }; assert!(target_pc.is_offset()); - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); Ok(InsnFunctionStepResult::Step) } @@ -216,7 +216,7 @@ pub fn op_drop_index( unreachable!("unexpected Insn {:?}", insn) }; let mut schema = program.connection.schema.write(); - schema.remove_index(&index); + schema.remove_index(index); state.pc += 1; Ok(InsnFunctionStepResult::Step) } @@ -448,7 +448,7 @@ pub fn op_jump( std::cmp::Ordering::Equal => *target_pc_eq, std::cmp::Ordering::Greater => *target_pc_gt, }; - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); Ok(InsnFunctionStepResult::Step) } @@ -500,7 +500,7 @@ pub fn op_if_pos( let target_pc = *target_pc; match state.registers[reg].get_owned_value() { Value::Integer(n) if *n > 0 => { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); state.registers[reg] = Register::Value(Value::Integer(*n - *decrement_by as i64)); } Value::Integer(_) => { @@ -533,7 +533,7 @@ pub fn op_not_null( state.pc += 1; } _ => { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } } Ok(InsnFunctionStepResult::Step) @@ -709,7 +709,7 @@ pub fn op_comparison( // Fast path for integers if matches!(lhs_value, Value::Integer(_)) && matches!(rhs_value, Value::Integer(_)) { if op.compare_integers(lhs_value, rhs_value) { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -719,7 +719,7 @@ pub fn op_comparison( // Handle NULL values if matches!(lhs_value, Value::Null) || matches!(rhs_value, Value::Null) { if op.handle_nulls(lhs_value, rhs_value, nulleq, jump_if_null) { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -801,7 +801,7 @@ pub fn op_comparison( } if should_jump { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -829,7 +829,7 @@ pub fn op_if( .get_owned_value() .exec_if(*jump_if_null, false) { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -856,7 +856,7 @@ pub fn op_if_not( .get_owned_value() .exec_if(*jump_if_null, true) { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -903,7 +903,7 @@ pub fn op_open_read( let schema = conn.schema.try_read().ok_or(LimboError::SchemaLocked)?; let table = schema .get_table(&index.table_name) - .map_or(None, |table| table.btree()); + .and_then(|table| table.btree()); let collations = table.map_or(Vec::new(), |table| { index .columns @@ -1043,7 +1043,7 @@ pub fn op_vfilter( cursor.filter(*idx_num as i32, idx_str, *arg_count, args)? }; if !has_rows { - state.pc = pc_if_empty.to_offset_int(); + state.pc = pc_if_empty.as_offset_int(); } else { state.pc += 1; } @@ -1157,7 +1157,7 @@ pub fn op_vnext( cursor.next()? }; if has_more { - state.pc = pc_if_next.to_offset_int(); + state.pc = pc_if_next.as_offset_int(); } else { state.pc += 1; } @@ -1205,7 +1205,7 @@ pub fn op_open_pseudo( }; { let mut cursors = state.cursors.borrow_mut(); - let cursor = PseudoCursor::new(); + let cursor = PseudoCursor::default(); cursors .get_mut(*cursor_id) .unwrap() @@ -1237,7 +1237,7 @@ pub fn op_rewind( cursor.is_empty() }; if is_empty { - state.pc = pc_if_empty.to_offset_int(); + state.pc = pc_if_empty.as_offset_int(); } else { state.pc += 1; } @@ -1266,7 +1266,7 @@ pub fn op_last( cursor.is_empty() }; if is_empty { - state.pc = pc_if_empty.to_offset_int(); + state.pc = pc_if_empty.as_offset_int(); } else { state.pc += 1; } @@ -1407,7 +1407,7 @@ pub fn op_type_check( else { unreachable!("unexpected Insn {:?}", insn) }; - assert_eq!(table_reference.is_strict, true); + assert!(table_reference.is_strict); state.registers[*start_reg..*start_reg + *count] .iter_mut() .zip(table_reference.columns.iter()) @@ -1420,7 +1420,7 @@ pub fn op_type_check( bail_constraint_error!( "NOT NULL constraint failed: {}.{} ({})", &table_reference.name, - col.name.as_ref().map(|s| s.as_str()).unwrap_or(""), + col.name.as_deref().unwrap_or(""), SQLITE_CONSTRAINT ) } else if col.is_rowid_alias && matches!(reg.get_owned_value(), Value::Null) { @@ -1442,7 +1442,7 @@ pub fn op_type_check( v, t, &table_reference.name, - col.name.as_ref().map(|s| s.as_str()).unwrap_or(""), + col.name.as_deref().unwrap_or(""), SQLITE_CONSTRAINT ), }; @@ -1518,7 +1518,7 @@ pub fn op_next( cursor.is_empty() }; if !is_empty { - state.pc = pc_if_next.to_offset_int(); + state.pc = pc_if_next.as_offset_int(); } else { state.pc += 1; } @@ -1549,7 +1549,7 @@ pub fn op_prev( cursor.is_empty() }; if !is_empty { - state.pc = pc_if_prev.to_offset_int(); + state.pc = pc_if_prev.as_offset_int(); } else { state.pc += 1; } @@ -1662,7 +1662,7 @@ pub fn op_halt_if_null( unreachable!("unexpected Insn {:?}", insn) }; if state.registers[*target_reg].get_owned_value() == &Value::Null { - halt(program, state, pager, mv_store, *err_code, &description) + halt(program, state, pager, mv_store, *err_code, description) } else { state.pc += 1; Ok(InsnFunctionStepResult::Step) @@ -1765,13 +1765,13 @@ pub fn op_auto_commit( "cannot commit - no transaction is active".to_string(), )); } - return match program.commit_txn(pager.clone(), state, mv_store)? { + match program.commit_txn(pager.clone(), state, mv_store)? { super::StepResult::Done => Ok(InsnFunctionStepResult::Done), super::StepResult::IO => Ok(InsnFunctionStepResult::IO), super::StepResult::Row => Ok(InsnFunctionStepResult::Row), super::StepResult::Interrupt => Ok(InsnFunctionStepResult::Interrupt), super::StepResult::Busy => Ok(InsnFunctionStepResult::Busy), - }; + } } pub fn op_goto( @@ -1785,7 +1785,7 @@ pub fn op_goto( unreachable!("unexpected Insn {:?}", insn) }; assert!(target_pc.is_offset()); - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); Ok(InsnFunctionStepResult::Step) } @@ -1805,7 +1805,7 @@ pub fn op_gosub( }; assert!(target_pc.is_offset()); state.registers[*return_reg] = Register::Value(Value::Integer((state.pc + 1) as i64)); - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); Ok(InsnFunctionStepResult::Step) } @@ -1933,10 +1933,11 @@ pub fn op_row_data( let cursor = cursor_ref.as_btree_mut(); let record_option = return_if_io!(cursor.record()); - let ret = record_option - .ok_or_else(|| LimboError::InternalError("RowData: cursor has no record".to_string()))? - .clone(); - ret + let record = record_option.ok_or_else(|| { + LimboError::InternalError("RowData: cursor has no record".to_string()) + })?; + + record.clone() }; let reg = &mut state.registers[*dest]; @@ -1989,7 +1990,7 @@ pub fn op_row_id( let mut cursors = state.cursors.borrow_mut(); if let Some(Cursor::BTree(btree_cursor)) = cursors.get_mut(*cursor_id).unwrap() { if let Some(ref rowid) = return_if_io!(btree_cursor.rowid()) { - state.registers[*dest] = Register::Value(Value::Integer(*rowid as i64)); + state.registers[*dest] = Register::Value(Value::Integer(*rowid)); } else { state.registers[*dest] = Register::Value(Value::Null); } @@ -2024,7 +2025,7 @@ pub fn op_idx_row_id( let cursor = cursor.as_btree_mut(); let rowid = return_if_io!(cursor.rowid()); state.registers[*dest] = match rowid { - Some(rowid) => Register::Value(Value::Integer(rowid as i64)), + Some(rowid) => Register::Value(Value::Integer(rowid)), None => Register::Value(Value::Null), }; state.pc += 1; @@ -2075,12 +2076,12 @@ pub fn op_seek_rowid( cursor.seek(SeekKey::TableRowId(rowid), SeekOp::GE { eq_only: true }) ); if !found { - target_pc.to_offset_int() + target_pc.as_offset_int() } else { state.pc + 1 } } - None => target_pc.to_offset_int(), + None => target_pc.as_offset_int(), } }; state.pc = pc; @@ -2173,11 +2174,10 @@ pub fn op_seek( let mut cursor = state.get_cursor(*cursor_id); let cursor = cursor.as_btree_mut(); let record_from_regs = make_record(&state.registers, start_reg, num_regs); - let found = return_if_io!(cursor.seek(SeekKey::IndexKey(&record_from_regs), op)); - found + return_if_io!(cursor.seek(SeekKey::IndexKey(&record_from_regs), op)) }; if !found { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -2209,29 +2209,25 @@ pub fn op_seek( 0 }; - if c > 0 { - // If approximation is larger than actual search term - match op { - SeekOp::GT => SeekOp::GE { eq_only: false }, // (x > 4.9) -> (x >= 5) - SeekOp::LE { .. } => SeekOp::LT, // (x <= 4.9) -> (x < 5) - other => other, - } - } else if c < 0 { - // If approximation is smaller than actual search term - match op { + match c.cmp(&0) { + std::cmp::Ordering::Less => match op { SeekOp::LT => SeekOp::LE { eq_only: false }, // (x < 5.1) -> (x <= 5) SeekOp::GE { .. } => SeekOp::GT, // (x >= 5.1) -> (x > 5) other => other, - } - } else { - op + }, + std::cmp::Ordering::Greater => match op { + SeekOp::GT => SeekOp::GE { eq_only: false }, // (x > 4.9) -> (x >= 5) + SeekOp::LE { .. } => SeekOp::LT, // (x <= 4.9) -> (x < 5) + other => other, + }, + std::cmp::Ordering::Equal => op, } } Value::Text(_) | Value::Blob(_) => { match op { SeekOp::GT | SeekOp::GE { .. } => { // No integers are > or >= non-numeric text, jump to target (empty result) - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); return Ok(InsnFunctionStepResult::Step); } SeekOp::LT | SeekOp::LE { .. } => { @@ -2256,12 +2252,12 @@ pub fn op_seek( let rowid = if matches!(original_value, Value::Null) { match actual_op { SeekOp::GE { .. } | SeekOp::GT => { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); return Ok(InsnFunctionStepResult::Step); } SeekOp::LE { .. } | SeekOp::LT => { // No integers are < NULL, so jump to target - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); return Ok(InsnFunctionStepResult::Step); } } @@ -2273,7 +2269,7 @@ pub fn op_seek( let found = return_if_io!(cursor.seek(SeekKey::TableRowId(rowid), actual_op)); if !found { - target_pc.to_offset_int() + target_pc.as_offset_int() } else { state.pc + 1 } @@ -2310,18 +2306,18 @@ pub fn op_idx_ge( let idx_values = &idx_values[..record_from_regs.len()]; let record_values = record_from_regs.get_values(); let ord = compare_immutable( - &idx_values, - &record_values, + idx_values, + record_values, cursor.key_sort_order(), cursor.collations(), ); if ord.is_ge() { - target_pc.to_offset_int() + target_pc.as_offset_int() } else { state.pc + 1 } } else { - target_pc.to_offset_int() + target_pc.as_offset_int() }; pc }; @@ -2374,18 +2370,18 @@ pub fn op_idx_le( let idx_values = &idx_values[..record_from_regs.len()]; let record_values = record_from_regs.get_values(); let ord = compare_immutable( - &idx_values, - &record_values, + idx_values, + record_values, cursor.key_sort_order(), cursor.collations(), ); if ord.is_le() { - target_pc.to_offset_int() + target_pc.as_offset_int() } else { state.pc + 1 } } else { - target_pc.to_offset_int() + target_pc.as_offset_int() }; pc }; @@ -2420,18 +2416,18 @@ pub fn op_idx_gt( let idx_values = &idx_values[..record_from_regs.len()]; let record_values = record_from_regs.get_values(); let ord = compare_immutable( - &idx_values, - &record_values, + idx_values, + record_values, cursor.key_sort_order(), cursor.collations(), ); if ord.is_gt() { - target_pc.to_offset_int() + target_pc.as_offset_int() } else { state.pc + 1 } } else { - target_pc.to_offset_int() + target_pc.as_offset_int() }; pc }; @@ -2466,18 +2462,18 @@ pub fn op_idx_lt( let idx_values = &idx_values[..record_from_regs.len()]; let record_values = record_from_regs.get_values(); let ord = compare_immutable( - &idx_values, - &record_values, + idx_values, + record_values, cursor.key_sort_order(), cursor.collations(), ); if ord.is_lt() { - target_pc.to_offset_int() + target_pc.as_offset_int() } else { state.pc + 1 } } else { - target_pc.to_offset_int() + target_pc.as_offset_int() }; pc }; @@ -2501,7 +2497,7 @@ pub fn op_decr_jump_zero( let n = n - 1; state.registers[*reg] = Register::Value(Value::Integer(n)); if n == 0 { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -2663,7 +2659,7 @@ pub fn op_agg_step( } (Some(Value::Integer(ref mut current_max)), Value::Integer(value)) => { if *value > *current_max { - *current_max = value.clone(); + *current_max = *value; } } (Some(Value::Float(ref mut current_max)), Value::Float(value)) => { @@ -2749,8 +2745,8 @@ pub fn op_agg_step( unreachable!(); }; - let mut key_vec = convert_dbtype_to_raw_jsonb(&key.get_owned_value())?; - let mut val_vec = convert_dbtype_to_raw_jsonb(&value.get_owned_value())?; + let mut key_vec = convert_dbtype_to_raw_jsonb(key.get_owned_value())?; + let mut val_vec = convert_dbtype_to_raw_jsonb(value.get_owned_value())?; match acc { Value::Blob(vec) => { @@ -2777,7 +2773,7 @@ pub fn op_agg_step( unreachable!(); }; - let mut data = convert_dbtype_to_raw_jsonb(&col.get_owned_value())?; + let mut data = convert_dbtype_to_raw_jsonb(col.get_owned_value())?; match acc { Value::Blob(vec) => { if vec.is_empty() { @@ -2993,7 +2989,7 @@ pub fn op_sorter_data( let record = { let mut cursor = state.get_cursor(*cursor_id); let cursor = cursor.as_sorter_mut(); - cursor.record().map(|r| r.clone()) + cursor.record().cloned() }; let record = match record { Some(record) => record, @@ -3062,7 +3058,7 @@ pub fn op_sorter_sort( is_empty }; if is_empty { - state.pc = pc_if_empty.to_offset_int(); + state.pc = pc_if_empty.as_offset_int(); } else { state.pc += 1; } @@ -3091,7 +3087,7 @@ pub fn op_sorter_next( cursor.has_more() }; if has_more { - state.pc = pc_if_next.to_offset_int(); + state.pc = pc_if_next.as_offset_int(); } else { state.pc += 1; } @@ -3455,7 +3451,7 @@ pub fn op_function( } ScalarFunc::LastInsertRowid => { state.registers[*dest] = - Register::Value(Value::Integer(program.connection.last_insert_rowid() as i64)); + Register::Value(Value::Integer(program.connection.last_insert_rowid())); } ScalarFunc::Like => { let pattern = &state.registers[*start_reg]; @@ -3676,13 +3672,13 @@ pub fn op_function( ScalarFunc::UnixEpoch => { if *start_reg == 0 { let unixepoch: String = exec_unixepoch(&Value::build_text("now"))?; - state.registers[*dest] = Register::Value(Value::build_text(&unixepoch)); + state.registers[*dest] = Register::Value(Value::build_text(unixepoch)); } else { let datetime_value = &state.registers[*start_reg]; let unixepoch = exec_unixepoch(datetime_value.get_owned_value()); match unixepoch { Ok(time) => { - state.registers[*dest] = Register::Value(Value::build_text(&time)) + state.registers[*dest] = Register::Value(Value::build_text(time)) } Err(e) => { return Err(LimboError::ParseError(format!( @@ -3696,7 +3692,7 @@ pub fn op_function( ScalarFunc::SqliteVersion => { let version_integer: i64 = DATABASE_VERSION.get().unwrap().parse()?; let version = execute_sqlite_version(version_integer); - state.registers[*dest] = Register::Value(Value::build_text(&version)); + state.registers[*dest] = Register::Value(Value::build_text(version)); } ScalarFunc::SqliteSourceId => { let src_id = format!( @@ -3704,7 +3700,7 @@ pub fn op_function( info::build::BUILT_TIME_SQLITE, info::build::GIT_COMMIT_HASH.unwrap_or("unknown") ); - state.registers[*dest] = Register::Value(Value::build_text(&src_id)); + state.registers[*dest] = Register::Value(Value::build_text(src_id)); } ScalarFunc::Replace => { assert_eq!(arg_count, 3); @@ -3854,7 +3850,7 @@ pub fn op_function( }, }, crate::function::Func::AlterTable(alter_func) => { - let r#type = &state.registers[*start_reg + 0].get_owned_value().clone(); + let r#type = &state.registers[*start_reg].get_owned_value().clone(); let Value::Text(name) = &state.registers[*start_reg + 1].get_owned_value() else { panic!("sqlite_schema.name should be TEXT") @@ -4029,7 +4025,7 @@ pub fn op_function( for column in &mut columns { match &mut column.expr { ast::Expr::Id(ast::Id(id)) - if normalize_ident(&id) == rename_from => + if normalize_ident(id) == rename_from => { *id = rename_to.clone(); } @@ -4111,7 +4107,7 @@ pub fn op_function( } }; - state.registers[*dest + 0] = Register::Value(r#type.clone()); + state.registers[*dest] = Register::Value(r#type.clone()); state.registers[*dest + 1] = Register::Value(Value::Text(Text::from(new_name))); state.registers[*dest + 2] = Register::Value(Value::Text(Text::from(new_tbl_name))); state.registers[*dest + 3] = Register::Value(Value::Integer(*root_page)); @@ -4146,10 +4142,10 @@ pub fn op_init_coroutine( unreachable!("unexpected Insn {:?}", insn) }; assert!(jump_on_definition.is_offset()); - let start_offset = start_offset.to_offset_int(); + let start_offset = start_offset.as_offset_int(); state.registers[*yield_reg] = Register::Value(Value::Integer(start_offset as i64)); state.ended_coroutine.unset(*yield_reg); - let jump_on_definition = jump_on_definition.to_offset_int(); + let jump_on_definition = jump_on_definition.as_offset_int(); state.pc = if jump_on_definition == 0 { state.pc + 1 } else { @@ -4196,7 +4192,7 @@ pub fn op_yield( }; if let Value::Integer(pc) = state.registers[*yield_reg].get_owned_value() { if state.ended_coroutine.get(*yield_reg) { - state.pc = end_offset.to_offset_int(); + state.pc = end_offset.as_offset_int(); } else { let pc: u32 = (*pc) .try_into() @@ -4347,7 +4343,7 @@ pub fn op_idx_delete( let mut cursor = state.get_cursor(*cursor_id); let cursor = cursor.as_btree_mut(); let found = return_if_io!( - cursor.seek(SeekKey::IndexKey(&record), SeekOp::GE { eq_only: true }) + cursor.seek(SeekKey::IndexKey(record), SeekOp::GE { eq_only: true }) ); tracing::debug!( "op_idx_delete: found={:?}, rootpage={}, key={:?}", @@ -4432,33 +4428,31 @@ pub fn op_idx_insert( // a write/balancing operation. If it did, it means we already moved to the place we wanted. let moved_before = if cursor.is_write_in_progress() { true + } else if index_meta.unique { + // check for uniqueness violation + match cursor.key_exists_in_index(record)? { + CursorResult::Ok(true) => { + return Err(LimboError::Constraint( + "UNIQUE constraint failed: duplicate key".into(), + )) + } + CursorResult::IO => return Ok(InsnFunctionStepResult::IO), + CursorResult::Ok(false) => {} + }; + // uniqueness check already moved us to the correct place in the index. + // the uniqueness check uses SeekOp::GE, which means a non-matching entry + // will now be positioned at the insertion point where there currently is + // a) nothing, or + // b) the first entry greater than the key we are inserting. + // In both cases, we can insert the new entry without moving again. + // + // This is re-entrant, because once we call cursor.insert() with moved_before=true, + // we will immediately set BTreeCursor::state to CursorState::Write(WriteInfo::new()), + // in BTreeCursor::insert_into_page; thus, if this function is called again, + // moved_before will again be true due to cursor.is_write_in_progress() returning true. + true } else { - if index_meta.unique { - // check for uniqueness violation - match cursor.key_exists_in_index(record)? { - CursorResult::Ok(true) => { - return Err(LimboError::Constraint( - "UNIQUE constraint failed: duplicate key".into(), - )) - } - CursorResult::IO => return Ok(InsnFunctionStepResult::IO), - CursorResult::Ok(false) => {} - }; - // uniqueness check already moved us to the correct place in the index. - // the uniqueness check uses SeekOp::GE, which means a non-matching entry - // will now be positioned at the insertion point where there currently is - // a) nothing, or - // b) the first entry greater than the key we are inserting. - // In both cases, we can insert the new entry without moving again. - // - // This is re-entrant, because once we call cursor.insert() with moved_before=true, - // we will immediately set BTreeCursor::state to CursorState::Write(WriteInfo::new()), - // in BTreeCursor::insert_into_page; thus, if this function is called again, - // moved_before will again be true due to cursor.is_write_in_progress() returning true. - true - } else { - flags.has(IdxInsertFlags::USE_SEEK) - } + flags.has(IdxInsertFlags::USE_SEEK) }; // Start insertion of row. This might trigger a balance procedure which will take care of moving to different pages, @@ -4489,8 +4483,7 @@ pub fn op_new_rowid( let mut cursor = state.get_cursor(*cursor); let cursor = cursor.as_btree_mut(); // TODO: make io handle rng - let rowid = return_if_io!(get_new_rowid(cursor, thread_rng())); - rowid + return_if_io!(get_new_rowid(cursor, thread_rng())) }; state.registers[*rowid_reg] = Register::Value(Value::Integer(rowid)); state.pc += 1; @@ -4567,15 +4560,14 @@ pub fn op_no_conflict( let cursor = cursor_ref.as_btree_mut(); let record = if *num_regs == 0 { - let record = match &state.registers[*record_reg] { + match &state.registers[*record_reg] { Register::Record(r) => r, _ => { return Err(LimboError::InternalError( "NoConflict: exepected a record in the register".into(), )); } - }; - record + } } else { &make_record(&state.registers, record_reg, num_regs) }; @@ -4587,7 +4579,7 @@ pub fn op_no_conflict( if contains_nulls { drop(cursor_ref); - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); return Ok(InsnFunctionStepResult::Step); } @@ -4595,7 +4587,7 @@ pub fn op_no_conflict( return_if_io!(cursor.seek(SeekKey::IndexKey(record), SeekOp::GE { eq_only: true })); drop(cursor_ref); if !conflict { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -4620,13 +4612,12 @@ pub fn op_not_exists( let exists = { let mut cursor = must_be_btree_cursor!(*cursor, program.cursor_ref, state, "NotExists"); let cursor = cursor.as_btree_mut(); - let exists = return_if_io!(cursor.exists(state.registers[*rowid_reg].get_owned_value())); - exists + return_if_io!(cursor.exists(state.registers[*rowid_reg].get_owned_value())) }; if exists { state.pc += 1; } else { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } Ok(InsnFunctionStepResult::Step) } @@ -4724,7 +4715,7 @@ pub fn op_open_write( let schema = conn.schema.try_read().ok_or(LimboError::SchemaLocked)?; let table = schema .get_table(&index.table_name) - .map_or(None, |table| table.btree()); + .and_then(|table| table.btree()); let collations = table.map_or(Vec::new(), |table| { index .columns @@ -4883,7 +4874,7 @@ pub fn op_is_null( unreachable!("unexpected Insn {:?}", insn) }; if matches!(state.registers[*reg], Register::Value(Value::Null)) { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -5016,17 +5007,17 @@ pub fn op_set_cookie( Cookie::UserVersion => { let mut header_guard = pager.db_header.lock(); header_guard.user_version = *value; - pager.write_database_header(&*header_guard)?; + pager.write_database_header(&header_guard)?; } Cookie::LargestRootPageNumber => { let mut header_guard = pager.db_header.lock(); header_guard.vacuum_mode_largest_root_page = *value as u32; - pager.write_database_header(&*header_guard)?; + pager.write_database_header(&header_guard)?; } Cookie::IncrementalVacuum => { let mut header_guard = pager.db_header.lock(); header_guard.incremental_vacuum_enabled = *value as u32; - pager.write_database_header(&*header_guard)?; + pager.write_database_header(&header_guard)?; } cookie => todo!("{cookie:?} is not yet implement for SetCookie"), } @@ -5137,7 +5128,7 @@ pub fn op_concat( state.registers[*dest] = Register::Value( state.registers[*lhs] .get_owned_value() - .exec_concat(&state.registers[*rhs].get_owned_value()), + .exec_concat(state.registers[*rhs].get_owned_value()), ); state.pc += 1; Ok(InsnFunctionStepResult::Step) @@ -5156,7 +5147,7 @@ pub fn op_and( state.registers[*dest] = Register::Value( state.registers[*lhs] .get_owned_value() - .exec_and(&state.registers[*rhs].get_owned_value()), + .exec_and(state.registers[*rhs].get_owned_value()), ); state.pc += 1; Ok(InsnFunctionStepResult::Step) @@ -5175,7 +5166,7 @@ pub fn op_or( state.registers[*dest] = Register::Value( state.registers[*lhs] .get_owned_value() - .exec_or(&state.registers[*rhs].get_owned_value()), + .exec_or(state.registers[*rhs].get_owned_value()), ); state.pc += 1; Ok(InsnFunctionStepResult::Step) @@ -5319,7 +5310,7 @@ pub fn op_once( assert!(target_pc_when_reentered.is_offset()); let offset = state.pc; if state.once.iter().any(|o| o == offset) { - state.pc = target_pc_when_reentered.to_offset_int(); + state.pc = target_pc_when_reentered.as_offset_int(); return Ok(InsnFunctionStepResult::Step); } state.once.push(offset); @@ -5366,7 +5357,7 @@ pub fn op_found( } }; - return_if_io!(cursor.seek(SeekKey::IndexKey(&record), SeekOp::GE { eq_only: true })) + return_if_io!(cursor.seek(SeekKey::IndexKey(record), SeekOp::GE { eq_only: true })) } else { let record = make_record(&state.registers, record_reg, num_regs); return_if_io!(cursor.seek(SeekKey::IndexKey(&record), SeekOp::GE { eq_only: true })) @@ -5375,7 +5366,7 @@ pub fn op_found( let do_jump = (!found && not) || (found && !not); if do_jump { - state.pc = target_pc.to_offset_int(); + state.pc = target_pc.as_offset_int(); } else { state.pc += 1; } @@ -5436,8 +5427,7 @@ pub fn op_count( let count = { let mut cursor = must_be_btree_cursor!(*cursor_id, program.cursor_ref, state, "Count"); let cursor = cursor.as_btree_mut(); - let count = return_if_io!(cursor.count()); - count + return_if_io!(cursor.count()) }; state.registers[*target_reg] = Register::Value(Value::Integer(count as i64)); @@ -5511,7 +5501,7 @@ pub fn op_integrity_check( impl Value { pub fn exec_lower(&self) -> Option { match self { - Value::Text(t) => Some(Value::build_text(&t.as_str().to_lowercase())), + Value::Text(t) => Some(Value::build_text(t.as_str().to_lowercase())), t => Some(t.to_owned()), } } @@ -5538,7 +5528,7 @@ impl Value { pub fn exec_upper(&self) -> Option { match self { - Value::Text(t) => Some(Value::build_text(&t.as_str().to_uppercase())), + Value::Text(t) => Some(Value::build_text(t.as_str().to_uppercase())), t => Some(t.to_owned()), } } @@ -5670,7 +5660,7 @@ impl Value { // Retain the first 4 characters and convert to uppercase result.truncate(4); - Value::build_text(&result.to_uppercase()) + Value::build_text(result.to_uppercase()) } pub fn exec_abs(&self) -> Result { @@ -5735,7 +5725,7 @@ impl Value { } } quoted.push('\''); - Value::build_text("ed) + Value::build_text(quoted) } } } @@ -5832,9 +5822,9 @@ impl Value { match self { Value::Text(_) | Value::Integer(_) | Value::Float(_) => { let text = self.to_string(); - Value::build_text(&hex::encode_upper(text)) + Value::build_text(hex::encode_upper(text)) } - Value::Blob(blob_bytes) => Value::build_text(&hex::encode_upper(blob_bytes)), + Value::Blob(blob_bytes) => Value::build_text(hex::encode_upper(blob_bytes)), _ => Value::Null, } } @@ -5992,7 +5982,7 @@ impl Value { Affinity::Text => { // Convert everything to text representation // TODO: handle encoding and whatever sqlite3_snprintf does - Value::build_text(&self.to_string()) + Value::build_text(self.to_string()) } Affinity::Real => match self { Value::Blob(b) => { @@ -6069,7 +6059,7 @@ impl Value { let result = source .as_str() .replace(pattern.as_str(), replacement.as_str()); - Value::build_text(&result) + Value::build_text(result) } _ => unreachable!("text cast should never fail"), } @@ -6255,31 +6245,31 @@ impl Value { pub fn exec_concat(&self, rhs: &Value) -> Value { match (self, rhs) { (Value::Text(lhs_text), Value::Text(rhs_text)) => { - Value::build_text(&(lhs_text.as_str().to_string() + rhs_text.as_str())) + Value::build_text(lhs_text.as_str().to_string() + rhs_text.as_str()) } (Value::Text(lhs_text), Value::Integer(rhs_int)) => { - Value::build_text(&(lhs_text.as_str().to_string() + &rhs_int.to_string())) + Value::build_text(lhs_text.as_str().to_string() + &rhs_int.to_string()) } (Value::Text(lhs_text), Value::Float(rhs_float)) => { - Value::build_text(&(lhs_text.as_str().to_string() + &rhs_float.to_string())) + Value::build_text(lhs_text.as_str().to_string() + &rhs_float.to_string()) } (Value::Integer(lhs_int), Value::Text(rhs_text)) => { - Value::build_text(&(lhs_int.to_string() + rhs_text.as_str())) + Value::build_text(lhs_int.to_string() + rhs_text.as_str()) } (Value::Integer(lhs_int), Value::Integer(rhs_int)) => { - Value::build_text(&(lhs_int.to_string() + &rhs_int.to_string())) + Value::build_text(lhs_int.to_string() + &rhs_int.to_string()) } (Value::Integer(lhs_int), Value::Float(rhs_float)) => { - Value::build_text(&(lhs_int.to_string() + &rhs_float.to_string())) + Value::build_text(lhs_int.to_string() + &rhs_float.to_string()) } (Value::Float(lhs_float), Value::Text(rhs_text)) => { - Value::build_text(&(lhs_float.to_string() + rhs_text.as_str())) + Value::build_text(lhs_float.to_string() + rhs_text.as_str()) } (Value::Float(lhs_float), Value::Integer(rhs_int)) => { - Value::build_text(&(lhs_float.to_string() + &rhs_int.to_string())) + Value::build_text(lhs_float.to_string() + &rhs_int.to_string()) } (Value::Float(lhs_float), Value::Float(rhs_float)) => { - Value::build_text(&(lhs_float.to_string() + &rhs_float.to_string())) + Value::build_text(lhs_float.to_string() + &rhs_float.to_string()) } (Value::Null, _) | (_, Value::Null) => Value::Null, (Value::Blob(_), _) | (_, Value::Blob(_)) => { @@ -6350,7 +6340,7 @@ fn exec_concat_strings(registers: &[Register]) -> Value { v => result.push_str(&format!("{}", v)), } } - Value::build_text(&result) + Value::build_text(result) } fn exec_concat_ws(registers: &[Register]) -> Value { @@ -6376,7 +6366,7 @@ fn exec_concat_ws(registers: &[Register]) -> Value { } } - Value::build_text(&result) + Value::build_text(result) } fn exec_char(values: &[Register]) -> Value { @@ -6390,7 +6380,7 @@ fn exec_char(values: &[Register]) -> Value { } }) .collect(); - Value::build_text(&result) + Value::build_text(result) } fn construct_like_regex(pattern: &str) -> Regex { @@ -6787,32 +6777,36 @@ fn create_result_from_significand( let mut result = significand as f64; let mut exp = exponent; - if exp > 0 { - while exp >= 100 { - result *= 1e100; - exp -= 100; + match exp.cmp(&0) { + std::cmp::Ordering::Greater => { + while exp >= 100 { + result *= 1e100; + exp -= 100; + } + while exp >= 10 { + result *= 1e10; + exp -= 10; + } + while exp >= 1 { + result *= 10.0; + exp -= 1; + } } - while exp >= 10 { - result *= 1e10; - exp -= 10; - } - while exp >= 1 { - result *= 10.0; - exp -= 1; - } - } else if exp < 0 { - while exp <= -100 { - result *= 1e-100; - exp += 100; - } - while exp <= -10 { - result *= 1e-10; - exp += 10; - } - while exp <= -1 { - result *= 0.1; - exp += 1; + std::cmp::Ordering::Less => { + while exp <= -100 { + result *= 1e-100; + exp += 100; + } + while exp <= -10 { + result *= 1e-10; + exp += 10; + } + while exp <= -1 { + result *= 0.1; + exp += 1; + } } + std::cmp::Ordering::Equal => {} } if sign < 0 { @@ -6899,11 +6893,11 @@ fn is_numeric_value(reg: &Register) -> bool { fn stringify_register(reg: &mut Register) -> bool { match reg.get_owned_value() { Value::Integer(i) => { - *reg = Register::Value(Value::build_text(&i.to_string())); + *reg = Register::Value(Value::build_text(i.to_string())); true } Value::Float(f) => { - *reg = Register::Value(Value::build_text(&f.to_string())); + *reg = Register::Value(Value::build_text(f.to_string())); true } Value::Text(_) | Value::Null | Value::Blob(_) => false, @@ -6913,38 +6907,38 @@ fn stringify_register(reg: &mut Register) -> bool { #[cfg(test)] mod tests { use super::*; - use crate::types::{Text, Value}; + use crate::types::Value; #[test] fn test_apply_numeric_affinity_partial_numbers() { - let mut reg = Register::Value(Value::Text(Text::from_str("123abc"))); + let mut reg = Register::Value(Value::Text("123abc".into())); assert!(!apply_numeric_affinity(&mut reg, false)); assert!(matches!(reg, Register::Value(Value::Text(_)))); - let mut reg = Register::Value(Value::Text(Text::from_str("-53093015420544-15062897"))); + let mut reg = Register::Value(Value::Text("-53093015420544-15062897".into())); assert!(!apply_numeric_affinity(&mut reg, false)); assert!(matches!(reg, Register::Value(Value::Text(_)))); - let mut reg = Register::Value(Value::Text(Text::from_str("123.45xyz"))); + let mut reg = Register::Value(Value::Text("123.45xyz".into())); assert!(!apply_numeric_affinity(&mut reg, false)); assert!(matches!(reg, Register::Value(Value::Text(_)))); } #[test] fn test_apply_numeric_affinity_complete_numbers() { - let mut reg = Register::Value(Value::Text(Text::from_str("123"))); + let mut reg = Register::Value(Value::Text("123".into())); assert!(apply_numeric_affinity(&mut reg, false)); assert_eq!(*reg.get_owned_value(), Value::Integer(123)); - let mut reg = Register::Value(Value::Text(Text::from_str("123.45"))); + let mut reg = Register::Value(Value::Text("123.45".into())); assert!(apply_numeric_affinity(&mut reg, false)); assert_eq!(*reg.get_owned_value(), Value::Float(123.45)); - let mut reg = Register::Value(Value::Text(Text::from_str(" -456 "))); + let mut reg = Register::Value(Value::Text(" -456 ".into())); assert!(apply_numeric_affinity(&mut reg, false)); assert_eq!(*reg.get_owned_value(), Value::Integer(-456)); - let mut reg = Register::Value(Value::Text(Text::from_str("0"))); + let mut reg = Register::Value(Value::Text("0".into())); assert!(apply_numeric_affinity(&mut reg, false)); assert_eq!(*reg.get_owned_value(), Value::Integer(0)); } @@ -6959,22 +6953,16 @@ mod tests { (Value::Null, Value::Null), (Value::Null, Value::Integer(1)), (Value::Null, Value::Float(1.0)), - (Value::Null, Value::Text(Text::from_str("2"))), + (Value::Null, Value::Text("2".into())), (Value::Integer(1), Value::Null), (Value::Float(1.0), Value::Null), - (Value::Text(Text::from_str("1")), Value::Null), - ( - Value::Text(Text::from_str("1")), - Value::Text(Text::from_str("3")), - ), - ( - Value::Text(Text::from_str("1.0")), - Value::Text(Text::from_str("3.0")), - ), - (Value::Text(Text::from_str("1.0")), Value::Float(3.0)), - (Value::Text(Text::from_str("1.0")), Value::Integer(3)), - (Value::Float(1.0), Value::Text(Text::from_str("3.0"))), - (Value::Integer(1), Value::Text(Text::from_str("3"))), + (Value::Text("1".into()), Value::Null), + (Value::Text("1".into()), Value::Text("3".into())), + (Value::Text("1.0".into()), Value::Text("3.0".into())), + (Value::Text("1.0".into()), Value::Float(3.0)), + (Value::Text("1.0".into()), Value::Integer(3)), + (Value::Float(1.0), Value::Text("3.0".into())), + (Value::Integer(1), Value::Text("3".into())), ]; let outputs = [ @@ -7023,22 +7011,16 @@ mod tests { (Value::Null, Value::Null), (Value::Null, Value::Integer(1)), (Value::Null, Value::Float(1.0)), - (Value::Null, Value::Text(Text::from_str("1"))), + (Value::Null, Value::Text("1".into())), (Value::Integer(1), Value::Null), (Value::Float(1.0), Value::Null), - (Value::Text(Text::from_str("4")), Value::Null), - ( - Value::Text(Text::from_str("1")), - Value::Text(Text::from_str("3")), - ), - ( - Value::Text(Text::from_str("1.0")), - Value::Text(Text::from_str("3.0")), - ), - (Value::Text(Text::from_str("1.0")), Value::Float(3.0)), - (Value::Text(Text::from_str("1.0")), Value::Integer(3)), - (Value::Float(1.0), Value::Text(Text::from_str("3.0"))), - (Value::Integer(1), Value::Text(Text::from_str("3"))), + (Value::Text("4".into()), Value::Null), + (Value::Text("1".into()), Value::Text("3".into())), + (Value::Text("1.0".into()), Value::Text("3.0".into())), + (Value::Text("1.0".into()), Value::Float(3.0)), + (Value::Text("1.0".into()), Value::Integer(3)), + (Value::Float(1.0), Value::Text("3.0".into())), + (Value::Integer(1), Value::Text("3".into())), ]; let outputs = [ @@ -7087,22 +7069,16 @@ mod tests { (Value::Null, Value::Null), (Value::Null, Value::Integer(1)), (Value::Null, Value::Float(1.0)), - (Value::Null, Value::Text(Text::from_str("1"))), + (Value::Null, Value::Text("1".into())), (Value::Integer(1), Value::Null), (Value::Float(1.0), Value::Null), - (Value::Text(Text::from_str("4")), Value::Null), - ( - Value::Text(Text::from_str("2")), - Value::Text(Text::from_str("3")), - ), - ( - Value::Text(Text::from_str("2.0")), - Value::Text(Text::from_str("3.0")), - ), - (Value::Text(Text::from_str("2.0")), Value::Float(3.0)), - (Value::Text(Text::from_str("2.0")), Value::Integer(3)), - (Value::Float(2.0), Value::Text(Text::from_str("3.0"))), - (Value::Integer(2), Value::Text(Text::from_str("3.0"))), + (Value::Text("4".into()), Value::Null), + (Value::Text("2".into()), Value::Text("3".into())), + (Value::Text("2.0".into()), Value::Text("3.0".into())), + (Value::Text("2.0".into()), Value::Float(3.0)), + (Value::Text("2.0".into()), Value::Integer(3)), + (Value::Float(2.0), Value::Text("3.0".into())), + (Value::Integer(2), Value::Text("3.0".into())), ]; let outputs = [ @@ -7153,11 +7129,8 @@ mod tests { (Value::Null, Value::Integer(2)), (Value::Integer(2), Value::Null), (Value::Null, Value::Null), - ( - Value::Text(Text::from_str("6")), - Value::Text(Text::from_str("2")), - ), - (Value::Text(Text::from_str("6")), Value::Integer(2)), + (Value::Text("6".into()), Value::Text("2".into())), + (Value::Text("6".into()), Value::Integer(2)), ]; let outputs = [ @@ -7196,7 +7169,7 @@ mod tests { (Value::Null, Value::Null), (Value::Null, Value::Float(1.0)), (Value::Null, Value::Integer(1)), - (Value::Null, Value::Text(Text::from_str("1"))), + (Value::Null, Value::Text("1".into())), (Value::Float(1.0), Value::Null), (Value::Integer(1), Value::Null), (Value::Integer(12), Value::Integer(0)), @@ -7212,12 +7185,9 @@ mod tests { (Value::Float(12.0), Value::Float(-3.0)), (Value::Float(12.0), Value::Integer(-3)), (Value::Integer(12), Value::Float(-3.0)), - ( - Value::Text(Text::from_str("12.0")), - Value::Text(Text::from_str("3.0")), - ), - (Value::Text(Text::from_str("12.0")), Value::Float(3.0)), - (Value::Float(12.0), Value::Text(Text::from_str("3.0"))), + (Value::Text("12.0".into()), Value::Text("3.0".into())), + (Value::Text("12.0".into()), Value::Float(3.0)), + (Value::Float(12.0), Value::Text("3.0".into())), ]; let outputs = vec![ Value::Null, @@ -7269,9 +7239,9 @@ mod tests { (Value::Null, Value::Null), (Value::Float(0.0), Value::Null), (Value::Integer(1), Value::Float(2.2)), - (Value::Integer(0), Value::Text(Text::from_str("string"))), - (Value::Integer(0), Value::Text(Text::from_str("1"))), - (Value::Integer(1), Value::Text(Text::from_str("1"))), + (Value::Integer(0), Value::Text("string".into())), + (Value::Integer(0), Value::Text("1".into())), + (Value::Integer(1), Value::Text("1".into())), ]; let outputs = [ Value::Integer(0), @@ -7309,9 +7279,9 @@ mod tests { (Value::Float(0.0), Value::Null), (Value::Integer(1), Value::Float(2.2)), (Value::Float(0.0), Value::Integer(0)), - (Value::Integer(0), Value::Text(Text::from_str("string"))), - (Value::Integer(0), Value::Text(Text::from_str("1"))), - (Value::Integer(0), Value::Text(Text::from_str(""))), + (Value::Integer(0), Value::Text("string".into())), + (Value::Integer(0), Value::Text("1".into())), + (Value::Integer(0), Value::Text("".into())), ]; let outputs = [ Value::Null, diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 3a95c9971..7527cb731 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -27,11 +27,11 @@ pub fn insn_to_str( Insn::Init { target_pc } => ( "Init", 0, - target_pc.to_debug_int(), + target_pc.as_debug_int(), 0, Value::build_text(""), 0, - format!("Start at {}", target_pc.to_debug_int()), + format!("Start at {}", target_pc.as_debug_int()), ), Insn::Add { lhs, rhs, dest } => ( "Add", @@ -141,11 +141,11 @@ pub fn insn_to_str( Insn::NotNull { reg, target_pc } => ( "NotNull", *reg as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), 0, Value::build_text(""), 0, - format!("r[{}]!=NULL -> goto {}", reg, target_pc.to_debug_int()), + format!("r[{}]!=NULL -> goto {}", reg, target_pc.as_debug_int()), ), Insn::Compare { start_reg_a, @@ -157,7 +157,7 @@ pub fn insn_to_str( *start_reg_a as i32, *start_reg_b as i32, *count as i32, - Value::build_text(&format!("k({count}, {})", collation.unwrap_or_default())), + Value::build_text(format!("k({count}, {})", collation.unwrap_or_default())), 0, format!( "r[{}..{}]==r[{}..{}]", @@ -173,9 +173,9 @@ pub fn insn_to_str( target_pc_gt, } => ( "Jump", - target_pc_lt.to_debug_int(), - target_pc_eq.to_debug_int(), - target_pc_gt.to_debug_int(), + target_pc_lt.as_debug_int(), + target_pc_eq.as_debug_int(), + target_pc_gt.as_debug_int(), Value::build_text(""), 0, "".to_string(), @@ -206,7 +206,7 @@ pub fn insn_to_str( } => ( "IfPos", *reg as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), 0, Value::build_text(""), 0, @@ -215,7 +215,7 @@ pub fn insn_to_str( reg, reg, decrement_by, - target_pc.to_debug_int() + target_pc.as_debug_int() ), ), Insn::Eq { @@ -228,14 +228,14 @@ pub fn insn_to_str( "Eq", *lhs as i32, *rhs as i32, - target_pc.to_debug_int(), - Value::build_text(&collation.map_or("".to_string(), |c| c.to_string())), + target_pc.as_debug_int(), + Value::build_text(collation.map_or("".to_string(), |c| c.to_string())), 0, format!( "if r[{}]==r[{}] goto {}", lhs, rhs, - target_pc.to_debug_int() + target_pc.as_debug_int() ), ), Insn::Ne { @@ -248,14 +248,14 @@ pub fn insn_to_str( "Ne", *lhs as i32, *rhs as i32, - target_pc.to_debug_int(), - Value::build_text(&collation.map_or("".to_string(), |c| c.to_string())), + target_pc.as_debug_int(), + Value::build_text(collation.map_or("".to_string(), |c| c.to_string())), 0, format!( "if r[{}]!=r[{}] goto {}", lhs, rhs, - target_pc.to_debug_int() + target_pc.as_debug_int() ), ), Insn::Lt { @@ -268,10 +268,10 @@ pub fn insn_to_str( "Lt", *lhs as i32, *rhs as i32, - target_pc.to_debug_int(), - Value::build_text(&collation.map_or("".to_string(), |c| c.to_string())), + target_pc.as_debug_int(), + Value::build_text(collation.map_or("".to_string(), |c| c.to_string())), 0, - format!("if r[{}]r[{}] goto {}", lhs, rhs, target_pc.to_debug_int()), + format!("if r[{}]>r[{}] goto {}", lhs, rhs, target_pc.as_debug_int()), ), Insn::Ge { lhs, @@ -318,14 +318,14 @@ pub fn insn_to_str( "Ge", *lhs as i32, *rhs as i32, - target_pc.to_debug_int(), - Value::build_text(&collation.map_or("".to_string(), |c| c.to_string())), + target_pc.as_debug_int(), + Value::build_text(collation.map_or("".to_string(), |c| c.to_string())), 0, format!( "if r[{}]>=r[{}] goto {}", lhs, rhs, - target_pc.to_debug_int() + target_pc.as_debug_int() ), ), Insn::If { @@ -335,11 +335,11 @@ pub fn insn_to_str( } => ( "If", *reg as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), *jump_if_null as i32, Value::build_text(""), 0, - format!("if r[{}] goto {}", reg, target_pc.to_debug_int()), + format!("if r[{}] goto {}", reg, target_pc.as_debug_int()), ), Insn::IfNot { reg, @@ -348,11 +348,11 @@ pub fn insn_to_str( } => ( "IfNot", *reg as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), *jump_if_null as i32, Value::build_text(""), 0, - format!("if !r[{}] goto {}", reg, target_pc.to_debug_int()), + format!("if !r[{}] goto {}", reg, target_pc.as_debug_int()), ), Insn::OpenRead { cursor_id, @@ -427,7 +427,7 @@ pub fn insn_to_str( } => ( "VFilter", *cursor_id as i32, - pc_if_empty.to_debug_int(), + pc_if_empty.as_debug_int(), *arg_count as i32, Value::build_text(""), 0, @@ -466,7 +466,7 @@ pub fn insn_to_str( } => ( "VNext", *cursor_id as i32, - pc_if_next.to_debug_int(), + pc_if_next.as_debug_int(), 0, Value::build_text(""), 0, @@ -500,7 +500,7 @@ pub fn insn_to_str( } => ( "Rewind", *cursor_id as i32, - pc_if_empty.to_debug_int(), + pc_if_empty.as_debug_int(), 0, Value::build_text(""), 0, @@ -617,7 +617,7 @@ pub fn insn_to_str( } => ( "Next", *cursor_id as i32, - pc_if_next.to_debug_int(), + pc_if_next.as_debug_int(), 0, Value::build_text(""), 0, @@ -631,7 +631,7 @@ pub fn insn_to_str( *err_code as i32, 0, 0, - Value::build_text(&description), + Value::build_text(description), 0, "".to_string(), ), @@ -644,7 +644,7 @@ pub fn insn_to_str( *err_code as i32, 0, *target_reg as i32, - Value::build_text(&description), + Value::build_text(description), 0, "".to_string(), ), @@ -660,7 +660,7 @@ pub fn insn_to_str( Insn::Goto { target_pc } => ( "Goto", 0, - target_pc.to_debug_int(), + target_pc.as_debug_int(), 0, Value::build_text(""), 0, @@ -672,7 +672,7 @@ pub fn insn_to_str( } => ( "Gosub", *return_reg as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), 0, Value::build_text(""), 0, @@ -779,7 +779,7 @@ pub fn insn_to_str( "SeekRowid", *cursor_id as i32, *src_reg as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), Value::build_text(""), 0, format!( @@ -795,7 +795,7 @@ pub fn insn_to_str( get_table_or_index_name(*cursor_id), )) .unwrap_or(format!("cursor {}", cursor_id)), - target_pc.to_debug_int() + target_pc.as_debug_int() ), ), Insn::DeferredSeek { @@ -848,7 +848,7 @@ pub fn insn_to_str( _ => unreachable!(), }, *cursor_id as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), *start_reg as i32, Value::build_text(""), 0, @@ -910,7 +910,7 @@ pub fn insn_to_str( _ => unreachable!(), }, *cursor_id as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), *start_reg as i32, Value::build_text(""), 0, @@ -919,11 +919,11 @@ pub fn insn_to_str( Insn::DecrJumpZero { reg, target_pc } => ( "DecrJumpZero", *reg as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), 0, Value::build_text(""), 0, - format!("if (--r[{}]==0) goto {}", reg, target_pc.to_debug_int()), + format!("if (--r[{}]==0) goto {}", reg, target_pc.as_debug_int()), ), Insn::AggStep { func, @@ -975,7 +975,7 @@ pub fn insn_to_str( *cursor_id as i32, *columns as i32, 0, - Value::build_text(&(format!("k({},{})", order.len(), to_print.join(",")))), + Value::build_text(format!("k({},{})", order.len(), to_print.join(","))), 0, format!("cursor={}", cursor_id), ) @@ -1011,7 +1011,7 @@ pub fn insn_to_str( } => ( "SorterSort", *cursor_id as i32, - pc_if_empty.to_debug_int(), + pc_if_empty.as_debug_int(), 0, Value::build_text(""), 0, @@ -1023,7 +1023,7 @@ pub fn insn_to_str( } => ( "SorterNext", *cursor_id as i32, - pc_if_next.to_debug_int(), + pc_if_next.as_debug_int(), 0, Value::build_text(""), 0, @@ -1045,7 +1045,7 @@ pub fn insn_to_str( } else { func.func.to_string() }; - Value::build_text(&s) + Value::build_text(s) }, 0, if func.arg_count == 0 { @@ -1068,8 +1068,8 @@ pub fn insn_to_str( } => ( "InitCoroutine", *yield_reg as i32, - jump_on_definition.to_debug_int(), - start_offset.to_debug_int(), + jump_on_definition.as_debug_int(), + start_offset.as_debug_int(), Value::build_text(""), 0, "".to_string(), @@ -1089,7 +1089,7 @@ pub fn insn_to_str( } => ( "Yield", *yield_reg as i32, - end_offset.to_debug_int(), + end_offset.as_debug_int(), 0, Value::build_text(""), 0, @@ -1106,7 +1106,7 @@ pub fn insn_to_str( *cursor as i32, *record_reg as i32, *key_reg as i32, - Value::build_text(&table_name), + Value::build_text(table_name), flag.0 as u16, format!("intkey=r[{}] data=r[{}]", key_reg, record_reg), ), @@ -1177,9 +1177,9 @@ pub fn insn_to_str( ( "NoConflict", *cursor_id as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), *record_reg as i32, - Value::build_text(&format!("{num_regs}")), + Value::build_text(format!("{num_regs}")), 0, key, ) @@ -1191,7 +1191,7 @@ pub fn insn_to_str( } => ( "NotExists", *cursor as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), *rowid_reg as i32, Value::build_text(""), 0, @@ -1306,7 +1306,7 @@ pub fn insn_to_str( } => ( "Last", *cursor_id as i32, - pc_if_empty.to_debug_int(), + pc_if_empty.as_debug_int(), 0, Value::build_text(""), 0, @@ -1315,11 +1315,11 @@ pub fn insn_to_str( Insn::IsNull { reg, target_pc } => ( "IsNull", *reg as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), 0, Value::build_text(""), 0, - format!("if (r[{}]==NULL) goto {}", reg, target_pc.to_debug_int()), + format!("if (r[{}]==NULL) goto {}", reg, target_pc.as_debug_int()), ), Insn::ParseSchema { db, where_clause } => ( "ParseSchema", @@ -1336,7 +1336,7 @@ pub fn insn_to_str( } => ( "Prev", *cursor_id as i32, - pc_if_prev.to_debug_int(), + pc_if_prev.as_debug_int(), 0, Value::build_text(""), 0, @@ -1491,12 +1491,12 @@ pub fn insn_to_str( target_pc_when_reentered, } => ( "Once", - target_pc_when_reentered.to_debug_int(), + target_pc_when_reentered.as_debug_int(), 0, 0, Value::build_text(""), 0, - format!("goto {}", target_pc_when_reentered.to_debug_int()), + format!("goto {}", target_pc_when_reentered.as_debug_int()), ), Insn::BeginSubrtn { dest, dest_end } => ( "BeginSubrtn", @@ -1527,7 +1527,7 @@ pub fn insn_to_str( "Found" }, *cursor_id as i32, - target_pc.to_debug_int(), + target_pc.as_debug_int(), *record_reg as i32, Value::build_text(""), 0, @@ -1538,7 +1538,7 @@ pub fn insn_to_str( } else { "" }, - target_pc.to_debug_int() + target_pc.as_debug_int() ), ), Insn::Affinity { diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 54a565fc5..d1ca4c1af 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -46,7 +46,7 @@ impl CmpInsFlags { } pub fn with_affinity(mut self, affinity: Affinity) -> Self { - let aff_code = affinity.to_char_code() as usize; + let aff_code = affinity.as_char_code() as usize; self.0 = (self.0 & !Self::AFFINITY_MASK) | aff_code; self } diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 88ef7ecc1..cf3837e9c 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -35,7 +35,7 @@ use crate::{ use crate::{ storage::{btree::BTreeCursor, pager::Pager, sqlite3_ondisk::DatabaseHeader}, translate::plan::ResultSetColumn, - types::{AggContext, Cursor, CursorResult, ImmutableRecord, SeekKey, SeekOp, Value}, + types::{AggContext, Cursor, CursorResult, ImmutableRecord, Value}, vdbe::{builder::CursorType, insn::Insn}, }; @@ -45,10 +45,7 @@ use crate::{Connection, MvStore, Result, TransactionState}; use builder::CursorKey; use execute::{InsnFunction, InsnFunctionStepResult, OpIdxDeleteState, OpIntegrityCheckState}; -use rand::{ - distributions::{Distribution, Uniform}, - Rng, -}; +use rand::Rng; use regex::Regex; use std::{ cell::{Cell, RefCell}, @@ -65,10 +62,11 @@ use tracing::{instrument, Level}; /// /// In some cases, we want to jump to EXACTLY a specific instruction. /// - Example: a condition is not met, so we want to jump to wherever Halt is. +/// /// In other cases, we don't care what the exact instruction is, but we know that we /// want to jump to whatever comes AFTER a certain instruction. /// - Example: a Next instruction will want to jump to "whatever the start of the loop is", -/// but it doesn't care what instruction that is. +/// but it doesn't care what instruction that is. /// /// The reason this distinction is important is that we might reorder instructions that are /// constant at compile time, and when we do that, we need to change the offsets of any impacted @@ -106,7 +104,7 @@ impl BranchOffset { } /// Returns the offset value. Panics if the branch offset is a label or placeholder. - pub fn to_offset_int(&self) -> InsnReference { + pub fn as_offset_int(&self) -> InsnReference { match self { BranchOffset::Label(v) => unreachable!("Unresolved label: {}", v), BranchOffset::Offset(v) => *v, @@ -117,7 +115,7 @@ impl BranchOffset { /// Returns the branch offset as a signed integer. /// Used in explain output, where we don't want to panic in case we have an unresolved /// label or placeholder. - pub fn to_debug_int(&self) -> i32 { + pub fn as_debug_int(&self) -> i32 { match self { BranchOffset::Label(v) => *v as i32, BranchOffset::Offset(v) => *v as i32, @@ -129,11 +127,11 @@ impl BranchOffset { /// Returns a new branch offset. /// Panics if the branch offset is a label or placeholder. pub fn add>(self, n: N) -> BranchOffset { - BranchOffset::Offset(self.to_offset_int() + n.into()) + BranchOffset::Offset(self.as_offset_int() + n.into()) } pub fn sub>(self, n: N) -> BranchOffset { - BranchOffset::Offset(self.to_offset_int() - n.into()) + BranchOffset::Offset(self.as_offset_int() - n.into()) } } @@ -206,9 +204,9 @@ impl Bitfield { /// The commit state of the program. /// There are two states: /// - Ready: The program is ready to run the next instruction, or has shut down after -/// the last instruction. +/// the last instruction. /// - Committing: The program is committing a write transaction. It is waiting for the pager to finish flushing the cache to disk, -/// primarily to the WAL, but also possibly checkpointing the WAL to the database file. +/// primarily to the WAL, but also possibly checkpointing the WAL to the database file. enum CommitState { Ready, Committing, @@ -488,37 +486,38 @@ impl Program { } } -fn get_new_rowid(cursor: &mut BTreeCursor, mut rng: R) -> Result> { +fn get_new_rowid(cursor: &mut BTreeCursor, mut _rng: R) -> Result> { match cursor.seek_to_last()? { CursorResult::Ok(()) => {} CursorResult::IO => return Ok(CursorResult::IO), } - let mut rowid = match cursor.rowid()? { + let rowid = match cursor.rowid()? { CursorResult::Ok(Some(rowid)) => rowid.checked_add(1).unwrap_or(i64::MAX), // add 1 but be careful with overflows, in case of overflow - use i64::MAX CursorResult::Ok(None) => 1, CursorResult::IO => return Ok(CursorResult::IO), }; - if rowid > i64::MAX.try_into().unwrap() { - let distribution = Uniform::from(1..=i64::MAX); - let max_attempts = 100; - for count in 0..max_attempts { - rowid = distribution.sample(&mut rng).try_into().unwrap(); - match cursor.seek(SeekKey::TableRowId(rowid), SeekOp::GE { eq_only: true })? { - CursorResult::Ok(false) => break, // Found a non-existing rowid - CursorResult::Ok(true) => { - if count == max_attempts - 1 { - return Err(LimboError::InternalError( - "Failed to generate a new rowid".to_string(), - )); - } else { - continue; // Try next random rowid - } - } - CursorResult::IO => return Ok(CursorResult::IO), - } - } - } - Ok(CursorResult::Ok(rowid.try_into().unwrap())) + // NOTE(nilskch): I commented this part out because this condition will never be true. + // if rowid > i64::MAX { + // let distribution = Uniform::from(1..=i64::MAX); + // let max_attempts = 100; + // for count in 0..max_attempts { + // rowid = distribution.sample(&mut rng); + // match cursor.seek(SeekKey::TableRowId(rowid), SeekOp::GE { eq_only: true })? { + // CursorResult::Ok(false) => break, // Found a non-existing rowid + // CursorResult::Ok(true) => { + // if count == max_attempts - 1 { + // return Err(LimboError::InternalError( + // "Failed to generate a new rowid".to_string(), + // )); + // } else { + // continue; // Try next random rowid + // } + // } + // CursorResult::IO => return Ok(CursorResult::IO), + // } + // } + // } + Ok(CursorResult::Ok(rowid)) } fn make_record(registers: &[Register], start_reg: &usize, count: &usize) -> ImmutableRecord { @@ -575,7 +574,7 @@ fn print_insn(program: &Program, addr: InsnReference, insn: &Insn, indent: Strin // Yield SeekGt SeekLt RowSetRead Rewind // or if the P1 parameter is one instead of zero, then increase the indent number for all // opcodes between the earlier instruction and "Goto" -fn get_indent_counts(insns: &Vec<(Insn, InsnFunction)>) -> Vec { +fn get_indent_counts(insns: &[(Insn, InsnFunction)]) -> Vec { let mut indents = vec![0; insns.len()]; for (i, (insn, _)) in insns.iter().enumerate() { @@ -583,14 +582,14 @@ fn get_indent_counts(insns: &Vec<(Insn, InsnFunction)>) -> Vec { let mut end = 0; match insn { Insn::Next { pc_if_next, .. } | Insn::VNext { pc_if_next, .. } => { - let dest = pc_if_next.to_debug_int() as usize; + let dest = pc_if_next.as_debug_int() as usize; if dest < i { start = dest; end = i; } } Insn::Prev { pc_if_prev, .. } => { - let dest = pc_if_prev.to_debug_int() as usize; + let dest = pc_if_prev.as_debug_int() as usize; if dest < i { start = dest; end = i; @@ -598,7 +597,7 @@ fn get_indent_counts(insns: &Vec<(Insn, InsnFunction)>) -> Vec { } Insn::Goto { target_pc } => { - let dest = target_pc.to_debug_int() as usize; + let dest = target_pc.as_debug_int() as usize; if dest < i && matches!( insns.get(dest).map(|(insn, _)| insn), @@ -615,8 +614,8 @@ fn get_indent_counts(insns: &Vec<(Insn, InsnFunction)>) -> Vec { _ => {} } - for i in start..end { - indents[i] += 1; + for indent in indents.iter_mut().take(end).skip(start) { + *indent += 1; } } @@ -681,7 +680,7 @@ impl Row { T::from_value(value) } - pub fn get_value<'a>(&'a self, idx: usize) -> &'a Value { + pub fn get_value(&self, idx: usize) -> &Value { let value = unsafe { self.values.add(idx).as_ref().unwrap() }; match value { Register::Value(owned_value) => owned_value, diff --git a/core/vector/mod.rs b/core/vector/mod.rs index 7f46eee58..d68bbe90c 100644 --- a/core/vector/mod.rs +++ b/core/vector/mod.rs @@ -62,7 +62,7 @@ pub fn vector_extract(args: &[Register]) -> Result { let vector_type = vector_type(blob)?; let vector = vector_deserialize(vector_type, blob)?; - Ok(Value::build_text(&vector_to_text(&vector))) + Ok(Value::build_text(vector_to_text(&vector))) } pub fn vector_distance_cos(args: &[Register]) -> Result { diff --git a/core/vector/vector_types.rs b/core/vector/vector_types.rs index e47380389..ef0b12677 100644 --- a/core/vector/vector_types.rs +++ b/core/vector/vector_types.rs @@ -116,7 +116,7 @@ pub fn parse_vector(value: &Register, vec_ty: Option) -> Result) -> Result Err(LimboError::ConversionError( "Invalid vector type".to_string(), @@ -138,8 +138,8 @@ pub fn vector_to_text(vector: &Vector) -> String { match vector.vector_type { VectorType::Float32 => { let data = vector.as_f32_slice(); - for i in 0..vector.dims { - text.push_str(&data[i].to_string()); + for (i, value) in data.iter().enumerate().take(vector.dims) { + text.push_str(&value.to_string()); if i < vector.dims - 1 { text.push(','); } @@ -147,8 +147,8 @@ pub fn vector_to_text(vector: &Vector) -> String { } VectorType::Float64 => { let data = vector.as_f64_slice(); - for i in 0..vector.dims { - text.push_str(&data[i].to_string()); + for (i, value) in data.iter().enumerate().take(vector.dims) { + text.push_str(&value.to_string()); if i < vector.dims - 1 { text.push(','); } @@ -555,7 +555,7 @@ mod tests { // Skip test if types are different return true; } - match do_vector_distance_cos(&v1, &v2) { + match do_vector_distance_cos(v1, v2) { Ok(distance) => { // Cosine distance is always between 0 and 2 (0.0..=2.0).contains(&distance) diff --git a/core/vtab.rs b/core/vtab.rs index b9d81781a..0d42b2707 100644 --- a/core/vtab.rs +++ b/core/vtab.rs @@ -32,7 +32,7 @@ impl VirtualTable { syms: &SymbolTable, ) -> crate::Result> { let module = syms.vtab_modules.get(name); - let (vtab_type, schema) = if let Some(_) = module { + let (vtab_type, schema) = if module.is_some() { let ext_args = match args { Some(ref args) => vtable_args(args), None => vec![], diff --git a/extensions/completion/src/lib.rs b/extensions/completion/src/lib.rs index db4ca26bf..c713e8e3f 100644 --- a/extensions/completion/src/lib.rs +++ b/extensions/completion/src/lib.rs @@ -41,10 +41,10 @@ enum CompletionPhase { Eof = 11, } -impl Into for CompletionPhase { - fn into(self) -> i64 { +impl From for i64 { + fn from(val: CompletionPhase) -> Self { use self::CompletionPhase::*; - match self { + match val { Keywords => 1, // Pragmas => 2, // Functions => 3, diff --git a/extensions/crypto/src/crypto.rs b/extensions/crypto/src/crypto.rs index ddebbd0a6..9bac086a5 100644 --- a/extensions/crypto/src/crypto.rs +++ b/extensions/crypto/src/crypto.rs @@ -119,7 +119,7 @@ pub fn decode(data: &Value, format: &Value) -> Result { )) } "base85" => { - let decoded = decode_ascii85(&input_text).map_err(|_| Error::DecodeFailed)?; + let decoded = decode_ascii85(input_text).map_err(|_| Error::DecodeFailed)?; Ok(Value::from_text( String::from_utf8(decoded).map_err(|_| Error::InvalidUtf8)?, @@ -157,7 +157,7 @@ fn decode_ascii85(input: &str) -> Result, Box> { } } - if digit < 33 || digit > 117 { + if !(33..=117).contains(&digit) { return Err("Input char is out of range for Ascii85".into()); } @@ -205,8 +205,8 @@ fn encode_ascii85(input: &[u8]) -> String { let number = u32::from_be_bytes(chunk.as_ref().try_into().expect("Internal Error")); - for i in 0..count { - let digit = (((number / TABLE[i]) % 85) + 33) as u8; + for value in TABLE.iter().take(count) { + let digit = (((number / value) % 85) + 33) as u8; result.push(digit as char); } } diff --git a/extensions/csv/src/lib.rs b/extensions/csv/src/lib.rs index 55dee67fc..0b9808714 100644 --- a/extensions/csv/src/lib.rs +++ b/extensions/csv/src/lib.rs @@ -182,10 +182,7 @@ impl VTabModule for CsvVTabModule { if table.header { let headers = reader.headers().map_err(|_| ResultCode::Error)?; if column_count.is_none() && schema.is_none() { - columns = headers - .into_iter() - .map(|header| Self::escape_double_quote(header)) - .collect(); + columns = headers.into_iter().map(Self::escape_double_quote).collect(); } if columns.is_empty() { columns.push("(NULL)".to_owned()); diff --git a/extensions/regexp/src/lib.rs b/extensions/regexp/src/lib.rs index ce9b6d2ee..583747e87 100644 --- a/extensions/regexp/src/lib.rs +++ b/extensions/regexp/src/lib.rs @@ -64,7 +64,7 @@ fn regexp_replace(&self, args: &[Value]) -> Value { None => "", // If args[2] does not exist, use an empty string }; - match (args.get(0), args.get(1)) { + match (args.first(), args.get(1)) { (Some(haystack), Some(pattern)) => { let Some(haystack_text) = haystack.to_text() else { return Value::from_text("".to_string()); // Return an empty string if haystack is not valid @@ -73,11 +73,11 @@ fn regexp_replace(&self, args: &[Value]) -> Value { return Value::from_text("".to_string()); // Return an empty string if pattern is not valid }; - let re = match Regex::new(&pattern_text) { + let re = match Regex::new(pattern_text) { Ok(re) => re, Err(_) => return Value::from_text("".to_string()), // Return an empty string if regex compilation fails }; - Value::from_text(re.replace(&haystack_text, replacement).to_string()) + Value::from_text(re.replace(haystack_text, replacement).to_string()) } _ => Value::from_text("".to_string()), // Return an empty string for invalid value types } diff --git a/extensions/time/src/time.rs b/extensions/time/src/time.rs index 238d599d5..cfe8082d7 100644 --- a/extensions/time/src/time.rs +++ b/extensions/time/src/time.rs @@ -103,6 +103,12 @@ pub enum TimeRoundField { Micro, } +impl Default for Time { + fn default() -> Self { + Self::new() + } +} + impl Time { /// Returns a new instance of Time with tracking UTC::now pub fn new() -> Self { diff --git a/simulator/generation/expr.rs b/simulator/generation/expr.rs index d3c09f869..fc60c6248 100644 --- a/simulator/generation/expr.rs +++ b/simulator/generation/expr.rs @@ -50,10 +50,7 @@ where { fn arbitrary_from(rng: &mut R, t: A) -> Self { let size = rng.gen_range(0..5); - (0..size) - .into_iter() - .map(|_| T::arbitrary_from(rng, t)) - .collect() + (0..size).map(|_| T::arbitrary_from(rng, t)).collect() } } @@ -61,7 +58,8 @@ where impl ArbitraryFrom<&SimulatorEnv> for Expr { fn arbitrary_from(rng: &mut R, t: &SimulatorEnv) -> Self { let choice = rng.gen_range(0..13); - let expr = match choice { + + match choice { 0 => Expr::Between { lhs: Box::arbitrary_from(rng, t), not: rng.gen_bool(0.5), @@ -78,7 +76,6 @@ impl ArbitraryFrom<&SimulatorEnv> for Expr { when_then_pairs: { let size = rng.gen_range(0..5); (0..size) - .into_iter() .map(|_| (Self::arbitrary_from(rng, t), Self::arbitrary_from(rng, t))) .collect() }, @@ -136,8 +133,7 @@ impl ArbitraryFrom<&SimulatorEnv> for Expr { // TODO: skip Raise // TODO: skip subquery _ => unreachable!(), - }; - expr + } } } @@ -257,7 +253,7 @@ impl ArbitraryFrom<&Vec<&SimValue>> for ast::Expr { return Self::Literal(ast::Literal::Null); } // TODO: for now just convert the value to an ast::Literal - let value = pick(&values, rng); + let value = pick(values, rng); Expr::Literal((*value).into()) } } diff --git a/simulator/generation/mod.rs b/simulator/generation/mod.rs index 8e61472ce..2e4f57aca 100644 --- a/simulator/generation/mod.rs +++ b/simulator/generation/mod.rs @@ -10,6 +10,9 @@ pub mod property; pub mod query; pub mod table; +type ArbitraryFromFunc<'a, R, T> = Box T + 'a>; +type Choice<'a, R, T> = (usize, Box Option + 'a>); + /// Arbitrary trait for generating random values /// An implementation of arbitrary is assumed to be a uniform sampling of /// the possible values of the type, with a bias towards smaller values for @@ -42,12 +45,11 @@ pub trait ArbitraryFromMaybe { // todo: switch to a simpler type signature that can accommodate all integer and float types, which // should be enough for our purposes. pub(crate) fn frequency< - 'a, T, R: Rng, N: Sum + PartialOrd + Copy + Default + SampleUniform + SubAssign, >( - choices: Vec<(N, Box T + 'a>)>, + choices: Vec<(N, ArbitraryFromFunc)>, rng: &mut R, ) -> T { let total = choices.iter().map(|(weight, _)| *weight).sum::(); @@ -64,7 +66,7 @@ pub(crate) fn frequency< } /// one_of is a helper function for composing different generators with equal probability of occurrence. -pub(crate) fn one_of<'a, T, R: Rng>(choices: Vec T + 'a>>, rng: &mut R) -> T { +pub(crate) fn one_of(choices: Vec>, rng: &mut R) -> T { let index = rng.gen_range(0..choices.len()); choices[index](rng) } @@ -72,10 +74,7 @@ pub(crate) fn one_of<'a, T, R: Rng>(choices: Vec T + 'a>>, /// backtrack is a helper function for composing different "failable" generators. /// The function takes a list of functions that return an Option, along with number of retries /// to make before giving up. -pub(crate) fn backtrack<'a, T, R: Rng>( - mut choices: Vec<(usize, Box Option + 'a>)>, - rng: &mut R, -) -> Option { +pub(crate) fn backtrack(mut choices: Vec>, rng: &mut R) -> Option { loop { // If there are no more choices left, we give up let choices_ = choices diff --git a/simulator/generation/predicate/binary.rs b/simulator/generation/predicate/binary.rs index ef6379738..6c64e522b 100644 --- a/simulator/generation/predicate/binary.rs +++ b/simulator/generation/predicate/binary.rs @@ -54,7 +54,7 @@ impl Predicate { } /// Produces a true [ast::Expr::Binary] [Predicate] that is true for the provided row in the given table - pub fn true_binary(rng: &mut R, t: &Table, row: &Vec) -> Predicate { + pub fn true_binary(rng: &mut R, t: &Table, row: &[SimValue]) -> Predicate { // Pick a column let column_index = rng.gen_range(0..t.columns.len()); let column = &t.columns[column_index]; @@ -146,7 +146,7 @@ impl Predicate { } /// Produces an [ast::Expr::Binary] [Predicate] that is false for the provided row in the given table - pub fn false_binary(rng: &mut R, t: &Table, row: &Vec) -> Predicate { + pub fn false_binary(rng: &mut R, t: &Table, row: &[SimValue]) -> Predicate { // Pick a column let column_index = rng.gen_range(0..t.columns.len()); let column = &t.columns[column_index]; @@ -321,11 +321,11 @@ impl CompoundPredicate { ) -> Self { // Cannot pick a row if the table is empty if table.rows.is_empty() { - return Self( - predicate_value - .then_some(Predicate::true_()) - .unwrap_or(Predicate::false_()), - ); + return Self(if predicate_value { + Predicate::true_() + } else { + Predicate::false_() + }); } let row = pick(&table.rows, rng); let predicate = if rng.gen_bool(0.7) { @@ -449,7 +449,7 @@ mod tests { let predicate = Predicate::true_binary(&mut rng, &table, row); let value = expr_to_value(&predicate.0, row, &table); assert!( - value.as_ref().map_or(false, |value| value.into_bool()), + value.as_ref().map_or(false, |value| value.as_bool()), "Predicate: {:#?}\nValue: {:#?}\nSeed: {}", predicate, value, @@ -478,7 +478,7 @@ mod tests { let predicate = Predicate::false_binary(&mut rng, &table, row); let value = expr_to_value(&predicate.0, row, &table); assert!( - !value.as_ref().map_or(false, |value| value.into_bool()), + !value.as_ref().map_or(false, |value| value.as_bool()), "Predicate: {:#?}\nValue: {:#?}\nSeed: {}", predicate, value, diff --git a/simulator/generation/predicate/mod.rs b/simulator/generation/predicate/mod.rs index 846f1d23a..3e2e95554 100644 --- a/simulator/generation/predicate/mod.rs +++ b/simulator/generation/predicate/mod.rs @@ -268,7 +268,7 @@ mod tests { let predicate = SimplePredicate::arbitrary_from(&mut rng, (&table, row, true)).0; let value = expr_to_value(&predicate.0, row, &table); assert!( - value.as_ref().map_or(false, |value| value.into_bool()), + value.as_ref().map_or(false, |value| value.as_bool()), "Predicate: {:#?}\nValue: {:#?}\nSeed: {}", predicate, value, @@ -297,7 +297,7 @@ mod tests { let predicate = SimplePredicate::arbitrary_from(&mut rng, (&table, row, false)).0; let value = expr_to_value(&predicate.0, row, &table); assert!( - !value.as_ref().map_or(false, |value| value.into_bool()), + !value.as_ref().map_or(false, |value| value.as_bool()), "Predicate: {:#?}\nValue: {:#?}\nSeed: {}", predicate, value, @@ -326,7 +326,7 @@ mod tests { let predicate = Predicate::arbitrary_from(&mut rng, (&table, row)); let value = expr_to_value(&predicate.0, row, &table); assert!( - value.as_ref().map_or(false, |value| value.into_bool()), + value.as_ref().map_or(false, |value| value.as_bool()), "Predicate: {:#?}\nValue: {:#?}\nSeed: {}", predicate, value, diff --git a/simulator/generation/predicate/unary.rs b/simulator/generation/predicate/unary.rs index 8f090a206..13c4d1dbc 100644 --- a/simulator/generation/predicate/unary.rs +++ b/simulator/generation/predicate/unary.rs @@ -20,7 +20,7 @@ impl ArbitraryFromMaybe<&SimValue> for TrueValue { Self: Sized, { // If the Value is a true value return it else you cannot return a true Value - value.into_bool().then_some(Self(value.clone())) + value.as_bool().then_some(Self(value.clone())) } } @@ -46,7 +46,7 @@ impl ArbitraryFromMaybe<&SimValue> for FalseValue { Self: Sized, { // If the Value is a false value return it else you cannot return a false Value - (!value.into_bool()).then_some(Self(value.clone())) + (!value.as_bool()).then_some(Self(value.clone())) } } @@ -76,7 +76,7 @@ impl ArbitraryFromMaybe<(&SimValue, bool)> for BitNotValue { { let bit_not_val = value.unary_exec(ast::UnaryOperator::BitwiseNot); // If you bit not the Value and it meets the predicate return Some, else None - (bit_not_val.into_bool() == predicate).then_some(BitNotValue(value.clone())) + (bit_not_val.as_bool() == predicate).then_some(BitNotValue(value.clone())) } } @@ -115,7 +115,7 @@ impl SimplePredicate { num_retries, Box::new(|rng| { TrueValue::arbitrary_from_maybe(rng, column_value).map(|value| { - assert!(value.0.into_bool()); + assert!(value.0.as_bool()); // Positive is a no-op in Sqlite Expr::unary(ast::UnaryOperator::Positive, Expr::Literal(value.0.into())) }) @@ -125,7 +125,7 @@ impl SimplePredicate { num_retries, Box::new(|rng| { TrueValue::arbitrary_from_maybe(rng, column_value).map(|value| { - assert!(value.0.into_bool()); + assert!(value.0.as_bool()); // True Value with negative is still True Expr::unary(ast::UnaryOperator::Negative, Expr::Literal(value.0.into())) }) @@ -146,7 +146,7 @@ impl SimplePredicate { num_retries, Box::new(|rng| { FalseValue::arbitrary_from_maybe(rng, column_value).map(|value| { - assert!(!value.0.into_bool()); + assert!(!value.0.as_bool()); Expr::unary(ast::UnaryOperator::Not, Expr::Literal(value.0.into())) }) }), @@ -176,7 +176,7 @@ impl SimplePredicate { num_retries, Box::new(|rng| { FalseValue::arbitrary_from_maybe(rng, column_value).map(|value| { - assert!(!value.0.into_bool()); + assert!(!value.0.as_bool()); // Positive is a no-op in Sqlite Expr::unary(ast::UnaryOperator::Positive, Expr::Literal(value.0.into())) }) @@ -186,7 +186,7 @@ impl SimplePredicate { num_retries, Box::new(|rng| { FalseValue::arbitrary_from_maybe(rng, column_value).map(|value| { - assert!(!value.0.into_bool()); + assert!(!value.0.as_bool()); // True Value with negative is still True Expr::unary(ast::UnaryOperator::Negative, Expr::Literal(value.0.into())) }) @@ -207,7 +207,7 @@ impl SimplePredicate { num_retries, Box::new(|rng| { TrueValue::arbitrary_from_maybe(rng, column_value).map(|value| { - assert!(value.0.into_bool()); + assert!(value.0.as_bool()); Expr::unary(ast::UnaryOperator::Not, Expr::Literal(value.0.into())) }) }), diff --git a/simulator/generation/property.rs b/simulator/generation/property.rs index 0694a3d86..4a78ad7bd 100644 --- a/simulator/generation/property.rs +++ b/simulator/generation/property.rs @@ -416,7 +416,7 @@ impl Property { .iter() .filter(|vs| { let v = vs.first().unwrap(); - v.into_bool() + v.as_bool() }) .count(); Ok(rows1_count == rows2.len()) @@ -582,15 +582,12 @@ fn property_double_create_failure( // - [ ] Table `t` will not be renamed or dropped.(todo: add this constraint once ALTER or DROP is implemented) for _ in 0..rng.gen_range(0..3) { let query = Query::arbitrary_from(rng, (env, remaining)); - match &query { - Query::Create(Create { table: t }) => { - // There will be no errors in the middle interactions. - // - Creating the same table is an error - if t.name == table.name { - continue; - } + if let Query::Create(Create { table: t }) = &query { + // There will be no errors in the middle interactions. + // - Creating the same table is an error + if t.name == table.name { + continue; } - _ => (), } queries.push(query); } @@ -658,14 +655,11 @@ fn property_drop_select( // - [-] The table `t` will not be created, no table will be renamed to `t`. (todo: update this constraint once ALTER is implemented) for _ in 0..rng.gen_range(0..3) { let query = Query::arbitrary_from(rng, (env, remaining)); - match &query { - Query::Create(Create { table: t }) => { - // - The table `t` will not be created - if t.name == table.name { - continue; - } + if let Query::Create(Create { table: t }) = &query { + // - The table `t` will not be created + if t.name == table.name { + continue; } - _ => (), } queries.push(query); } diff --git a/simulator/generation/query.rs b/simulator/generation/query.rs index 09d0f6421..0cc1083d1 100644 --- a/simulator/generation/query.rs +++ b/simulator/generation/query.rs @@ -55,12 +55,7 @@ impl ArbitraryFrom<&SimulatorEnv> for Insert { let _gen_select = |rng: &mut R| { // Find a non-empty table - let table = env.tables.iter().find(|t| !t.rows.is_empty()); - if table.is_none() { - return None; - } - - let select_table = table.unwrap(); + let select_table = env.tables.iter().find(|t| !t.rows.is_empty())?; let row = pick(&select_table.rows, rng); let predicate = Predicate::arbitrary_from(rng, (select_table, row)); // Pick another table to insert into @@ -81,7 +76,7 @@ impl ArbitraryFrom<&SimulatorEnv> for Insert { // Backtrack here cannot return None backtrack( vec![ - (1, Box::new(|rng| gen_values(rng))), + (1, Box::new(gen_values)), // todo: test and enable this once `INSERT INTO SELECT * FROM
` is supported // (1, Box::new(|rng| gen_select(rng))), ], diff --git a/simulator/generation/table.rs b/simulator/generation/table.rs index 45b2a5a73..eacfc1627 100644 --- a/simulator/generation/table.rs +++ b/simulator/generation/table.rs @@ -122,8 +122,8 @@ impl ArbitraryFrom<&SimValue> for LTValue { let index = rng.gen_range(0..t.len()); t[index] -= 1; // Mutate the rest of the string - for i in (index + 1)..t.len() { - t[i] = rng.gen_range('a' as u32..='z' as u32); + for val in t.iter_mut().skip(index + 1) { + *val = rng.gen_range('a' as u32..='z' as u32); } let t = t .into_iter() @@ -142,8 +142,8 @@ impl ArbitraryFrom<&SimValue> for LTValue { let index = rng.gen_range(0..b.len()); b[index] -= 1; // Mutate the rest of the blob - for i in (index + 1)..b.len() { - b[i] = rng.gen_range(0..=255); + for val in b.iter_mut().skip(index + 1) { + *val = rng.gen_range(0..=255); } Value::Blob(b) } @@ -184,8 +184,8 @@ impl ArbitraryFrom<&SimValue> for GTValue { let index = rng.gen_range(0..t.len()); t[index] += 1; // Mutate the rest of the string - for i in (index + 1)..t.len() { - t[i] = rng.gen_range('a' as u32..='z' as u32); + for val in t.iter_mut().skip(index + 1) { + *val = rng.gen_range('a' as u32..='z' as u32); } let t = t .into_iter() @@ -204,8 +204,8 @@ impl ArbitraryFrom<&SimValue> for GTValue { let index = rng.gen_range(0..b.len()); b[index] += 1; // Mutate the rest of the blob - for i in (index + 1)..b.len() { - b[i] = rng.gen_range(0..=255); + for val in b.iter_mut().skip(index + 1) { + *val = rng.gen_range(0..=255); } Value::Blob(b) } diff --git a/simulator/model/query/predicate.rs b/simulator/model/query/predicate.rs index 9e3c0f4eb..b3b6251d2 100644 --- a/simulator/model/query/predicate.rs +++ b/simulator/model/query/predicate.rs @@ -22,7 +22,7 @@ impl Predicate { pub(crate) fn test(&self, row: &[SimValue], table: &Table) -> bool { let value = expr_to_value(&self.0, row, table); - value.map_or(false, |value| value.into_bool()) + value.map_or(false, |value| value.as_bool()) } } diff --git a/simulator/model/table.rs b/simulator/model/table.rs index e304cab0d..b7e1a6a32 100644 --- a/simulator/model/table.rs +++ b/simulator/model/table.rs @@ -106,7 +106,7 @@ impl SimValue { pub const FALSE: Self = SimValue(types::Value::Integer(0)); pub const TRUE: Self = SimValue(types::Value::Integer(1)); - pub fn into_bool(&self) -> bool { + pub fn as_bool(&self) -> bool { Numeric::from(&self.0).try_into_bool().unwrap_or_default() } @@ -197,7 +197,7 @@ impl From<&ast::Literal> for SimValue { ast::Literal::Null => types::Value::Null, ast::Literal::Numeric(number) => Numeric::from(number).into(), // TODO: see how to avoid sanitizing here - ast::Literal::String(string) => types::Value::build_text(sanitize_string(&string)), + ast::Literal::String(string) => types::Value::build_text(sanitize_string(string)), ast::Literal::Blob(blob) => types::Value::Blob( blob.as_bytes() .chunks_exact(2) @@ -235,7 +235,11 @@ impl From<&SimValue> for ast::Literal { impl From for SimValue { fn from(value: bool) -> Self { - value.then_some(SimValue::TRUE).unwrap_or(SimValue::FALSE) + if value { + SimValue::TRUE + } else { + SimValue::FALSE + } } } diff --git a/simulator/runner/bugbase.rs b/simulator/runner/bugbase.rs index 9dba0c49e..c9b1e3a90 100644 --- a/simulator/runner/bugbase.rs +++ b/simulator/runner/bugbase.rs @@ -338,7 +338,7 @@ impl BugBase { } pub(crate) fn load_bugs(&mut self) -> anyhow::Result> { - let seeds = self.bugs.keys().map(|seed| *seed).collect::>(); + let seeds = self.bugs.keys().copied().collect::>(); seeds .iter() diff --git a/simulator/runner/file.rs b/simulator/runner/file.rs index 090be055f..fe9c9921e 100644 --- a/simulator/runner/file.rs +++ b/simulator/runner/file.rs @@ -57,8 +57,8 @@ impl SimulatorFile { "--------- -------- --------".to_string(), format!("total {:8} {:8}", sum_calls, sum_faults), ]; - let table = stats_table.join("\n"); - table + + stats_table.join("\n") } } diff --git a/stress/main.rs b/stress/main.rs index 527a31cba..2d77ed4da 100644 --- a/stress/main.rs +++ b/stress/main.rs @@ -5,7 +5,6 @@ use antithesis_sdk::random::{get_random, AntithesisRng}; use antithesis_sdk::*; use clap::Parser; use core::panic; -use hex; use limbo::Builder; use opts::Opts; use std::collections::HashSet; @@ -323,14 +322,12 @@ fn generate_plan(opts: &Opts) -> Result Result<(), Box> { let _g = init_tracing()?; antithesis_init(); - let mut opts = Opts::parse(); - + let opts = Opts::parse(); if opts.nr_threads > 1 { println!("ERROR: Multi-threaded data access is not yet supported: https://github.com/tursodatabase/limbo/issues/1552"); return Ok(()); } let plan = if opts.load_log { - println!("Loading plan from log file..."); - read_plan_from_log_file(&mut opts)? + read_plan_from_log_file(&opts)? } else { - println!("Generating plan..."); generate_plan(&opts)? }; @@ -461,23 +455,10 @@ async fn main() -> Result<(), Box> { let handle = tokio::spawn(async move { let conn = db.connect()?; - println!("\rExecuting queries..."); for query_index in 0..nr_iterations { let sql = &plan.queries_per_thread[thread][query_index]; - if !opts.silent { - if opts.verbose { - println!("executing query {}", sql); - } else { - if query_index % 100 == 0 { - print!( - "\r{:.2} %", - (query_index as f64 / nr_iterations as f64 * 100.0) - ); - std::io::stdout().flush().unwrap(); - } - } - } - if let Err(e) = conn.execute(&sql, ()).await { + println!("executing: {}", sql); + if let Err(e) = conn.execute(sql, ()).await { match e { limbo::Error::SqlExecutionFailure(e) => { if e.contains("Corrupt database") { diff --git a/testing/sqlite_test_ext/src/lib.rs b/testing/sqlite_test_ext/src/lib.rs index 6ccd206fe..3be581f25 100644 --- a/testing/sqlite_test_ext/src/lib.rs +++ b/testing/sqlite_test_ext/src/lib.rs @@ -7,6 +7,13 @@ extern "C" { } #[no_mangle] +/// Initialize the Limbo SQLite Test Extension. +/// +/// # Safety +/// +/// This function is unsafe because it interacts with raw pointers and FFI. +/// Caller must ensure that `db`, `err_msg`, and `api` are valid pointers, +/// and that the SQLite database handle is properly initialized. pub unsafe extern "C" fn sqlite3_limbosqlitetestext_init( db: *mut std::ffi::c_void, err_msg: *mut *mut i8, diff --git a/tests/integration/common.rs b/tests/integration/common.rs index bee439e69..f61132cf7 100644 --- a/tests/integration/common.rs +++ b/tests/integration/common.rs @@ -142,7 +142,7 @@ pub(crate) fn sqlite_exec_rows( conn: &rusqlite::Connection, query: &str, ) -> Vec> { - let mut stmt = conn.prepare(&query).unwrap(); + let mut stmt = conn.prepare(query).unwrap(); let mut rows = stmt.query(params![]).unwrap(); let mut results = Vec::new(); while let Some(row) = rows.next().unwrap() { @@ -221,8 +221,6 @@ pub(crate) fn limbo_exec_rows_error( #[cfg(test)] mod tests { use std::vec; - - use rand::Rng; use tempfile::TempDir; use super::{limbo_exec_rows, limbo_exec_rows_error, TempDatabase}; @@ -297,6 +295,8 @@ mod tests { #[test] #[cfg(feature = "index_experimental")] fn test_unique_index_ordering() -> anyhow::Result<()> { + use rand::Rng; + let db = TempDatabase::new_empty(); let conn = db.connect_limbo(); diff --git a/tests/integration/fuzz/grammar_generator.rs b/tests/integration/fuzz/grammar_generator.rs index 8222bdeb2..aa046f065 100644 --- a/tests/integration/fuzz/grammar_generator.rs +++ b/tests/integration/fuzz/grammar_generator.rs @@ -6,7 +6,7 @@ /// 2. Symbol -> [Int]: generate terminals which form integer from specified range /// 3. Symbol -> (Inner)?: generate expansion for Inner symbol with some probability /// 4. Symbol -> (Inner){n..m}: generate k expansions for Inner symbol where k \in [n..m) with uniform distribution -/// (note, that every repetition will be expanded independently) +/// (note, that every repetition will be expanded independently) /// 5. Symbol -> Inner1 Inner2 .. Inner[n]: concatenate expansions from inner symbols and insert separator string between them /// 6. Symbol -> Choice1 | Choice2 | .. | Choice[n]: pick random choice according to their weights randomly and generate expansion for it /// @@ -123,7 +123,7 @@ impl GrammarGenerator { root: SymbolHandle, is_recursive: &mut HashMap, ) -> bool { - if let Some(_) = is_recursive.get(&root) { + if is_recursive.get(&root).is_some() { is_recursive.insert(root, true); return true; } @@ -233,10 +233,10 @@ impl GrammarGenerator { values .iter() .filter(|x| is_recursive.get(&x.0) != Some(&true)) - .map(|x| *x) + .copied() .collect::>() }; - if handles.len() == 0 { + if handles.is_empty() { handles = values.clone(); } diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index 484ac89da..55104cfbe 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -2,9 +2,12 @@ pub mod grammar_generator; #[cfg(test)] mod tests { + #[cfg(feature = "index_experimental")] + use rand::seq::IndexedRandom; + #[cfg(feature = "index_experimental")] use std::collections::HashSet; - use rand::{seq::IndexedRandom, Rng, SeedableRng}; + use rand::{Rng, SeedableRng}; use rand_chacha::ChaCha8Rng; use rusqlite::params; @@ -400,7 +403,7 @@ mod tests { comp3.map(|x| format!("z {} {}", x, col_val_third.unwrap())), ] .into_iter() - .filter_map(|x| x) + .flatten() .collect::>(); let where_clause = if where_clause_components.is_empty() { "".to_string() @@ -415,7 +418,7 @@ mod tests { order_by3.map(|x| format!("z {}", x)), ] .into_iter() - .filter_map(|x| x) + .flatten() .collect::>(); let order_by = if order_by_components.is_empty() { "".to_string() @@ -436,7 +439,7 @@ mod tests { // Execute the query on all databases and compare the results for (i, sqlite_conn) in sqlite_conns.iter().enumerate() { let limbo = limbo_exec_rows(&dbs[i], &limbo_conns[i], &query); - let sqlite = sqlite_exec_rows(&sqlite_conn, &query); + let sqlite = sqlite_exec_rows(sqlite_conn, &query); if limbo != sqlite { // if the order by contains exclusively components that are constrained by an equality (=), // sqlite sometimes doesn't bother with ASC/DESC because it doesn't semantically matter @@ -457,7 +460,7 @@ mod tests { let query_no_limit = format!("SELECT * FROM t {} {} {}", where_clause, order_by, ""); let limbo_no_limit = limbo_exec_rows(&dbs[i], &limbo_conns[i], &query_no_limit); - let sqlite_no_limit = sqlite_exec_rows(&sqlite_conn, &query_no_limit); + let sqlite_no_limit = sqlite_exec_rows(sqlite_conn, &query_no_limit); let limbo_rev = limbo_no_limit.iter().cloned().rev().collect::>(); if limbo_rev == sqlite_no_limit && order_by_only_equalities { continue; @@ -988,6 +991,7 @@ mod tests { pub cast_expr: SymbolHandle, pub case_expr: SymbolHandle, pub cmp_op: SymbolHandle, + #[cfg(feature = "index_experimental")] pub number: SymbolHandle, } @@ -1222,10 +1226,12 @@ mod tests { cast_expr, case_expr, cmp_op, + #[cfg(feature = "index_experimental")] number, } } + #[cfg(feature = "index_experimental")] fn predicate_builders(g: &GrammarGenerator, tables: Option<&[TestTable]>) -> PredicateBuilders { let (in_op, in_op_builder) = g.create_handle(); let (column, column_builder) = g.create_handle(); @@ -1439,7 +1445,7 @@ mod tests { i += 1; } // verify the same number of rows in both tables - let query = format!("SELECT COUNT(*) FROM t"); + let query = "SELECT COUNT(*) FROM t".to_string(); let limbo = limbo_exec_rows(&db, &limbo_conn, &query); let sqlite = sqlite_exec_rows(&sqlite_conn, &query); assert_eq!(limbo, sqlite, "seed: {}", seed); diff --git a/tests/integration/query_processing/test_read_path.rs b/tests/integration/query_processing/test_read_path.rs index 8838819f2..c6dfb2d31 100644 --- a/tests/integration/query_processing/test_read_path.rs +++ b/tests/integration/query_processing/test_read_path.rs @@ -80,7 +80,7 @@ fn test_statement_bind() -> anyhow::Result<()> { } if let limbo_core::Value::Blob(v) = row.get::<&Value>(3).unwrap() { - assert_eq!(v.as_slice(), &vec![0x1 as u8, 0x2, 0x3]) + assert_eq!(v.as_slice(), &vec![0x1_u8, 0x2, 0x3]) } if let limbo_core::Value::Float(f) = row.get::<&Value>(4).unwrap() { diff --git a/tests/integration/query_processing/test_write_path.rs b/tests/integration/query_processing/test_write_path.rs index 313baec76..9963abdc9 100644 --- a/tests/integration/query_processing/test_write_path.rs +++ b/tests/integration/query_processing/test_write_path.rs @@ -96,8 +96,7 @@ fn test_sequential_overflow_page() -> anyhow::Result<()> { huge_texts.push(huge_text); } - for i in 0..iterations { - let huge_text = &huge_texts[i]; + for (i, huge_text) in huge_texts.iter().enumerate().take(iterations) { let insert_query = format!("INSERT INTO test VALUES ({}, '{}')", i, huge_text.as_str()); match conn.query(insert_query) { Ok(Some(ref mut rows)) => loop { @@ -168,7 +167,7 @@ fn test_sequential_write() -> anyhow::Result<()> { run_query(&tmp_db, &conn, &insert_query)?; let mut current_read_index = 0; - run_query_on_row(&tmp_db, &conn, &list_query, |row: &Row| { + run_query_on_row(&tmp_db, &conn, list_query, |row: &Row| { let first_value = row.get::<&Value>(0).expect("missing id"); let id = match first_value { limbo_core::Value::Integer(i) => *i as i32, diff --git a/vendored/sqlite3-parser/src/parser/ast/check.rs b/vendored/sqlite3-parser/src/parser/ast/check.rs index cbe2cacc7..2df65bc73 100644 --- a/vendored/sqlite3-parser/src/parser/ast/check.rs +++ b/vendored/sqlite3-parser/src/parser/ast/check.rs @@ -105,18 +105,15 @@ impl Stmt { match self { Self::AlterTable(alter_table) => { let (_, body) = &**alter_table; - match body { - AlterTableBody::AddColumn(cd) => { - for c in cd { - if let ColumnConstraint::PrimaryKey { .. } = c { - return Err(custom_err!("Cannot add a PRIMARY KEY column")); - } - if let ColumnConstraint::Unique(..) = c { - return Err(custom_err!("Cannot add a UNIQUE column")); - } + if let AlterTableBody::AddColumn(cd) = body { + for c in cd { + if let ColumnConstraint::PrimaryKey { .. } = c { + return Err(custom_err!("Cannot add a PRIMARY KEY column")); + } + if let ColumnConstraint::Unique(..) = c { + return Err(custom_err!("Cannot add a UNIQUE column")); } } - _ => {} } Ok(()) } @@ -164,10 +161,8 @@ impl Stmt { let Delete { order_by, limit, .. } = &**delete; - if let Some(_) = order_by { - if limit.is_none() { - return Err(custom_err!("ORDER BY without LIMIT on DELETE")); - } + if order_by.is_some() && limit.is_none() { + return Err(custom_err!("ORDER BY without LIMIT on DELETE")); } Ok(()) } @@ -177,7 +172,7 @@ impl Stmt { return Ok(()); } let columns = columns.as_ref().unwrap(); - match &*body { + match body { InsertBody::Select(select, ..) => match select.body.select.column_count() { ColumnCount::Fixed(n) if n != columns.len() => { Err(custom_err!("{} values for {} columns", n, columns.len())) @@ -193,10 +188,8 @@ impl Stmt { let Update { order_by, limit, .. } = &**update; - if let Some(_) = order_by { - if limit.is_none() { - return Err(custom_err!("ORDER BY without LIMIT on UPDATE")); - } + if order_by.is_some() && limit.is_none() { + return Err(custom_err!("ORDER BY without LIMIT on UPDATE")); } Ok(()) diff --git a/vendored/sqlite3-parser/src/to_sql_string/expr.rs b/vendored/sqlite3-parser/src/to_sql_string/expr.rs index 7727543da..e04c6bb2c 100644 --- a/vendored/sqlite3-parser/src/to_sql_string/expr.rs +++ b/vendored/sqlite3-parser/src/to_sql_string/expr.rs @@ -71,7 +71,7 @@ impl ToSqlString for Expr { Expr::Collate(expr, name) => { ret.push_str(&expr.to_sql_string(context)); ret.push_str(" COLLATE "); - ret.push_str(&name); + ret.push_str(name); } Expr::DoublyQualified(name, name1, name2) => { ret.push_str(&name.0); diff --git a/vendored/sqlite3-parser/src/to_sql_string/mod.rs b/vendored/sqlite3-parser/src/to_sql_string/mod.rs index d47617bce..af0bab5ef 100644 --- a/vendored/sqlite3-parser/src/to_sql_string/mod.rs +++ b/vendored/sqlite3-parser/src/to_sql_string/mod.rs @@ -23,7 +23,7 @@ pub trait ToSqlString { impl ToSqlString for Box { fn to_sql_string(&self, context: &C) -> String { - T::to_sql_string(&self, context) + T::to_sql_string(self, context) } } diff --git a/vendored/sqlite3-parser/src/to_sql_string/stmt/create_table.rs b/vendored/sqlite3-parser/src/to_sql_string/stmt/create_table.rs index 416d17e3a..4fef81042 100644 --- a/vendored/sqlite3-parser/src/to_sql_string/stmt/create_table.rs +++ b/vendored/sqlite3-parser/src/to_sql_string/stmt/create_table.rs @@ -44,7 +44,7 @@ impl ToSqlString for ast::NamedTableConstraint { self.constraint.to_sql_string(context) ) } else { - format!("{}", self.constraint.to_sql_string(context)) + self.constraint.to_sql_string(context).to_string() } } } diff --git a/vendored/sqlite3-parser/src/to_sql_string/stmt/create_trigger.rs b/vendored/sqlite3-parser/src/to_sql_string/stmt/create_trigger.rs index 8c734ad46..7ca70e38a 100644 --- a/vendored/sqlite3-parser/src/to_sql_string/stmt/create_trigger.rs +++ b/vendored/sqlite3-parser/src/to_sql_string/stmt/create_trigger.rs @@ -9,14 +9,22 @@ impl ToSqlString for ast::CreateTrigger { fn to_sql_string(&self, context: &C) -> String { format!( "CREATE{} TRIGGER {}{}{} {} ON {}{}{} BEGIN {} END;", - self.temporary.then_some(" TEMP").unwrap_or(""), - self.if_not_exists.then_some("IF NOT EXISTS ").unwrap_or(""), + if self.temporary { " TEMP" } else { "" }, + if self.if_not_exists { + "IF NOT EXISTS " + } else { + "" + }, self.trigger_name.to_sql_string(context), self.time .map_or("".to_string(), |time| format!(" {}", time)), self.event, self.tbl_name.to_sql_string(context), - self.for_each_row.then_some(" FOR EACH ROW").unwrap_or(""), + if self.for_each_row { + " FOR EACH ROW" + } else { + "" + }, self.when_clause .as_ref() .map_or("".to_string(), |expr| format!( diff --git a/vendored/sqlite3-parser/src/to_sql_string/stmt/create_virtual_table.rs b/vendored/sqlite3-parser/src/to_sql_string/stmt/create_virtual_table.rs index 027eb5128..5617867c3 100644 --- a/vendored/sqlite3-parser/src/to_sql_string/stmt/create_virtual_table.rs +++ b/vendored/sqlite3-parser/src/to_sql_string/stmt/create_virtual_table.rs @@ -4,7 +4,11 @@ impl ToSqlString for ast::CreateVirtualTable { fn to_sql_string(&self, context: &C) -> String { format!( "CREATE VIRTUAL TABLE {}{} USING {}{};", - self.if_not_exists.then_some("IF NOT EXISTS ").unwrap_or(""), + if self.if_not_exists { + "IF NOT EXISTS " + } else { + "" + }, self.tbl_name.to_sql_string(context), self.module_name.0, self.args diff --git a/vendored/sqlite3-parser/src/to_sql_string/stmt/mod.rs b/vendored/sqlite3-parser/src/to_sql_string/stmt/mod.rs index d28c3af2a..6692ae5f1 100644 --- a/vendored/sqlite3-parser/src/to_sql_string/stmt/mod.rs +++ b/vendored/sqlite3-parser/src/to_sql_string/stmt/mod.rs @@ -26,7 +26,7 @@ impl ToSqlString for ast::Stmt { if let Some(name) = name { format!("ANALYZE {};", name.to_sql_string(context)) } else { - format!("ANALYZE;") + "ANALYZE;".to_string() } } Self::Attach { @@ -211,15 +211,15 @@ mod tests { ($test_name:ident, $input:expr) => { #[test] fn $test_name() { - let context = crate::to_sql_string::stmt::tests::TestContext; + let context = $crate::to_sql_string::stmt::tests::TestContext; let input = $input.split_whitespace().collect::>().join(" "); - let mut parser = crate::lexer::sql::Parser::new(input.as_bytes()); + let mut parser = $crate::lexer::sql::Parser::new(input.as_bytes()); let cmd = fallible_iterator::FallibleIterator::next(&mut parser) .unwrap() .unwrap(); assert_eq!( input, - crate::to_sql_string::ToSqlString::to_sql_string(cmd.stmt(), &context) + $crate::to_sql_string::ToSqlString::to_sql_string(cmd.stmt(), &context) ); } }; @@ -227,15 +227,15 @@ mod tests { #[test] $(#[$attribute])* fn $test_name() { - let context = crate::to_sql_string::stmt::tests::TestContext; + let context = $crate::to_sql_string::stmt::tests::TestContext; let input = $input.split_whitespace().collect::>().join(" "); - let mut parser = crate::lexer::sql::Parser::new(input.as_bytes()); + let mut parser = $crate::lexer::sql::Parser::new(input.as_bytes()); let cmd = fallible_iterator::FallibleIterator::next(&mut parser) .unwrap() .unwrap(); assert_eq!( input, - crate::to_sql_string::ToSqlString::to_sql_string(cmd.stmt(), &context) + $crate::to_sql_string::ToSqlString::to_sql_string(cmd.stmt(), &context) ); } }