Merge 'Direct schema mutation – add instruction' from Levy A.

Resolves #2378.
```
`ALTER TABLE _ RENAME TO _`/limbo_rename_table/
                        time:   [15.645 ms 15.741 ms 15.850 ms]
Found 12 outliers among 100 measurements (12.00%)
  8 (8.00%) high mild
  4 (4.00%) high severe
`ALTER TABLE _ RENAME TO _`/sqlite_rename_table/
                        time:   [34.728 ms 35.260 ms 35.955 ms]
Found 15 outliers among 100 measurements (15.00%)
  8 (8.00%) high mild
  7 (7.00%) high severe
  ```
<img width="1000" height="199" alt="image" src="https://github.com/user-
attachments/assets/ad943355-b57d-43d9-8a84-850461b8af41" />

Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>

Closes #2399
This commit is contained in:
Jussi Saurio
2025-08-04 16:55:38 +03:00
committed by GitHub
5 changed files with 134 additions and 15 deletions

View File

@@ -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);

View File

@@ -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);

View File

@@ -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<Pager>,
mv_store: Option<&Arc<MvStore>>,
) -> Result<InsnFunctionStepResult> {
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<Self> {
match self {

View File

@@ -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} {}",

View File

@@ -1009,6 +1009,10 @@ pub enum Insn {
roots: Vec<usize>,
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,
}
}
}