From d9f5cd870d11bc175a29d81fba53b4ae419799ce Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Sat, 29 Mar 2025 21:47:35 +0100 Subject: [PATCH] clippy --- bindings/go/rs_src/types.rs | 17 ------- cli/app.rs | 2 +- core/json/json_operations.rs | 4 +- core/json/mod.rs | 1 - core/storage/btree.rs | 17 ++++--- core/storage/sqlite3_ondisk.rs | 4 -- core/types.rs | 4 -- core/vdbe/mod.rs | 9 ++++ core/vdbe/sorter.rs | 2 +- .../functions/test_function_rowid.rs | 6 +-- .../query_processing/test_read_path.rs | 22 +++++---- .../query_processing/test_write_path.rs | 45 ++++--------------- tests/integration/wal/test_wal.rs | 6 +-- 13 files changed, 47 insertions(+), 92 deletions(-) diff --git a/bindings/go/rs_src/types.rs b/bindings/go/rs_src/types.rs index 712306a34..f58a4a0b0 100644 --- a/bindings/go/rs_src/types.rs +++ b/bindings/go/rs_src/types.rs @@ -161,23 +161,6 @@ impl LimboValue { } } } - pub fn from_value(value: &limbo_core::RefValue) -> Self { - match value { - limbo_core::RefValue::Integer(i) => { - LimboValue::new(ValueType::Integer, ValueUnion::from_int(*i)) - } - limbo_core::RefValue::Float(r) => { - LimboValue::new(ValueType::Real, ValueUnion::from_real(*r)) - } - limbo_core::RefValue::Text(s) => { - LimboValue::new(ValueType::Text, ValueUnion::from_str(s.as_str())) - } - limbo_core::RefValue::Blob(b) => { - LimboValue::new(ValueType::Blob, ValueUnion::from_bytes(b.to_slice())) - } - limbo_core::RefValue::Null => LimboValue::new(ValueType::Null, ValueUnion::from_null()), - } - } // The values we get from Go need to be temporarily owned by the statement until they are bound // then they can be cleaned up immediately afterwards diff --git a/cli/app.rs b/cli/app.rs index 4e3167200..aa6556869 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -5,7 +5,7 @@ use crate::{ opcodes_dictionary::OPCODE_DESCRIPTIONS, }; use comfy_table::{Attribute, Cell, CellAlignment, Color, ContentArrangement, Row, Table}; -use limbo_core::{Database, LimboError, OwnedValue, RefValue, Statement, StepResult}; +use limbo_core::{Database, LimboError, OwnedValue, Statement, StepResult}; use clap::{Parser, ValueEnum}; use rustyline::{history::DefaultHistory, Editor}; diff --git a/core/json/json_operations.rs b/core/json/json_operations.rs index 17369cc3c..ea0ba9560 100644 --- a/core/json/json_operations.rs +++ b/core/json/json_operations.rs @@ -1,4 +1,4 @@ -use std::{collections::VecDeque, rc::Rc}; +use std::collections::VecDeque; use crate::{types::OwnedValue, vdbe::Register}; @@ -283,8 +283,6 @@ pub fn jsonb_insert(args: &[Register], json_cache: &JsonCacheCell) -> crate::Res #[cfg(test)] mod tests { - use std::rc::Rc; - use crate::types::Text; use super::*; diff --git a/core/json/mod.rs b/core/json/mod.rs index 07d93b06d..d35fa13d5 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -22,7 +22,6 @@ use jsonb::{ElementType, Jsonb, JsonbHeader, PathOperationMode, SearchOperation, use ser::to_string_pretty; use serde::{Deserialize, Serialize}; use std::borrow::Cow; -use std::rc::Rc; use std::str::FromStr; #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] diff --git a/core/storage/btree.rs b/core/storage/btree.rs index cf24b3105..1b8dc6d66 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -7,8 +7,7 @@ use crate::storage::sqlite3_ondisk::{ use crate::MvCursor; use crate::types::{ - compare_immutable, compare_immutable_to_record, CursorResult, ImmutableRecord, OwnedValue, - RefValue, SeekKey, SeekOp, + compare_immutable, CursorResult, ImmutableRecord, OwnedValue, RefValue, SeekKey, SeekOp, }; use crate::{return_corrupt, LimboError, Result}; @@ -329,7 +328,7 @@ impl BTreeCursor { /// Move the cursor to the previous record and return it. /// Used in backwards iteration. - fn get_prev_record(&mut self) -> Result)>> { + fn get_prev_record(&mut self) -> Result>> { loop { let page = self.stack.top(); let cell_idx = self.stack.current_cell_index(); @@ -346,7 +345,7 @@ impl BTreeCursor { self.stack.pop(); } else { // moved to begin of btree - return Ok(CursorResult::Ok((None))); + return Ok(CursorResult::Ok(None)); } } // continue to next loop to get record from the new page @@ -398,7 +397,7 @@ impl BTreeCursor { first_overflow_page, payload_size, }) => { - let record = if let Some(next_page) = first_overflow_page { + if let Some(next_page) = first_overflow_page { return_if_io!(self.process_overflow_read(_payload, next_page, payload_size)) } else { crate::storage::sqlite3_ondisk::read_record( @@ -583,7 +582,7 @@ impl BTreeCursor { payload_size, }) => { assert!(predicate.is_none()); - let record = if let Some(next_page) = first_overflow_page { + if let Some(next_page) = first_overflow_page { return_if_io!(self.process_overflow_read( _payload, *next_page, @@ -609,7 +608,7 @@ impl BTreeCursor { self.stack.push(mem_page); continue; } - let record = if let Some(next_page) = first_overflow_page { + if let Some(next_page) = first_overflow_page { return_if_io!(self.process_overflow_read( payload, *next_page, @@ -662,7 +661,7 @@ impl BTreeCursor { first_overflow_page, payload_size, }) => { - let record = if let Some(next_page) = first_overflow_page { + if let Some(next_page) = first_overflow_page { return_if_io!(self.process_overflow_read( payload, *next_page, @@ -754,7 +753,7 @@ impl BTreeCursor { SeekOp::EQ => *cell_rowid == rowid_key, }; if found { - let record = if let Some(next_page) = first_overflow_page { + if let Some(next_page) = first_overflow_page { return_if_io!(self.process_overflow_read( payload, *next_page, diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index de5b7c4bb..f64e9e424 100644 --- a/core/storage/sqlite3_ondisk.rs +++ b/core/storage/sqlite3_ondisk.rs @@ -1081,10 +1081,6 @@ impl SmallVec { self.len += 1; } } - - pub fn has_extra_data(&self) -> bool { - self.len >= self.data.len() - } } pub fn read_record(payload: &[u8], reuse_immutable: &mut ImmutableRecord) -> Result<()> { diff --git a/core/types.rs b/core/types.rs index 674b0256c..1705603cf 100644 --- a/core/types.rs +++ b/core/types.rs @@ -4,15 +4,12 @@ use crate::error::LimboError; use crate::ext::{ExtValue, ExtValueType}; use crate::pseudo::PseudoCursor; use crate::storage::btree::BTreeCursor; -use crate::storage::buffer_pool; use crate::storage::sqlite3_ondisk::write_varint; use crate::vdbe::sorter::Sorter; use crate::vdbe::{Register, VTabOpaqueCursor}; use crate::Result; use std::cmp::Ordering; use std::fmt::Display; -use std::pin::Pin; -use std::rc::Rc; const MAX_REAL_SIZE: u8 = 15; @@ -1327,7 +1324,6 @@ impl RawSlice { #[cfg(test)] mod tests { use super::*; - use std::rc::Rc; #[test] fn test_serialize_null() { diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index b6a63cd95..a6f158023 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -4548,6 +4548,15 @@ impl Row { T::from_value(value) } + pub fn get_value<'a>(&'a self, idx: usize) -> &'a OwnedValue { + let value = unsafe { self.values.add(idx).as_ref().unwrap() }; + let value = match value { + Register::OwnedValue(owned_value) => owned_value, + _ => unreachable!("a row should be formed of values only"), + }; + value + } + pub fn get_values(&self) -> impl Iterator { let values = unsafe { std::slice::from_raw_parts(self.values, self.count) }; // This should be ownedvalues diff --git a/core/vdbe/sorter.rs b/core/vdbe/sorter.rs index 7b87a5387..584a29271 100644 --- a/core/vdbe/sorter.rs +++ b/core/vdbe/sorter.rs @@ -1,4 +1,4 @@ -use crate::types::{ImmutableRecord, Record}; +use crate::types::ImmutableRecord; use std::cmp::Ordering; pub struct Sorter { diff --git a/tests/integration/functions/test_function_rowid.rs b/tests/integration/functions/test_function_rowid.rs index 6afdae0ca..78a77ce9f 100644 --- a/tests/integration/functions/test_function_rowid.rs +++ b/tests/integration/functions/test_function_rowid.rs @@ -30,7 +30,7 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - if let limbo_core::RefValue::Integer(id) = row.get_value(0) { + if let limbo_core::OwnedValue::Integer(id) = row.get_value(0) { assert_eq!(*id, 1, "First insert should have rowid 1"); } } @@ -66,7 +66,7 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - if let limbo_core::RefValue::Integer(id) = row.get_value(0) { + if let limbo_core::OwnedValue::Integer(id) = row.get_value(0) { last_id = *id; } } @@ -112,7 +112,7 @@ fn test_integer_primary_key() -> anyhow::Result<()> { match select_query.step()? { StepResult::Row => { let row = select_query.row().unwrap(); - if let limbo_core::RefValue::Integer(id) = row.get_value(0) { + if let limbo_core::OwnedValue::Integer(id) = row.get_value(0) { rowids.push(*id); } } diff --git a/tests/integration/query_processing/test_read_path.rs b/tests/integration/query_processing/test_read_path.rs index 072db306f..7f525f6a9 100644 --- a/tests/integration/query_processing/test_read_path.rs +++ b/tests/integration/query_processing/test_read_path.rs @@ -15,7 +15,10 @@ fn test_statement_reset_bind() -> anyhow::Result<()> { match stmt.step()? { StepResult::Row => { let row = stmt.row().unwrap(); - assert_eq!(*row.get_value(0), limbo_core::OwnedValue::Integer(1)); + assert_eq!( + *row.get::<&OwnedValue>(0).unwrap(), + limbo_core::OwnedValue::Integer(1) + ); } StepResult::IO => tmp_db.io.run_once()?, _ => break, @@ -30,7 +33,10 @@ fn test_statement_reset_bind() -> anyhow::Result<()> { match stmt.step()? { StepResult::Row => { let row = stmt.row().unwrap(); - assert_eq!(*row.get_value(0), limbo_core::OwnedValue::Integer(2)); + assert_eq!( + *row.get::<&OwnedValue>(0).unwrap(), + limbo_core::OwnedValue::Integer(2) + ); } StepResult::IO => tmp_db.io.run_once()?, _ => break, @@ -63,23 +69,23 @@ fn test_statement_bind() -> anyhow::Result<()> { match stmt.step()? { StepResult::Row => { let row = stmt.row().unwrap(); - if let limbo_core::RefValue::Text(s) = row.get_value(0) { + if let limbo_core::OwnedValue::Text(s) = row.get::<&OwnedValue>(0).unwrap() { assert_eq!(s.as_str(), "hello") } - if let limbo_core::RefValue::Text(s) = row.get_value(1) { + if let limbo_core::OwnedValue::Text(s) = row.get::<&OwnedValue>(1).unwrap() { assert_eq!(s.as_str(), "hello") } - if let limbo_core::RefValue::Integer(i) = row.get_value(2) { + if let limbo_core::OwnedValue::Integer(i) = row.get::<&OwnedValue>(2).unwrap() { assert_eq!(*i, 42) } - if let limbo_core::RefValue::Blob(v) = row.get_value(3) { - assert_eq!(v.to_slice(), &vec![0x1 as u8, 0x2, 0x3]) + if let limbo_core::OwnedValue::Blob(v) = row.get::<&OwnedValue>(3).unwrap() { + assert_eq!(v.as_slice(), &vec![0x1 as u8, 0x2, 0x3]) } - if let limbo_core::RefValue::Float(f) = row.get_value(4) { + if let limbo_core::OwnedValue::Float(f) = row.get::<&OwnedValue>(4).unwrap() { assert_eq!(*f, 0.5) } } diff --git a/tests/integration/query_processing/test_write_path.rs b/tests/integration/query_processing/test_write_path.rs index 543e0678a..6955f2483 100644 --- a/tests/integration/query_processing/test_write_path.rs +++ b/tests/integration/query_processing/test_write_path.rs @@ -44,17 +44,8 @@ fn test_simple_overflow_page() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0); - let text = row.get_value(1); - let id = match first_value { - limbo_core::RefValue::Integer(i) => *i as i32, - limbo_core::RefValue::Float(f) => *f as i32, - _ => unreachable!(), - }; - let text = match text { - limbo_core::RefValue::Text(t) => t.as_str(), - _ => unreachable!(), - }; + let id = row.get::(0).unwrap(); + let text = row.get::<&str>(0).unwrap(); assert_eq!(1, id); compare_string(&huge_text, text); } @@ -120,17 +111,8 @@ fn test_sequential_overflow_page() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0); - let text = row.get_value(1); - let id = match first_value { - limbo_core::RefValue::Integer(i) => *i as i32, - limbo_core::RefValue::Float(f) => *f as i32, - _ => unreachable!(), - }; - let text = match text { - limbo_core::RefValue::Text(t) => t.as_str(), - _ => unreachable!(), - }; + let id = row.get::(0).unwrap(); + let text = row.get::(1).unwrap(); let huge_text = &huge_texts[current_index]; compare_string(huge_text, text); assert_eq!(current_index, id as usize); @@ -305,7 +287,7 @@ fn test_statement_reset() -> anyhow::Result<()> { StepResult::Row => { let row = stmt.row().unwrap(); assert_eq!( - *row.get::<&OwnedValue>(0), + *row.get::<&OwnedValue>(0).unwrap(), limbo_core::OwnedValue::Integer(1) ); break; @@ -322,7 +304,7 @@ fn test_statement_reset() -> anyhow::Result<()> { StepResult::Row => { let row = stmt.row().unwrap(); assert_eq!( - *row.get::<&OwnedValue>(0), + *row.get::<&OwnedValue>(0).unwrap(), limbo_core::OwnedValue::Integer(1) ); break; @@ -374,12 +356,7 @@ fn test_wal_checkpoint() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0); - let id = match first_value { - RefValue::Integer(i) => *i as i32, - RefValue::Float(f) => *f as i32, - _ => unreachable!(), - }; + let id = row.get::(0).unwrap(); assert_eq!(current_index, id as usize); current_index += 1; } @@ -438,13 +415,9 @@ fn test_wal_restart() -> anyhow::Result<()> { match rows.step()? { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0); - let count = match first_value { - RefValue::Integer(i) => i, - _ => unreachable!(), - }; + let count = row.get::(0).unwrap(); debug!("counted {}", count); - return Ok(*count as usize); + return Ok(count as usize); } StepResult::IO => { tmp_db.io.run_once()?; diff --git a/tests/integration/wal/test_wal.rs b/tests/integration/wal/test_wal.rs index 53790a7df..646995957 100644 --- a/tests/integration/wal/test_wal.rs +++ b/tests/integration/wal/test_wal.rs @@ -81,11 +81,7 @@ fn test_wal_1_writer_1_reader() -> Result<()> { match rows.step().unwrap() { StepResult::Row => { let row = rows.row().unwrap(); - let first_value = row.get_value(0); - let id = match first_value { - RefValue::Integer(i) => *i as i32, - _ => unreachable!(), - }; + let id = row.get::(0).unwrap(); assert_eq!(id, i); i += 1; }