diff --git a/core/benches/benchmark.rs b/core/benches/benchmark.rs index 008516a6f..0d278aa6c 100644 --- a/core/benches/benchmark.rs +++ b/core/benches/benchmark.rs @@ -1,6 +1,6 @@ use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion}; use pprof::criterion::{Output, PProfProfiler}; -use std::sync::Arc; +use std::{sync::Arc, time::Instant}; use turso_core::{Database, PlatformIO}; fn rusqlite_open() -> rusqlite::Connection { @@ -54,6 +54,70 @@ fn bench_open(criterion: &mut Criterion) { group.finish(); } +fn bench_alter(criterion: &mut Criterion) { + // https://github.com/tursodatabase/turso/issues/174 + // The rusqlite benchmark crashes on Mac M1 when using the flamegraph features + let enable_rusqlite = std::env::var("DISABLE_RUSQLITE_BENCHMARK").is_err(); + + if !std::fs::exists("../testing/schema_5k.db").unwrap() { + #[allow(clippy::arc_with_non_send_sync)] + let io = Arc::new(PlatformIO::new().unwrap()); + let db = Database::open_file(io.clone(), "../testing/schema_5k.db", false, false).unwrap(); + let conn = db.connect().unwrap(); + + for i in 0..5000 { + conn.execute( + format!("CREATE TABLE table_{i} ( id INTEGER PRIMARY KEY, name TEXT, value INTEGER, created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP )") + ).unwrap(); + } + } + + let mut group = criterion.benchmark_group("`ALTER TABLE _ RENAME TO _`"); + + group.bench_function(BenchmarkId::new("limbo_rename_table", ""), |b| { + #[allow(clippy::arc_with_non_send_sync)] + let io = Arc::new(PlatformIO::new().unwrap()); + let db = Database::open_file(io.clone(), "../testing/schema_5k.db", false, false).unwrap(); + let conn = db.connect().unwrap(); + b.iter_custom(|iters| { + (0..iters) + .map(|_| { + conn.execute("CREATE TABLE x(a)").unwrap(); + let elapsed = { + let start = Instant::now(); + conn.execute("ALTER TABLE x RENAME TO y").unwrap(); + start.elapsed() + }; + conn.execute("DROP TABLE y").unwrap(); + elapsed + }) + .sum() + }); + }); + + if enable_rusqlite { + group.bench_function(BenchmarkId::new("sqlite_rename_table", ""), |b| { + let conn = rusqlite::Connection::open("../testing/schema_5k.db").unwrap(); + b.iter_custom(|iters| { + (0..iters) + .map(|_| { + conn.execute("CREATE TABLE x(a)", ()).unwrap(); + let elapsed = { + let start = Instant::now(); + conn.execute("ALTER TABLE x RENAME TO y", ()).unwrap(); + start.elapsed() + }; + conn.execute("DROP TABLE y", ()).unwrap(); + elapsed + }) + .sum() + }); + }); + } + + group.finish(); +} + fn bench_prepare_query(criterion: &mut Criterion) { // https://github.com/tursodatabase/turso/issues/174 // The rusqlite benchmark crashes on Mac M1 when using the flamegraph features @@ -376,6 +440,6 @@ fn bench_insert_rows(criterion: &mut Criterion) { criterion_group! { name = benches; config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None))); - targets = bench_open, bench_prepare_query, bench_execute_select_1, bench_execute_select_rows, bench_execute_select_count, bench_insert_rows + targets = bench_open, bench_alter, bench_prepare_query, bench_execute_select_1, bench_execute_select_rows, bench_execute_select_count, bench_insert_rows } criterion_main!(benches); diff --git a/core/translate/alter.rs b/core/translate/alter.rs index 24a8f1062..bf2b96522 100644 --- a/core/translate/alter.rs +++ b/core/translate/alter.rs @@ -7,7 +7,7 @@ use crate::{ schema::{Column, Schema}, util::normalize_ident, vdbe::{ - builder::ProgramBuilder, + builder::{CursorType, ProgramBuilder}, insn::{Cookie, Insn, RegisterOrLiteral}, }, LimboError, Result, SymbolTable, @@ -106,9 +106,7 @@ pub fn translate_alter_table( let root_page = btree.root_page; let table_name = btree.name.clone(); - let cursor_id = program.alloc_cursor_id( - crate::vdbe::builder::CursorType::BTreeTable(original_btree), - ); + let cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(original_btree)); program.emit_insn(Insn::OpenWrite { cursor_id, @@ -249,9 +247,7 @@ pub fn translate_alter_table( .get_btree_table(SQLITE_TABLEID) .expect("sqlite_schema should be on schema"); - let cursor_id = program.alloc_cursor_id(crate::vdbe::builder::CursorType::BTreeTable( - sqlite_schema.clone(), - )); + let cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(sqlite_schema.clone())); program.emit_insn(Insn::OpenWrite { cursor_id, @@ -336,9 +332,7 @@ pub fn translate_alter_table( .get_btree_table(SQLITE_TABLEID) .expect("sqlite_schema should be on schema"); - let cursor_id = program.alloc_cursor_id(crate::vdbe::builder::CursorType::BTreeTable( - sqlite_schema.clone(), - )); + let cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(sqlite_schema.clone())); program.emit_insn(Insn::OpenWrite { cursor_id, @@ -399,9 +393,9 @@ pub fn translate_alter_table( p5: 0, }); - program.emit_insn(Insn::ParseSchema { - db: usize::MAX, // TODO: This value is unused, change when we do something with it - where_clause: None, + program.emit_insn(Insn::RenameTable { + from: table_name.to_owned(), + to: new_name.to_owned(), }); program.epilogue(TransactionMode::Write); diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 74917d6e7..56bb30e23 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1,6 +1,7 @@ #![allow(unused_variables)] use crate::function::AlterTableFunc; use crate::numeric::{NullableInteger, Numeric}; +use crate::schema::Table; use crate::storage::btree::{integrity_check, IntegrityCheckError, IntegrityCheckState}; use crate::storage::database::DatabaseFile; use crate::storage::page_cache::DumbLruPageCache; @@ -6778,6 +6779,52 @@ pub fn op_cast( Ok(InsnFunctionStepResult::Step) } +pub fn op_rename_table( + program: &Program, + state: &mut ProgramState, + insn: &Insn, + pager: &Rc, + mv_store: Option<&Arc>, +) -> Result { + let Insn::RenameTable { from, to } = insn else { + unreachable!("unexpected Insn {:?}", insn) + }; + + let conn = program.connection.clone(); + + conn.with_schema_mut(|schema| { + if let Some(mut indexes) = schema.indexes.remove(from) { + indexes.iter_mut().for_each(|index| { + let index = Arc::make_mut(index); + index.table_name = to.to_owned(); + }); + + schema.indexes.insert(to.to_owned(), indexes); + }; + + let mut table = schema + .tables + .remove(from) + .expect("table being renamed should be in schema"); + + { + let table = Arc::make_mut(&mut table); + + let Table::BTree(btree) = table else { + panic!("only btree tables can be renamed"); + }; + + let btree = Arc::make_mut(btree); + btree.name = to.to_owned(); + } + + schema.tables.insert(to.to_owned(), table); + }); + + state.pc += 1; + Ok(InsnFunctionStepResult::Step) +} + impl Value { pub fn exec_lower(&self) -> Option { match self { diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 76a969698..0fefedcc2 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -1627,6 +1627,15 @@ pub fn insn_to_str( 0, format!("affinity(r[{}]={:?})", *reg, affinity), ), + Insn::RenameTable { from, to } => ( + "RenameTable", + 0, + 0, + 0, + Value::build_text(""), + 0, + format!("rename_table({from}, {to})"), + ), }; format!( "{:<4} {:<17} {:<4} {:<4} {:<4} {:<13} {:<2} {}", diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index c36a87091..6b41d1900 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -1009,6 +1009,10 @@ pub enum Insn { roots: Vec, message_register: usize, }, + RenameTable { + from: String, + to: String, + }, } impl Insn { @@ -1137,6 +1141,7 @@ impl Insn { Insn::IdxDelete { .. } => execute::op_idx_delete, Insn::Count { .. } => execute::op_count, Insn::IntegrityCk { .. } => execute::op_integrity_check, + Insn::RenameTable { .. } => execute::op_rename_table, } } }