From 7967cc5efc6536a83bc7a7d64a11d0d1dc580899 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Sun, 26 Jan 2025 16:22:04 +0200 Subject: [PATCH] core: Kill Rows wrapper struct It's just an useless wrapper, kill it. --- bindings/go/rs_src/rows.rs | 10 +++---- bindings/rust/src/lib.rs | 2 +- cli/app.rs | 12 ++++---- cli/import.rs | 2 +- core/benches/benchmark.rs | 6 ++-- core/lib.rs | 30 ++++--------------- core/util.rs | 10 +++++-- core/vdbe/mod.rs | 5 ++-- perf/latency/limbo/src/main.rs | 2 +- simulator/generation/plan.rs | 2 +- .../functions/test_function_rowid.rs | 8 ++--- .../query_processing/test_write_path.rs | 24 +++++++-------- 12 files changed, 49 insertions(+), 64 deletions(-) diff --git a/bindings/go/rs_src/rows.rs b/bindings/go/rs_src/rows.rs index c505924bc..ccd01b20e 100644 --- a/bindings/go/rs_src/rows.rs +++ b/bindings/go/rs_src/rows.rs @@ -2,17 +2,17 @@ use crate::{ statement::TursoStatement, types::{ResultCode, TursoValue}, }; -use limbo_core::{Rows, StepResult, Value}; +use limbo_core::{Statement, StepResult, Value}; use std::ffi::{c_char, c_void}; pub struct TursoRows<'a> { - rows: Rows, + rows: Statement, cursor: Option>>, stmt: Box>, } impl<'a> TursoRows<'a> { - pub fn new(rows: Rows, stmt: Box>) -> Self { + pub fn new(rows: Statement, stmt: Box>) -> Self { TursoRows { rows, stmt, @@ -40,7 +40,7 @@ pub extern "C" fn rows_next(ctx: *mut c_void) -> ResultCode { } let ctx = TursoRows::from_ptr(ctx); - match ctx.rows.next_row() { + match ctx.rows.step() { Ok(StepResult::Row(row)) => { ctx.cursor = Some(row.values); ResultCode::Row @@ -133,6 +133,6 @@ pub extern "C" fn free_rows(rows: *mut c_void) { return; } unsafe { - let _ = Box::from_raw(rows as *mut Rows); + let _ = Box::from_raw(rows as *mut Statement); } } diff --git a/bindings/rust/src/lib.rs b/bindings/rust/src/lib.rs index b2114045a..f19814c38 100644 --- a/bindings/rust/src/lib.rs +++ b/bindings/rust/src/lib.rs @@ -110,7 +110,7 @@ pub enum Params { pub struct Transaction {} pub struct Rows { - _inner: Rc, + _inner: Rc, } impl Rows { diff --git a/cli/app.rs b/cli/app.rs index 9c7e17d65..278bbca43 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -3,7 +3,7 @@ use crate::{ opcodes_dictionary::OPCODE_DESCRIPTIONS, }; use cli_table::{Cell, Table}; -use limbo_core::{Database, LimboError, Rows, StepResult, Value}; +use limbo_core::{Database, LimboError, Statement, StepResult, Value}; use clap::{Parser, ValueEnum}; use std::{ @@ -614,7 +614,7 @@ impl Limbo { fn print_query_result( &mut self, sql: &str, - mut output: Result, LimboError>, + mut output: Result, LimboError>, ) -> anyhow::Result<()> { match output { Ok(Some(ref mut rows)) => match self.opts.output_mode { @@ -624,7 +624,7 @@ impl Limbo { return Ok(()); } - match rows.next_row() { + match rows.step() { Ok(StepResult::Row(row)) => { for (i, value) in row.values.iter().enumerate() { if i > 0 { @@ -669,7 +669,7 @@ impl Limbo { } let mut table_rows: Vec> = vec![]; loop { - match rows.next_row() { + match rows.step() { Ok(StepResult::Row(row)) => { table_rows.push( row.values @@ -739,7 +739,7 @@ impl Limbo { Ok(Some(ref mut rows)) => { let mut found = false; loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { if let Some(Value::Text(schema)) = row.values.first() { let _ = self.write_fmt(format_args!("{};", schema)); @@ -796,7 +796,7 @@ impl Limbo { Ok(Some(ref mut rows)) => { let mut tables = String::new(); loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { if let Some(Value::Text(table)) = row.values.first() { tables.push_str(table); diff --git a/cli/import.rs b/cli/import.rs index a339bd276..b7557d58b 100644 --- a/cli/import.rs +++ b/cli/import.rs @@ -95,7 +95,7 @@ impl<'a> ImportFile<'a> { match self.conn.query(insert_string) { Ok(rows) => { if let Some(mut rows) = rows { - while let Ok(x) = rows.next_row() { + while let Ok(x) = rows.step() { match x { limbo_core::StepResult::IO => { self.io.run_once().unwrap(); diff --git a/core/benches/benchmark.rs b/core/benches/benchmark.rs index 184deac4a..9858a0c56 100644 --- a/core/benches/benchmark.rs +++ b/core/benches/benchmark.rs @@ -46,7 +46,7 @@ fn limbo_bench(criterion: &mut Criterion) { let io = io.clone(); b.iter(|| { let mut rows = stmt.query().unwrap(); - match rows.next_row().unwrap() { + match rows.step().unwrap() { limbo_core::StepResult::Row(row) => { assert_eq!(row.get::(0).unwrap(), 1); } @@ -74,7 +74,7 @@ fn limbo_bench(criterion: &mut Criterion) { let io = io.clone(); b.iter(|| { let mut rows = stmt.query().unwrap(); - match rows.next_row().unwrap() { + match rows.step().unwrap() { limbo_core::StepResult::Row(row) => { assert_eq!(row.get::(0).unwrap(), 1); } @@ -103,7 +103,7 @@ fn limbo_bench(criterion: &mut Criterion) { let io = io.clone(); b.iter(|| { let mut rows = stmt.query().unwrap(); - match rows.next_row().unwrap() { + match rows.step().unwrap() { limbo_core::StepResult::Row(row) => { assert_eq!(row.get::(0).unwrap(), 1); } diff --git a/core/lib.rs b/core/lib.rs index db59a4785..e6c812110 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -281,7 +281,7 @@ impl Connection { } } - pub fn query(self: &Rc, sql: impl Into) -> Result> { + pub fn query(self: &Rc, sql: impl Into) -> Result> { let sql = sql.into(); trace!("Querying: {}", sql); let mut parser = Parser::new(sql.as_bytes()); @@ -292,7 +292,7 @@ impl Connection { } } - pub(crate) fn run_cmd(self: &Rc, cmd: Cmd) -> Result> { + pub(crate) fn run_cmd(self: &Rc, cmd: Cmd) -> Result> { let db = self.db.clone(); let syms: &SymbolTable = &db.syms.borrow(); match cmd { @@ -306,7 +306,7 @@ impl Connection { syms, )?); let stmt = Statement::new(program, self.pager.clone()); - Ok(Some(Rows { stmt })) + Ok(Some(stmt)) } Cmd::Explain(stmt) => { let program = translate::translate( @@ -465,9 +465,9 @@ impl Statement { } } - pub fn query(&mut self) -> Result { + pub fn query(&mut self) -> Result { let stmt = Statement::new(self.program.clone(), self.pager.clone()); - Ok(Rows::new(stmt)) + Ok(stmt) } pub fn columns(&self) -> &[String] { @@ -512,24 +512,6 @@ impl<'a> Row<'a> { } } -pub struct Rows { - stmt: Statement, -} - -impl Rows { - pub fn new(stmt: Statement) -> Self { - Self { stmt } - } - - pub fn next_row(&mut self) -> Result> { - self.stmt.step() - } - - pub fn columns(&self) -> &[String] { - self.stmt.columns() - } -} - pub(crate) struct SymbolTable { pub functions: HashMap>, #[cfg(not(target_family = "wasm"))] @@ -605,7 +587,7 @@ impl<'a> QueryRunner<'a> { } impl Iterator for QueryRunner<'_> { - type Item = Result>; + type Item = Result>; fn next(&mut self) -> Option { match self.parser.next() { diff --git a/core/util.rs b/core/util.rs index 9e67313ea..c81747887 100644 --- a/core/util.rs +++ b/core/util.rs @@ -4,7 +4,7 @@ use sqlite3_parser::ast::{Expr, FunctionTail, Literal}; use crate::{ schema::{self, Schema}, - Result, Rows, StepResult, IO, + Result, Statement, StepResult, IO, }; // https://sqlite.org/lang_keywords.html @@ -25,11 +25,15 @@ pub fn normalize_ident(identifier: &str) -> String { pub const PRIMARY_KEY_AUTOMATIC_INDEX_NAME_PREFIX: &str = "sqlite_autoindex_"; -pub fn parse_schema_rows(rows: Option, schema: &mut Schema, io: Arc) -> Result<()> { +pub fn parse_schema_rows( + rows: Option, + schema: &mut Schema, + io: Arc, +) -> Result<()> { if let Some(mut rows) = rows { let mut automatic_indexes = Vec::new(); loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { let ty = row.get::<&str>(0)?; if ty != "table" && ty != "index" { diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 94a02e28a..4345aaa54 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -44,7 +44,7 @@ use crate::{ json::json_arrow_extract, json::json_arrow_shift_extract, json::json_error_position, json::json_extract, json::json_object, json::json_type, }; -use crate::{resolve_ext_path, Connection, Result, Rows, TransactionState, DATABASE_VERSION}; +use crate::{resolve_ext_path, Connection, Result, TransactionState, DATABASE_VERSION}; use datetime::{ exec_date, exec_datetime_full, exec_julianday, exec_strftime, exec_time, exec_unixepoch, }; @@ -2338,10 +2338,9 @@ impl Program { "SELECT * FROM sqlite_schema WHERE {}", where_clause ))?; - let rows = Rows { stmt }; let mut schema = RefCell::borrow_mut(&conn.schema); // TODO: This function below is synchronous, make it not async - parse_schema_rows(Some(rows), &mut schema, conn.pager.io.clone())?; + parse_schema_rows(Some(stmt), &mut schema, conn.pager.io.clone())?; state.pc += 1; } Insn::ShiftRight { lhs, rhs, dest } => { diff --git a/perf/latency/limbo/src/main.rs b/perf/latency/limbo/src/main.rs index b51ffb406..a7302e38a 100644 --- a/perf/latency/limbo/src/main.rs +++ b/perf/latency/limbo/src/main.rs @@ -36,7 +36,7 @@ fn main() { let mut rows = stmt.query().unwrap(); let mut count = 0; loop { - let row = rows.next_row().unwrap(); + let row = rows.step().unwrap(); match row { limbo_core::StepResult::Row(_) => { count += 1; diff --git a/simulator/generation/plan.rs b/simulator/generation/plan.rs index 3b124b8f8..2a794fbbc 100644 --- a/simulator/generation/plan.rs +++ b/simulator/generation/plan.rs @@ -414,7 +414,7 @@ impl Interaction { assert!(rows.is_some()); let mut rows = rows.unwrap(); let mut out = Vec::new(); - while let Ok(row) = rows.next_row() { + while let Ok(row) = rows.step() { match row { StepResult::Row(row) => { let mut r = Vec::new(); diff --git a/tests/integration/functions/test_function_rowid.rs b/tests/integration/functions/test_function_rowid.rs index 6655cee0d..54b72c680 100644 --- a/tests/integration/functions/test_function_rowid.rs +++ b/tests/integration/functions/test_function_rowid.rs @@ -11,7 +11,7 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> { let mut insert_query = conn.query("INSERT INTO test_rowid (id, val) VALUES (NULL, 'test1')")?; if let Some(ref mut rows) = insert_query { loop { - match rows.next_row()? { + match rows.step()? { StepResult::IO => { tmp_db.io.run_once()?; } @@ -25,7 +25,7 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> { let mut select_query = conn.query("SELECT last_insert_rowid()")?; if let Some(ref mut rows) = select_query { loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { if let Value::Integer(id) = row.values[0] { assert_eq!(id, 1, "First insert should have rowid 1"); @@ -44,7 +44,7 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> { // Test explicit rowid match conn.query("INSERT INTO test_rowid (id, val) VALUES (5, 'test2')") { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::IO => { tmp_db.io.run_once()?; } @@ -60,7 +60,7 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> { let mut last_id = 0; match conn.query("SELECT last_insert_rowid()") { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { if let Value::Integer(id) = row.values[0] { last_id = id; diff --git a/tests/integration/query_processing/test_write_path.rs b/tests/integration/query_processing/test_write_path.rs index 97b68a804..50d159a96 100644 --- a/tests/integration/query_processing/test_write_path.rs +++ b/tests/integration/query_processing/test_write_path.rs @@ -20,7 +20,7 @@ fn test_simple_overflow_page() -> anyhow::Result<()> { match conn.query(insert_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::IO => { tmp_db.io.run_once()?; } @@ -39,7 +39,7 @@ fn test_simple_overflow_page() -> anyhow::Result<()> { match conn.query(list_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { let first_value = &row.values[0]; let text = &row.values[1]; @@ -93,7 +93,7 @@ fn test_sequential_overflow_page() -> anyhow::Result<()> { let insert_query = format!("INSERT INTO test VALUES ({}, '{}')", i, huge_text.as_str()); match conn.query(insert_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::IO => { tmp_db.io.run_once()?; } @@ -112,7 +112,7 @@ fn test_sequential_overflow_page() -> anyhow::Result<()> { let mut current_index = 0; match conn.query(list_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { let first_value = &row.values[0]; let text = &row.values[1]; @@ -166,7 +166,7 @@ fn test_sequential_write() -> anyhow::Result<()> { let insert_query = format!("INSERT INTO test VALUES ({})", i); match conn.query(insert_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::IO => { tmp_db.io.run_once()?; } @@ -183,7 +183,7 @@ fn test_sequential_write() -> anyhow::Result<()> { let mut current_read_index = 0; match conn.query(list_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { let first_value = row.values.first().expect("missing id"); let id = match first_value { @@ -227,7 +227,7 @@ fn test_regression_multi_row_insert() -> anyhow::Result<()> { match conn.query(insert_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::IO => { tmp_db.io.run_once()?; } @@ -248,7 +248,7 @@ fn test_regression_multi_row_insert() -> anyhow::Result<()> { let mut actual_ids = Vec::new(); match conn.query(list_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { let first_value = row.values.first().expect("missing id"); let id = match first_value { @@ -334,7 +334,7 @@ fn test_wal_checkpoint() -> anyhow::Result<()> { conn.checkpoint()?; match conn.query(insert_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::IO => { tmp_db.io.run_once()?; } @@ -355,7 +355,7 @@ fn test_wal_checkpoint() -> anyhow::Result<()> { let mut current_index = 0; match conn.query(list_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { let first_value = &row.values[0]; let id = match first_value { @@ -394,7 +394,7 @@ fn test_wal_restart() -> anyhow::Result<()> { let insert_query = format!("INSERT INTO test VALUES ({})", i); match conn.query(insert_query) { Ok(Some(ref mut rows)) => loop { - match rows.next_row()? { + match rows.step()? { StepResult::IO => { tmp_db.io.run_once()?; } @@ -418,7 +418,7 @@ fn test_wal_restart() -> anyhow::Result<()> { loop { if let Some(ref mut rows) = conn.query(list_query)? { loop { - match rows.next_row()? { + match rows.step()? { StepResult::Row(row) => { let first_value = &row.values[0]; let count = match first_value {