diff --git a/core/translate/delete.rs b/core/translate/delete.rs index 3786a0e00..301b1586d 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -36,7 +36,7 @@ pub fn translate_delete( approx_num_labels: 0, }; program.extend(&opts); - emit_program(&mut program, delete_plan, schema, syms)?; + emit_program(&mut program, delete_plan, schema, syms, |_| {})?; Ok(program) } diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 501c0d7b9..30224147b 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -19,7 +19,6 @@ use super::order_by::{emit_order_by, init_order_by, SortMetadata}; use super::plan::{ JoinOrderMember, Operation, QueryDestination, SelectPlan, TableReferences, UpdatePlan, }; -use super::schema::ParseSchema; use super::select::emit_simple_count; use super::subquery::emit_subqueries; use crate::error::SQLITE_CONSTRAINT_PRIMARYKEY; @@ -182,11 +181,12 @@ pub fn emit_program( plan: Plan, schema: &Schema, syms: &SymbolTable, + after: impl FnOnce(&mut ProgramBuilder), ) -> Result<()> { match plan { Plan::Select(plan) => emit_program_for_select(program, plan, schema, syms), Plan::Delete(plan) => emit_program_for_delete(program, plan, schema, syms), - Plan::Update(plan) => emit_program_for_update(program, plan, schema, syms), + Plan::Update(plan) => emit_program_for_update(program, plan, schema, syms, after), Plan::CompoundSelect { .. } => { emit_program_for_compound_select(program, plan, schema, syms) } @@ -834,6 +834,7 @@ fn emit_program_for_update( mut plan: UpdatePlan, schema: &Schema, syms: &SymbolTable, + after: impl FnOnce(&mut ProgramBuilder), ) -> Result<()> { let mut t_ctx = TranslateCtx::new( program, @@ -902,16 +903,6 @@ fn emit_program_for_update( )?; emit_update_insns(&plan, &t_ctx, program, index_cursors)?; - match plan.parse_schema { - ParseSchema::None => {} - ParseSchema::Reload => { - program.emit_insn(crate::vdbe::insn::Insn::ParseSchema { - db: usize::MAX, // TODO: This value is unused, change when we do something with it - where_clause: None, - }); - } - } - close_loop( program, &mut t_ctx, @@ -921,6 +912,8 @@ fn emit_program_for_update( program.preassign_label_to_next_insn(after_main_loop_label); + after(program); + // Finalize program program.epilogue(TransactionMode::Write); program.result_columns = plan.returning.unwrap_or_default(); diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 716479c6e..1e0b05ea8 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -31,11 +31,12 @@ pub(crate) mod update; mod values; use crate::fast_lock::SpinLock; -use crate::schema::Schema; +use crate::schema::{Column, Schema}; use crate::storage::pager::Pager; use crate::storage::sqlite3_ondisk::DatabaseHeader; use crate::translate::delete::translate_delete; use crate::vdbe::builder::{ProgramBuilder, ProgramBuilderOpts, QueryMode}; +use crate::vdbe::insn::{Insn, RegisterOrLiteral}; use crate::vdbe::Program; use crate::{bail_parse_error, Connection, LimboError, Result, SymbolTable}; use fallible_iterator::FallibleIterator as _; @@ -44,15 +45,14 @@ use insert::translate_insert; use limbo_sqlite3_parser::ast::{self, Delete, Insert}; use limbo_sqlite3_parser::lexer::sql::Parser; use schema::{ - translate_create_table, translate_create_virtual_table, translate_drop_table, ParseSchema, - SQLITE_TABLEID, + translate_create_table, translate_create_virtual_table, translate_drop_table, SQLITE_TABLEID, }; use select::translate_select; use std::rc::{Rc, Weak}; use std::sync::Arc; use tracing::{instrument, Level}; use transaction::{translate_tx_begin, translate_tx_commit}; -use update::translate_update; +use update::{translate_update, translate_update_with_after}; #[instrument(skip_all, level = Level::TRACE)] pub fn translate( @@ -112,27 +112,245 @@ pub fn translate_inner( ) -> Result { let program = match stmt { ast::Stmt::AlterTable(a) => { - let (table_name, alter_table) = a.as_ref(); + let (table_name, alter_table) = *a; + let ast::Name(table_name) = table_name.name; + + let Some(table) = schema.tables.get(&table_name) else { + return Err(LimboError::ParseError(format!( + "no such table: {table_name}" + ))); + }; + + let Some(btree) = table.btree() else { todo!() }; + let mut btree = (*btree).clone(); match alter_table { - ast::AlterTableBody::RenameTo(name) => { - let rename = &name.0; - let name = &table_name.name.0; + ast::AlterTableBody::DropColumn(column) => { + let ast::Name(column) = column; - let Some(table) = schema.tables.get(name) else { - return Err(LimboError::ParseError(format!("no such table: {name}"))); + // Tables always have at least one column. + assert_ne!(btree.columns.len(), 0); + + if btree.columns.len() == 1 { + return Err(LimboError::ParseError(format!( + "cannot drop column \"{column}\": no other columns exist" + ))); + } + + let (dropped_col, col) = btree + .columns + .iter() + .enumerate() + .find(|(_, Column { name, .. })| name.as_ref() == Some(&column)) + .ok_or_else(|| { + LimboError::ParseError(format!("no such column: \"{column}\"")) + })?; + + if col.primary_key { + return Err(LimboError::ParseError(format!( + "cannot drop column \"{column}\": PRIMARY KEY" + ))); + } + + if col.unique { + return Err(LimboError::ParseError(format!( + "cannot drop column \"{column}\": UNIQUE" + ))); + } + + btree.columns.remove(dropped_col); + + let sql = btree.to_sql(); + + let stmt = format!( + r#" + UPDATE {SQLITE_TABLEID} + SET sql = '{sql}' + WHERE tbl_name = '{table_name}' + "#, + ); + + let mut parser = Parser::new(stmt.as_bytes()); + let Some(ast::Cmd::Stmt(ast::Stmt::Update(mut update))) = parser.next()? else { + unreachable!(); }; - if schema.tables.contains_key(rename) { + translate_update_with_after( + QueryMode::Normal, + schema, + &mut update, + syms, + program, + |program| { + let loop_start = program.allocate_label(); + let loop_end = program.allocate_label(); + + let column_count = btree.columns.len(); + let root_page = btree.root_page; + let table_name = btree.name.clone(); + + let cursor_id = program.alloc_cursor_id( + crate::vdbe::builder::CursorType::BTreeTable(Rc::new(btree)), + ); + + program.emit_insn(Insn::OpenWrite { + cursor_id, + root_page: RegisterOrLiteral::Literal(root_page), + name: table_name.clone(), + }); + + program.emit_insn(Insn::Rewind { + cursor_id, + pc_if_empty: loop_end, + }); + + let rowid = program.alloc_register(); + + program.emit_insn(Insn::RowId { + cursor_id, + dest: rowid, + }); + + program.preassign_label_to_next_insn(loop_start); + + let first_column = program.alloc_registers(column_count); + + let mut iter = first_column; + + for i in 0..(column_count + 1) { + if i == dropped_col { + continue; + } + + program.emit_insn(Insn::Column { + cursor_id, + column: i, + dest: iter, + }); + + iter += 1; + } + + let record = program.alloc_register(); + + program.emit_insn(Insn::MakeRecord { + start_reg: first_column, + count: column_count, + dest_reg: record, + index_name: None, + }); + + program.emit_insn(Insn::Insert { + cursor: cursor_id, + key_reg: rowid, + record_reg: record, + flag: 0, + table_name: table_name.clone(), + }); + + program.emit_insn(Insn::Next { + cursor_id, + pc_if_next: loop_start, + }); + + program.preassign_label_to_next_insn(loop_end); + + program.emit_insn(Insn::ParseSchema { + db: usize::MAX, // TODO: This value is unused, change when we do something with it + where_clause: None, + }) + }, + )? + } + ast::AlterTableBody::AddColumn(col_def) => { + btree.columns.push(Column { + name: Some(col_def.col_name.0), + ty: crate::schema::Type::Null, + ty_str: "".to_string(), + primary_key: false, + is_rowid_alias: false, + notnull: false, + default: None, + unique: false, + }); + + let sql = btree.to_sql(); + + let stmt = format!( + r#" + UPDATE {SQLITE_TABLEID} + SET sql = '{sql}' + WHERE tbl_name = '{table_name}' + "#, + ); + + let mut parser = Parser::new(stmt.as_bytes()); + let Some(ast::Cmd::Stmt(ast::Stmt::Update(mut update))) = parser.next()? else { + unreachable!(); + }; + + translate_update_with_after( + QueryMode::Normal, + schema, + &mut update, + syms, + |program| { + program.emit_insn(Insn::ParseSchema { + db: usize::MAX, // TODO: This value is unused, change when we do something with it + where_clause: None, + }); + }, + )? + } + ast::AlterTableBody::RenameColumn { old, new } => { + let Some(column) = btree + .columns + .iter_mut() + .find(|column| column.name == Some(old.0.clone())) + else { + return Err(LimboError::ParseError(format!("no such column: \"{old}\""))); + }; + + column.name = Some(new.0.clone()); + + let sql = btree.to_sql(); + + let stmt = format!( + r#" + UPDATE {SQLITE_TABLEID} + SET sql = '{sql}' + WHERE tbl_name = '{table_name}' + "#, + ); + + let mut parser = Parser::new(stmt.as_bytes()); + let Some(ast::Cmd::Stmt(ast::Stmt::Update(mut update))) = parser.next()? else { + unreachable!(); + }; + + translate_update_with_after( + QueryMode::Normal, + schema, + &mut update, + syms, + |program| { + program.emit_insn(Insn::ParseSchema { + db: usize::MAX, // TODO: This value is unused, change when we do something with it + where_clause: None, + }); + }, + )? + } + ast::AlterTableBody::RenameTo(name) => { + let ast::Name(rename) = name; + + if schema.tables.contains_key(&rename) { return Err(LimboError::ParseError(format!( "there is already another table or index with this name: {rename}" ))); }; - let Some(btree) = table.btree() else { todo!() }; - - let mut btree = (*btree).clone(); - btree.name = rename.clone(); + btree.name = rename; let sql = btree.to_sql(); @@ -142,8 +360,9 @@ pub fn translate_inner( SET name = '{rename}' , tbl_name = '{rename}' , sql = '{sql}' - WHERE tbl_name = '{name}' + WHERE tbl_name = '{table_name}' "#, + rename = &btree.name, ); let mut parser = Parser::new(stmt.as_bytes()); @@ -151,16 +370,20 @@ pub fn translate_inner( unreachable!(); }; - translate_update( + translate_update_with_after( QueryMode::Normal, schema, &mut update, syms, - ParseSchema::Reload, program, + |program| { + program.emit_insn(Insn::ParseSchema { + db: usize::MAX, // TODO: This value is unused, change when we do something with it + where_clause: None, + }); + }, )? } - _ => todo!(), } } ast::Stmt::Analyze(_) => bail_parse_error!("ANALYZE not supported yet"), @@ -253,7 +476,6 @@ pub fn translate_inner( schema, &mut update, syms, - ParseSchema::None, program, )?, ast::Stmt::Vacuum(_, _) => bail_parse_error!("VACUUM not supported yet"), diff --git a/core/translate/plan.rs b/core/translate/plan.rs index de0a5b00f..fca387cf5 100644 --- a/core/translate/plan.rs +++ b/core/translate/plan.rs @@ -17,7 +17,7 @@ use crate::{schema::Type, types::SeekOp, util::can_pushdown_predicate}; use limbo_sqlite3_parser::ast::TableInternalId; -use super::{emitter::OperationMode, planner::determine_where_to_eval_term, schema::ParseSchema}; +use super::{emitter::OperationMode, planner::determine_where_to_eval_term}; #[derive(Debug, Clone)] pub struct ResultSetColumn { @@ -564,7 +564,6 @@ pub struct UpdatePlan { // whether the WHERE clause is always false pub contains_constant_false_condition: bool, pub indexes_to_update: Vec>, - pub parse_schema: ParseSchema, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 1b1f6c040..5d9435b06 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -24,12 +24,6 @@ use crate::{bail_parse_error, Result}; use limbo_ext::VTabKind; use limbo_sqlite3_parser::ast::{fmt::ToTokens, CreateVirtualTable}; -#[derive(Debug, Clone, Copy)] -pub enum ParseSchema { - None, - Reload, -} - pub fn translate_create_table( query_mode: QueryMode, tbl_name: ast::QualifiedName, diff --git a/core/translate/select.rs b/core/translate/select.rs index 392bfcf53..4aff03a63 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -79,7 +79,7 @@ pub fn translate_select( }; program.extend(&opts); - emit_program(&mut program, select_plan, schema, syms)?; + emit_program(&mut program, select_plan, schema, syms, |_| {})?; Ok(TranslateSelectResult { program, num_result_cols, diff --git a/core/translate/update.rs b/core/translate/update.rs index 9d3d72f9e..d4af63b0e 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -17,8 +17,6 @@ use super::plan::{ }; use super::planner::bind_column_references; use super::planner::{parse_limit, parse_where}; -use super::schema::ParseSchema; - /* * Update is simple. By default we scan the table, and for each row, we check the WHERE * clause. If it evaluates to true, we build the new record with the updated value and insert. @@ -53,15 +51,9 @@ pub fn translate_update( schema: &Schema, body: &mut Update, syms: &SymbolTable, - parse_schema: ParseSchema, mut program: ProgramBuilder, ) -> crate::Result { - let mut plan = prepare_update_plan( - schema, - body, - parse_schema, - &mut program.table_reference_counter, - )?; + let mut plan = prepare_update_plan(schema, body, &mut program.table_reference_counter)?; optimize_plan(&mut plan, schema)?; // TODO: freestyling these numbers let opts = ProgramBuilderOpts { @@ -71,14 +63,35 @@ pub fn translate_update( approx_num_labels: 4, }; program.extend(&opts); - emit_program(&mut program, plan, schema, syms)?; + emit_program(&mut program, plan, schema, syms, |_| {})?; + Ok(program) +} + +pub fn translate_update_with_after( + query_mode: QueryMode, + schema: &Schema, + body: &mut Update, + syms: &SymbolTable, + mut program: ProgramBuilder, + after: impl FnOnce(&mut ProgramBuilder), +) -> crate::Result { + let mut plan = prepare_update_plan(schema, body, &mut program.table_reference_counter)?; + optimize_plan(&mut plan, schema)?; + // TODO: freestyling these numbers + let opts = ProgramBuilderOpts { + query_mode, + num_cursors: 1, + approx_num_insns: 20, + approx_num_labels: 4, + }; + program.extend(&opts); + emit_program(&mut program, plan, schema, syms, after)?; Ok(program) } pub fn prepare_update_plan( schema: &Schema, body: &mut Update, - parse_schema: ParseSchema, table_ref_counter: &mut TableRefIdCounter, ) -> crate::Result { if body.with.is_some() { @@ -215,6 +228,5 @@ pub fn prepare_update_plan( offset, contains_constant_false_condition: false, indexes_to_update, - parse_schema, })) } diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index fa6a2a70e..2e687598c 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -531,7 +531,7 @@ pub fn insn_to_str( let cursor_type = &program.cursor_ref[*cursor_id].1; let column_name: Option<&String> = match cursor_type { CursorType::BTreeTable(table) => { - let name = table.columns.get(*column).unwrap().name.as_ref(); + let name = table.columns.get(*column).and_then(|v| v.name.as_ref()); name } CursorType::BTreeIndex(index) => { diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 091feceb7..e56c4f280 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -523,9 +523,9 @@ checksum = "8355be11b20d696c8f18f6cc018c4e372165b1fa8126cef092399c9951984ffa" [[package]] name = "libmimalloc-sys" -version = "0.1.39" +version = "0.1.42" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23aa6811d3bd4deb8a84dde645f943476d13b248d818edcf8ce0b2f37f036b44" +checksum = "ec9d6fac27761dabcd4ee73571cdb06b7022dc99089acbe5435691edffaac0f4" dependencies = [ "cc", "libc", @@ -566,8 +566,9 @@ dependencies = [ [[package]] name = "limbo_core" -version = "0.0.19" +version = "0.0.20" dependencies = [ + "bitflags", "built", "cfg_block", "chrono", @@ -590,7 +591,7 @@ dependencies = [ "rand", "regex", "regex-syntax", - "rustix", + "rustix 1.0.7", "ryu", "strum", "thiserror 1.0.69", @@ -599,7 +600,7 @@ dependencies = [ [[package]] name = "limbo_ext" -version = "0.0.19" +version = "0.0.20" dependencies = [ "chrono", "getrandom 0.3.1", @@ -608,7 +609,7 @@ dependencies = [ [[package]] name = "limbo_macros" -version = "0.0.19" +version = "0.0.20" dependencies = [ "proc-macro2", "quote", @@ -617,7 +618,7 @@ dependencies = [ [[package]] name = "limbo_sqlite3_parser" -version = "0.0.19" +version = "0.0.20" dependencies = [ "bitflags", "cc", @@ -636,7 +637,7 @@ dependencies = [ [[package]] name = "limbo_time" -version = "0.0.19" +version = "0.0.20" dependencies = [ "chrono", "limbo_ext", @@ -648,7 +649,7 @@ dependencies = [ [[package]] name = "limbo_uuid" -version = "0.0.19" +version = "0.0.20" dependencies = [ "limbo_ext", "mimalloc", @@ -661,6 +662,12 @@ version = "0.4.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d26c52dbd32dccf2d10cac7725f8eae5296885fb5703b261f7d0a0739ec807ab" +[[package]] +name = "linux-raw-sys" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd945864f07fe9f5371a27ad7b52a172b4b499999f1d97574c9fa68373937e12" + [[package]] name = "litemap" version = "0.7.5" @@ -691,21 +698,20 @@ checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "miette" -version = "7.5.0" +version = "7.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a955165f87b37fd1862df2a59547ac542c77ef6d17c666f619d1ad22dd89484" +checksum = "5f98efec8807c63c752b5bd61f862c165c115b0a35685bdcfd9238c7aeb592b7" dependencies = [ "cfg-if", "miette-derive", - "thiserror 1.0.69", "unicode-width", ] [[package]] name = "miette-derive" -version = "7.5.0" +version = "7.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf45bf44ab49be92fd1227a3be6fc6f617f1a337c06af54981048574d8783147" +checksum = "db5b29714e950dbb20d5e6f74f9dcec4edbcc1067bb7f8ed198c097b8c1a818b" dependencies = [ "proc-macro2", "quote", @@ -714,9 +720,9 @@ dependencies = [ [[package]] name = "mimalloc" -version = "0.1.43" +version = "0.1.46" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68914350ae34959d83f732418d51e2427a794055d0b9529f48259ac07af65633" +checksum = "995942f432bbb4822a7e9c3faa87a695185b0d09273ba85f097b54f4e458f2af" dependencies = [ "libmimalloc-sys", ] @@ -826,7 +832,7 @@ dependencies = [ "concurrent-queue", "hermit-abi", "pin-project-lite", - "rustix", + "rustix 0.38.44", "tracing", "windows-sys", ] @@ -949,7 +955,20 @@ dependencies = [ "bitflags", "errno", "libc", - "linux-raw-sys", + "linux-raw-sys 0.4.15", + "windows-sys", +] + +[[package]] +name = "rustix" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c71e83d6afe7ff64890ec6b71d6a69bb8a610ab78ce364b3352876bb4c801266" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys 0.9.4", "windows-sys", ] diff --git a/testing/alter_table.test b/testing/alter_table.test index 65a397c9b..fe9b6f7cb 100755 --- a/testing/alter_table.test +++ b/testing/alter_table.test @@ -9,3 +9,40 @@ do_execsql_test_on_specific_db {:memory:} alter-table-rename-table { ALTER TABLE t1 RENAME TO t2; SELECT tbl_name FROM sqlite_schema; } { "t2" } + +# ALTER TABLE _ RENAME _ TO _ +do_execsql_test_on_specific_db {:memory:} alter-table-rename-column { + CREATE TABLE t(a); + ALTER TABLE t RENAME a TO b; + SELECT sql FROM sqlite_schema; +} { "CREATE TABLE t(b)" } + +# ALTER TABLE _ ADD _ +do_execsql_test_on_specific_db {:memory:} alter-table-add-column { + CREATE TABLE t(a); + INSERT INTO t VALUES (1); + SELECT * FROM t; + + ALTER TABLE t ADD b; + SELECT sql FROM sqlite_schema; + SELECT * FROM t; +} { + "1" + "CREATE TABLE t(a, b)" + "1|" +} + +# ALTER TABLE _ DROP _ +do_execsql_test_on_specific_db {:memory:} alter-table-add-column { + CREATE TABLE t(a, b); + INSERT INTO t VALUES (1, 2); + SELECT * FROM t; + + ALTER TABLE t DROP b; + SELECT sql FROM sqlite_schema; + SELECT * FROM t; +} { + "1|2" + "CREATE TABLE t(a)" + "1" +}