diff --git a/Dockerfile.antithesis b/Dockerfile.antithesis index 6f10a91bd..b3ce828d5 100644 --- a/Dockerfile.antithesis +++ b/Dockerfile.antithesis @@ -80,4 +80,6 @@ RUN chmod 777 -R /opt/antithesis/test/v1 RUN mkdir /opt/antithesis/catalog RUN ln -s /opt/antithesis/test/v1/bank-test/*.py /opt/antithesis/catalog -ENTRYPOINT ["/bin/docker-entrypoint.sh"] \ No newline at end of file +ENV RUST_BACKTRACE=1 + +ENTRYPOINT ["/bin/docker-entrypoint.sh"] diff --git a/antithesis-tests/bank-test/first_setup.py b/antithesis-tests/bank-test/first_setup.py index df833b96e..86580315d 100755 --- a/antithesis-tests/bank-test/first_setup.py +++ b/antithesis-tests/bank-test/first_setup.py @@ -20,7 +20,7 @@ cur.execute(f''' # randomly create up to 100 accounts with a balance up to 1e9 total = 0 -num_accts = get_random() % 100 +num_accts = get_random() % 100 + 1 for i in range(num_accts): bal = get_random() % 1e9 total += bal diff --git a/antithesis-tests/stress-composer/utils.py b/antithesis-tests/stress-composer/utils.py index f99052bf3..358e44670 100755 --- a/antithesis-tests/stress-composer/utils.py +++ b/antithesis-tests/stress-composer/utils.py @@ -4,17 +4,16 @@ from antithesis.random import get_random, random_choice def generate_random_identifier(type: str, num: int): return ''.join(type, '_', get_random() % num) -def generate_random_value(type: str): - match type: - case 'INTEGER': - return str(get_random() % 100) - case 'REAL': - return '{:.2f}'.format(get_random() % 100 / 100.0) - case 'TEXT': - return f"'{''.join(random_choice(string.ascii_lowercase) for _ in range(5))}'" - case 'BLOB': - return f"x'{''.join(random_choice(string.ascii_lowercase) for _ in range(5)).encode().hex()}'" - case 'NUMERIC': - return str(get_random() % 100) - case _: - return NULL \ No newline at end of file +def generate_random_value(type_str): + if type_str == 'INTEGER': + return str(get_random() % 100) + elif type_str == 'REAL': + return '{:.2f}'.format(get_random() % 100 / 100.0) + elif type_str == 'TEXT': + return f"'{''.join(random_choice(string.ascii_lowercase) for _ in range(5))}'" + elif type_str == 'BLOB': + return f"x'{''.join(random_choice(string.ascii_lowercase) for _ in range(5)).encode().hex()}'" + elif type_str == 'NUMERIC': + return str(get_random() % 100) + else: + return NULL diff --git a/cli/app.rs b/cli/app.rs index d214d139f..e1f0351a5 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -558,11 +558,13 @@ impl Limbo { } Ok(cmd) => match cmd.command { Command::Exit(args) => { + self.save_history(); std::process::exit(args.code); } Command::Quit => { let _ = self.writeln("Exiting Limbo SQL Shell."); let _ = self.close_conn(); + self.save_history(); std::process::exit(0) } Command::Open(args) => { @@ -641,6 +643,11 @@ impl Limbo { let _ = self.writeln(v); }); } + Command::ListIndexes(args) => { + if let Err(e) = self.display_indexes(args.tbl_name) { + let _ = self.writeln(e.to_string()); + } + } Command::Timer(timer_mode) => { self.opts.timer = match timer_mode.mode { TimerMode::On => true, @@ -916,6 +923,55 @@ impl Limbo { Ok(()) } + fn display_indexes(&mut self, maybe_table: Option) -> anyhow::Result<()> { + let sql = match maybe_table { + Some(ref tbl_name) => format!( + "SELECT name FROM sqlite_schema WHERE type='index' AND tbl_name = '{}' ORDER BY 1", + tbl_name + ), + None => String::from("SELECT name FROM sqlite_schema WHERE type='index' ORDER BY 1"), + }; + + match self.conn.query(&sql) { + Ok(Some(ref mut rows)) => { + let mut indexes = String::new(); + loop { + match rows.step()? { + StepResult::Row => { + let row = rows.row().unwrap(); + if let Ok(OwnedValue::Text(idx)) = row.get::<&OwnedValue>(0) { + indexes.push_str(idx.as_str()); + indexes.push(' '); + } + } + StepResult::IO => { + self.io.run_once()?; + } + StepResult::Interrupt => break, + StepResult::Done => break, + StepResult::Busy => { + let _ = self.writeln("database is busy"); + break; + } + } + } + if !indexes.is_empty() { + let _ = self.writeln(indexes.trim_end()); + } + } + Err(err) => { + if err.to_string().contains("no such table: sqlite_schema") { + return Err(anyhow::anyhow!("Unable to access database schema. The database may be using an older SQLite version or may not be properly initialized.")); + } else { + return Err(anyhow::anyhow!("Error querying schema: {}", err)); + } + } + Ok(None) => {} + } + + Ok(()) + } + fn display_tables(&mut self, pattern: Option<&str>) -> anyhow::Result<()> { let sql = match pattern { Some(pattern) => format!( @@ -1008,12 +1064,16 @@ impl Limbo { Ok(input) } } -} -impl Drop for Limbo { - fn drop(&mut self) { + fn save_history(&mut self) { if let Some(rl) = &mut self.rl { let _ = rl.save_history(HISTORY_FILE.as_path()); } } } + +impl Drop for Limbo { + fn drop(&mut self) { + self.save_history() + } +} diff --git a/cli/commands/args.rs b/cli/commands/args.rs index 750895049..4c36e6ef6 100644 --- a/cli/commands/args.rs +++ b/cli/commands/args.rs @@ -3,6 +3,12 @@ use clap_complete::{ArgValueCompleter, CompletionCandidate, PathCompleter}; use crate::{input::OutputMode, opcodes_dictionary::OPCODE_DESCRIPTIONS}; +#[derive(Debug, Clone, Args)] +pub struct IndexesArgs { + /// Name of table + pub tbl_name: Option, +} + #[derive(Debug, Clone, Args)] pub struct ExitArgs { /// Exit code diff --git a/cli/commands/mod.rs b/cli/commands/mod.rs index e01828517..bd94c6051 100644 --- a/cli/commands/mod.rs +++ b/cli/commands/mod.rs @@ -2,8 +2,8 @@ pub mod args; pub mod import; use args::{ - CwdArgs, EchoArgs, ExitArgs, LoadExtensionArgs, NullValueArgs, OpcodesArgs, OpenArgs, - OutputModeArgs, SchemaArgs, SetOutputArgs, TablesArgs, TimerArgs, + CwdArgs, EchoArgs, ExitArgs, IndexesArgs, LoadExtensionArgs, NullValueArgs, OpcodesArgs, + OpenArgs, OutputModeArgs, SchemaArgs, SetOutputArgs, TablesArgs, TimerArgs, }; use clap::Parser; use import::ImportArgs; @@ -72,6 +72,9 @@ pub enum Command { /// List vfs modules available #[command(name = "vfslist", display_name = ".vfslist")] ListVfs, + /// Show names of indexes + #[command(name = "indexes", display_name = ".indexes")] + ListIndexes(IndexesArgs), #[command(name = "timer", display_name = ".timer")] Timer(TimerArgs), } diff --git a/cli/input.rs b/cli/input.rs index eac5312dc..e20d5a71a 100644 --- a/cli/input.rs +++ b/cli/input.rs @@ -218,6 +218,8 @@ pub const AFTER_HELP_MSG: &str = r#"Usage Examples: 13. To list all available VFS: .listvfs +14. To show names of indexes: + .indexes ?TABLE? Note: - All SQL commands must end with a semicolon (;). diff --git a/core/storage/btree.rs b/core/storage/btree.rs index d191227a4..185887afc 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -165,21 +165,13 @@ enum DeleteState { cell_idx: usize, original_child_pointer: Option, }, - DropCell { - cell_idx: usize, - }, CheckNeedsBalancing, - StartBalancing { - target_key: DeleteSavepoint, - }, WaitForBalancingToComplete { target_key: DeleteSavepoint, }, SeekAfterBalancing { target_key: DeleteSavepoint, }, - StackRetreat, - Finish, } #[derive(Clone)] @@ -1043,18 +1035,8 @@ impl BTreeCursor { loop { if min > max { if let Some(leftmost_matching_cell) = leftmost_matching_cell { - self.stack.set_cell_index(leftmost_matching_cell as i32); - let matching_cell = contents.cell_get( - leftmost_matching_cell, - payload_overflow_threshold_max( - contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), + let left_child_page = contents.cell_table_interior_read_left_child_page( + leftmost_matching_cell as usize, )?; // If we found our target rowid in the left subtree, // we need to move the parent cell pointer forwards or backwards depending on the iteration direction. @@ -1064,19 +1046,15 @@ impl BTreeCursor { // this parent: rowid 666 // left child has: 664,665,666 // we need to move to the previous parent (with e.g. rowid 663) when iterating backwards. - self.stack.next_cell_in_direction(iter_dir); - let BTreeCell::TableInteriorCell(TableInteriorCell { - _left_child_page, - .. - }) = matching_cell - else { - unreachable!("unexpected cell type: {:?}", matching_cell); - }; - let mem_page = self.pager.read_page(_left_child_page as usize)?; + let index_change = + -1 + (iter_dir == IterationDirection::Forwards) as i32 * 2; + self.stack + .set_cell_index(leftmost_matching_cell as i32 + index_change); + let mem_page = self.pager.read_page(left_child_page as usize)?; self.stack.push(mem_page); continue 'outer; } - self.stack.set_cell_index(contents.cell_count() as i32 + 1); + self.stack.set_cell_index(cell_count as i32 + 1); match contents.rightmost_pointer() { Some(right_most_pointer) => { let mem_page = self.pager.read_page(right_most_pointer as usize)?; @@ -1088,8 +1066,7 @@ impl BTreeCursor { } } } - let cur_cell_idx = (min + max) / 2; - self.stack.set_cell_index(cur_cell_idx as i32); + let cur_cell_idx = (min + max) >> 1; // rustc generates extra insns for (min+max)/2 due to them being isize. we know min&max are >=0 here. let cell_rowid = contents.cell_table_interior_read_rowid(cur_cell_idx as usize)?; // in sqlite btrees left child pages have <= keys. // table btrees can have a duplicate rowid in the interior cell, so for example if we are looking for rowid=10, @@ -1201,7 +1178,7 @@ impl BTreeCursor { continue 'outer; } - let cur_cell_idx = (min + max) / 2; + let cur_cell_idx = (min + max) >> 1; // rustc generates extra insns for (min+max)/2 due to them being isize. we know min&max are >=0 here. self.stack.set_cell_index(cur_cell_idx as i32); let cell = contents.cell_get( cur_cell_idx as usize, @@ -1321,7 +1298,6 @@ impl BTreeCursor { let Some(nearest_matching_cell) = nearest_matching_cell else { return Ok(CursorResult::Ok(None)); }; - self.stack.set_cell_index(nearest_matching_cell as i32); let matching_cell = contents.cell_get( nearest_matching_cell, payload_overflow_threshold_max( @@ -1350,13 +1326,16 @@ impl BTreeCursor { first_overflow_page, payload_size )); - self.stack.next_cell_in_direction(iter_dir); - + let cell_idx = if iter_dir == IterationDirection::Forwards { + nearest_matching_cell as i32 + 1 + } else { + nearest_matching_cell as i32 - 1 + }; + self.stack.set_cell_index(cell_idx as i32); return Ok(CursorResult::Ok(Some(cell_rowid))); } - let cur_cell_idx = (min + max) / 2; - self.stack.set_cell_index(cur_cell_idx as i32); + let cur_cell_idx = (min + max) >> 1; // rustc generates extra insns for (min+max)/2 due to them being isize. we know min&max are >=0 here. let cell_rowid = contents.cell_table_leaf_read_rowid(cur_cell_idx as usize)?; let cmp = cell_rowid.cmp(&rowid); @@ -1398,7 +1377,12 @@ impl BTreeCursor { first_overflow_page, payload_size )); - self.stack.next_cell_in_direction(iter_dir); + let cell_idx = if iter_dir == IterationDirection::Forwards { + cur_cell_idx + 1 + } else { + cur_cell_idx - 1 + }; + self.stack.set_cell_index(cell_idx as i32); return Ok(CursorResult::Ok(Some(cell_rowid))); } @@ -1524,7 +1508,7 @@ impl BTreeCursor { return Ok(CursorResult::Ok(Some(rowid))); } - let cur_cell_idx = (min + max) / 2; + let cur_cell_idx = (min + max) >> 1; // rustc generates extra insns for (min+max)/2 due to them being isize. we know min&max are >=0 here. self.stack.set_cell_index(cur_cell_idx as i32); let cell = contents.cell_get( @@ -3538,15 +3522,12 @@ impl BTreeCursor { /// 1. Start -> check if the rowid to be delete is present in the page or not. If not we early return /// 2. LoadPage -> load the page. /// 3. FindCell -> find the cell to be deleted in the page. - /// 4. ClearOverflowPages -> clear overflow pages associated with the cell. here if the cell is a leaf page go to DropCell state - /// or else go to InteriorNodeReplacement + /// 4. ClearOverflowPages -> Clear the overflow pages if there are any before dropping the cell, then if we are in a leaf page we just drop the cell in place. + /// if we are in interior page, we need to rotate keys in order to replace current cell (InteriorNodeReplacement). /// 5. InteriorNodeReplacement -> we copy the left subtree leaf node into the deleted interior node's place. - /// 6. DropCell -> only for leaf nodes. drop the cell. - /// 7. CheckNeedsBalancing -> check if balancing is needed. If yes, move to StartBalancing else move to StackRetreat - /// 8. WaitForBalancingToComplete -> perform balancing - /// 9. SeekAfterBalancing -> adjust the cursor to a node that is closer to the deleted value. go to Finish - /// 10. StackRetreat -> perform stack retreat for cursor positioning. only when balancing is not needed. go to Finish - /// 11. Finish -> Delete operation is done. Return CursorResult(Ok()) + /// 6. WaitForBalancingToComplete -> perform balancing + /// 7. SeekAfterBalancing -> adjust the cursor to a node that is closer to the deleted value. go to Finish + /// 8. Finish -> Delete operation is done. Return CursorResult(Ok()) pub fn delete(&mut self) -> Result> { assert!(self.mv_cursor.is_none()); @@ -3562,10 +3543,13 @@ impl BTreeCursor { let delete_info = self.state.delete_info().expect("cannot get delete info"); delete_info.state.clone() }; + tracing::debug!("delete state: {:?}", delete_state); match delete_state { DeleteState::Start => { let page = self.stack.top(); + page.set_dirty(); + self.pager.add_dirty(page.get().id); if matches!( page.get_contents().page_type(), PageType::TableLeaf | PageType::TableInterior @@ -3654,7 +3638,11 @@ impl BTreeCursor { original_child_pointer, }; } else { - delete_info.state = DeleteState::DropCell { cell_idx }; + let contents = page.get().contents.as_mut().unwrap(); + drop_cell(contents, cell_idx, self.usable_space() as u16)?; + + let delete_info = self.state.mut_delete_info().unwrap(); + delete_info.state = DeleteState::CheckNeedsBalancing; } } @@ -3732,33 +3720,9 @@ impl BTreeCursor { delete_info.state = DeleteState::CheckNeedsBalancing; } - DeleteState::DropCell { cell_idx } => { - let page = self.stack.top(); - return_if_locked!(page); - - if !page.is_loaded() { - self.pager.load_page(page.clone())?; - return Ok(CursorResult::IO); - } - - page.set_dirty(); - self.pager.add_dirty(page.get().id); - - let contents = page.get().contents.as_mut().unwrap(); - drop_cell(contents, cell_idx, self.usable_space() as u16)?; - - let delete_info = self.state.mut_delete_info().unwrap(); - delete_info.state = DeleteState::CheckNeedsBalancing; - } - DeleteState::CheckNeedsBalancing => { let page = self.stack.top(); - return_if_locked!(page); - - if !page.is_loaded() { - self.pager.load_page(page.clone())?; - return Ok(CursorResult::IO); - } + return_if_locked_maybe_load!(self.pager, page); let contents = page.get().contents.as_ref().unwrap(); let free_space = compute_free_space(contents, self.usable_space() as u16); @@ -3772,24 +3736,20 @@ impl BTreeCursor { let delete_info = self.state.mut_delete_info().unwrap(); if needs_balancing { - delete_info.state = DeleteState::StartBalancing { target_key }; + if delete_info.balance_write_info.is_none() { + let mut write_info = WriteInfo::new(); + write_info.state = WriteState::BalanceStart; + delete_info.balance_write_info = Some(write_info); + } + + delete_info.state = DeleteState::WaitForBalancingToComplete { target_key } } else { - delete_info.state = DeleteState::StackRetreat; + self.stack.retreat(); + self.state = CursorState::None; + return Ok(CursorResult::Ok(())); } } - DeleteState::StartBalancing { target_key } => { - let delete_info = self.state.mut_delete_info().unwrap(); - - if delete_info.balance_write_info.is_none() { - let mut write_info = WriteInfo::new(); - write_info.state = WriteState::BalanceStart; - delete_info.balance_write_info = Some(write_info); - } - - delete_info.state = DeleteState::WaitForBalancingToComplete { target_key } - } - DeleteState::WaitForBalancingToComplete { target_key } => { let delete_info = self.state.mut_delete_info().unwrap(); @@ -3814,6 +3774,7 @@ impl BTreeCursor { } CursorResult::IO => { + // Move to seek state // Save balance progress and return IO let write_info = match &self.state { CursorState::Write(wi) => wi.clone(), @@ -3838,19 +3799,6 @@ impl BTreeCursor { }; return_if_io!(self.seek(key, SeekOp::EQ)); - let delete_info = self.state.mut_delete_info().unwrap(); - delete_info.state = DeleteState::Finish; - delete_info.balance_write_info = None; - } - - DeleteState::StackRetreat => { - self.stack.retreat(); - let delete_info = self.state.mut_delete_info().unwrap(); - delete_info.state = DeleteState::Finish; - delete_info.balance_write_info = None; - } - - DeleteState::Finish => { self.state = CursorState::None; return Ok(CursorResult::Ok(())); } @@ -5238,8 +5186,10 @@ mod tests { fast_lock::SpinLock, io::{Buffer, Completion, MemoryIO, OpenFlags, IO}, storage::{ - database::DatabaseFile, page_cache::DumbLruPageCache, sqlite3_ondisk, - sqlite3_ondisk::DatabaseHeader, + database::DatabaseFile, + page_cache::DumbLruPageCache, + pager::CreateBTreeFlags, + sqlite3_ondisk::{self, DatabaseHeader}, }, types::Text, vdbe::Register, @@ -5757,6 +5707,81 @@ mod tests { } } } + + fn btree_index_insert_fuzz_run(attempts: usize, inserts: usize) { + let (mut rng, seed) = if std::env::var("SEED").is_ok() { + let seed = std::env::var("SEED").unwrap(); + let seed = seed.parse::().unwrap(); + let rng = ChaCha8Rng::seed_from_u64(seed); + (rng, seed) + } else { + rng_from_time() + }; + let mut seen = HashSet::new(); + tracing::info!("super seed: {}", seed); + for _ in 0..attempts { + let (pager, _) = empty_btree(); + let index_root_page = pager.btree_create(&CreateBTreeFlags::new_index()); + let index_root_page = index_root_page as usize; + let mut cursor = BTreeCursor::new(None, pager.clone(), index_root_page); + let mut keys = Vec::new(); + tracing::info!("seed: {}", seed); + for _ in 0..inserts { + let key = { + let result; + loop { + let cols = (0..10) + .map(|_| (rng.next_u64() % (1 << 30)) as i64) + .collect::>(); + if seen.contains(&cols) { + continue; + } else { + seen.insert(cols.clone()); + } + result = cols; + break; + } + result + }; + keys.push(key.clone()); + let value = ImmutableRecord::from_registers( + &key.iter() + .map(|col| Register::OwnedValue(OwnedValue::Integer(*col))) + .collect::>(), + ); + run_until_done( + || { + cursor.insert( + &BTreeKey::new_index_key(&value), + cursor.is_write_in_progress(), + ) + }, + pager.deref(), + ) + .unwrap(); + keys.sort(); + cursor.move_to_root(); + } + keys.sort(); + cursor.move_to_root(); + for key in keys.iter() { + tracing::trace!("seeking key: {:?}", key); + run_until_done(|| cursor.next(), pager.deref()).unwrap(); + let record = cursor.record(); + let record = record.as_ref().unwrap(); + let cursor_key = record.get_values(); + assert_eq!( + cursor_key, + &key.iter() + .map(|col| RefValue::Integer(*col)) + .collect::>(), + "key {:?} is not found", + key + ); + } + } + } + #[test] pub fn test_drop_odd() { let db = get_database(); @@ -5810,6 +5835,11 @@ mod tests { } } + #[test] + pub fn btree_index_insert_fuzz_run_equal_size() { + btree_index_insert_fuzz_run(2, 1024 * 32); + } + #[test] pub fn btree_insert_fuzz_run_random() { btree_insert_fuzz_run(128, 16, |rng| (rng.next_u32() % 4096) as usize); diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index 8e091ef64..200dd5490 100644 --- a/core/storage/sqlite3_ondisk.rs +++ b/core/storage/sqlite3_ondisk.rs @@ -606,7 +606,7 @@ impl PageContent { /// Read the rowid of a table interior cell. #[inline(always)] pub fn cell_table_interior_read_rowid(&self, idx: usize) -> Result { - assert!(self.page_type() == PageType::TableInterior); + debug_assert!(self.page_type() == PageType::TableInterior); let buf = self.as_ptr(); const INTERIOR_PAGE_HEADER_SIZE_BYTES: usize = 12; let cell_pointer_array_start = INTERIOR_PAGE_HEADER_SIZE_BYTES; @@ -617,10 +617,27 @@ impl PageContent { Ok(rowid) } + /// Read the left child page of a table interior cell. + #[inline(always)] + pub fn cell_table_interior_read_left_child_page(&self, idx: usize) -> Result { + debug_assert!(self.page_type() == PageType::TableInterior); + let buf = self.as_ptr(); + const INTERIOR_PAGE_HEADER_SIZE_BYTES: usize = 12; + let cell_pointer_array_start = INTERIOR_PAGE_HEADER_SIZE_BYTES; + let cell_pointer = cell_pointer_array_start + (idx * 2); + let cell_pointer = self.read_u16(cell_pointer) as usize; + Ok(u32::from_be_bytes([ + buf[cell_pointer], + buf[cell_pointer + 1], + buf[cell_pointer + 2], + buf[cell_pointer + 3], + ])) + } + /// Read the rowid of a table leaf cell. #[inline(always)] pub fn cell_table_leaf_read_rowid(&self, idx: usize) -> Result { - assert!(self.page_type() == PageType::TableLeaf); + debug_assert!(self.page_type() == PageType::TableLeaf); let buf = self.as_ptr(); const LEAF_PAGE_HEADER_SIZE_BYTES: usize = 8; let cell_pointer_array_start = LEAF_PAGE_HEADER_SIZE_BYTES; diff --git a/core/translate/delete.rs b/core/translate/delete.rs index fb580b8e8..5cb38cf42 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -50,6 +50,11 @@ pub fn prepare_delete_plan( crate::bail_corrupt_error!("Table is neither a virtual table nor a btree table"); }; let name = tbl_name.name.0.as_str().to_string(); + let indexes = schema + .get_indices(table.get_name()) + .iter() + .cloned() + .collect(); let mut table_references = vec![TableReference { table, identifier: name, @@ -82,6 +87,7 @@ pub fn prepare_delete_plan( limit: resolved_limit, offset: resolved_offset, contains_constant_false_condition: false, + indexes, }; Ok(Plan::Delete(plan)) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 86283fa64..77a3efdce 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -2,13 +2,16 @@ // It handles translating high-level SQL operations into low-level bytecode that can be executed by the virtual machine. use std::rc::Rc; +use std::sync::Arc; use limbo_sqlite3_parser::ast::{self}; use crate::function::Func; +use crate::schema::Index; use crate::translate::plan::{DeletePlan, Plan, Search}; use crate::util::exprs_are_equivalent; use crate::vdbe::builder::ProgramBuilder; +use crate::vdbe::insn::RegisterOrLiteral; use crate::vdbe::{insn::Insn, BranchOffset}; use crate::{Result, SymbolTable}; @@ -373,7 +376,13 @@ fn emit_program_for_delete( &plan.table_references, &plan.where_clause, )?; - emit_delete_insns(program, &mut t_ctx, &plan.table_references, &plan.limit)?; + emit_delete_insns( + program, + &mut t_ctx, + &plan.table_references, + &plan.indexes, + &plan.limit, + )?; // Clean up and close the main execution loop close_loop(program, &mut t_ctx, &plan.table_references)?; @@ -390,6 +399,7 @@ fn emit_delete_insns( program: &mut ProgramBuilder, t_ctx: &mut TranslateCtx, table_references: &[TableReference], + index_references: &[Arc], limit: &Option, ) -> Result<()> { let table_reference = table_references.first().unwrap(); @@ -405,11 +415,12 @@ fn emit_delete_insns( }, _ => return Ok(()), }; + let main_table_cursor_id = program.resolve_cursor_id(table_reference.table.get_name()); // Emit the instructions to delete the row let key_reg = program.alloc_register(); program.emit_insn(Insn::RowId { - cursor_id, + cursor_id: main_table_cursor_id, dest: key_reg, }); @@ -430,7 +441,43 @@ fn emit_delete_insns( conflict_action, }); } else { - program.emit_insn(Insn::Delete { cursor_id }); + for index in index_references { + let index_cursor_id = program.alloc_cursor_id( + Some(index.name.clone()), + crate::vdbe::builder::CursorType::BTreeIndex(index.clone()), + ); + + program.emit_insn(Insn::OpenWrite { + cursor_id: index_cursor_id, + root_page: RegisterOrLiteral::Literal(index.root_page), + }); + let num_regs = index.columns.len() + 1; + let start_reg = program.alloc_registers(num_regs); + // Emit columns that are part of the index + index + .columns + .iter() + .enumerate() + .for_each(|(reg_offset, column_index)| { + program.emit_insn(Insn::Column { + cursor_id: main_table_cursor_id, + column: column_index.pos_in_table, + dest: start_reg + reg_offset, + }); + }); + program.emit_insn(Insn::RowId { + cursor_id: main_table_cursor_id, + dest: start_reg + num_regs - 1, + }); + program.emit_insn(Insn::IdxDelete { + start_reg, + num_regs, + cursor_id: index_cursor_id, + }); + } + program.emit_insn(Insn::Delete { + cursor_id: main_table_cursor_id, + }); } if let Some(limit) = limit { let limit_reg = program.alloc_register(); diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 79ccb1fe9..dba075ac2 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -1,8 +1,12 @@ use limbo_sqlite3_parser::ast::{self, UnaryOperator}; +use super::emitter::Resolver; +use super::optimizer::Optimizable; +use super::plan::{Operation, TableReference}; #[cfg(feature = "json")] use crate::function::JsonFunc; use crate::function::{Func, FuncCtx, MathFuncArity, ScalarFunc, VectorFunc}; +use crate::functions::datetime; use crate::schema::{Table, Type}; use crate::util::{exprs_are_equivalent, normalize_ident}; use crate::vdbe::{ @@ -12,10 +16,6 @@ use crate::vdbe::{ }; use crate::Result; -use super::emitter::Resolver; -use super::optimizer::Optimizable; -use super::plan::{Operation, TableReference}; - #[derive(Debug, Clone, Copy)] pub struct ConditionMetadata { pub jump_if_condition_is_true: bool, @@ -2020,9 +2020,27 @@ pub fn translate_expr( }); Ok(target_register) } - ast::Literal::CurrentDate => todo!(), - ast::Literal::CurrentTime => todo!(), - ast::Literal::CurrentTimestamp => todo!(), + ast::Literal::CurrentDate => { + program.emit_insn(Insn::String8 { + value: datetime::exec_date(&[]).to_string(), + dest: target_register, + }); + Ok(target_register) + } + ast::Literal::CurrentTime => { + program.emit_insn(Insn::String8 { + value: datetime::exec_time(&[]).to_string(), + dest: target_register, + }); + Ok(target_register) + } + ast::Literal::CurrentTimestamp => { + program.emit_insn(Insn::String8 { + value: datetime::exec_datetime_full(&[]).to_string(), + dest: target_register, + }); + Ok(target_register) + } }, ast::Expr::Name(_) => todo!(), ast::Expr::NotNull(_) => todo!(), diff --git a/core/translate/plan.rs b/core/translate/plan.rs index 44a43f73a..ad6514247 100644 --- a/core/translate/plan.rs +++ b/core/translate/plan.rs @@ -297,6 +297,8 @@ pub struct DeletePlan { pub offset: Option, /// query contains a constant condition that is always false pub contains_constant_false_condition: bool, + /// Indexes that must be updated by the delete operation. + pub indexes: Vec>, } #[derive(Debug, Clone)] diff --git a/core/types.rs b/core/types.rs index b2a7a053b..6ed66cc0c 100644 --- a/core/types.rs +++ b/core/types.rs @@ -903,6 +903,26 @@ impl ImmutableRecord { } } +impl Display for ImmutableRecord { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + for value in &self.values { + match value { + RefValue::Null => write!(f, "NULL")?, + RefValue::Integer(i) => write!(f, "Integer({})", *i)?, + RefValue::Float(flo) => write!(f, "Float({})", *flo)?, + RefValue::Text(text_ref) => write!(f, "Text({})", text_ref.as_str())?, + RefValue::Blob(raw_slice) => { + write!(f, "Blob({})", String::from_utf8_lossy(raw_slice.to_slice()))? + } + } + if value != self.values.last().unwrap() { + write!(f, ", ")?; + } + } + Ok(()) + } +} + impl Clone for ImmutableRecord { fn clone(&self) -> Self { let mut new_values = Vec::new(); @@ -1402,6 +1422,7 @@ impl SeekOp { /// A seek with SeekOp::LE implies: /// Find the last table/index key that compares less than or equal to the seek key /// -> used in backwards iteration. + #[inline(always)] pub fn iteration_direction(&self) -> IterationDirection { match self { SeekOp::EQ | SeekOp::GE | SeekOp::GT => IterationDirection::Forwards, diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 928f7f94a..1a1af59d2 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -3,6 +3,7 @@ use crate::numeric::{NullableInteger, Numeric}; use crate::storage::database::FileMemoryStorage; use crate::storage::page_cache::DumbLruPageCache; use crate::storage::pager::CreateBTreeFlags; +use crate::types::ImmutableRecord; use crate::{ error::{LimboError, SQLITE_CONSTRAINT, SQLITE_CONSTRAINT_PRIMARYKEY}, ext::ExtValue, @@ -1810,11 +1811,17 @@ pub fn op_row_id( let rowid = { let mut index_cursor = state.get_cursor(index_cursor_id); let index_cursor = index_cursor.as_btree_mut(); - index_cursor.rowid()? + let record = index_cursor.record(); + let record = record.as_ref().unwrap(); + let rowid = record.get_values().last().unwrap(); + match rowid { + RefValue::Integer(rowid) => *rowid as u64, + _ => unreachable!(), + } }; let mut table_cursor = state.get_cursor(table_cursor_id); let table_cursor = table_cursor.as_btree_mut(); - match table_cursor.seek(SeekKey::TableRowId(rowid.unwrap()), SeekOp::EQ)? { + match table_cursor.seek(SeekKey::TableRowId(rowid), SeekOp::EQ)? { CursorResult::Ok(_) => None, CursorResult::IO => Some((index_cursor_id, table_cursor_id)), } @@ -2069,7 +2076,6 @@ pub fn op_idx_ge( let idx_values = idx_record.get_values(); let idx_values = &idx_values[..record_from_regs.len()]; let record_values = record_from_regs.get_values(); - let record_values = &record_values[..idx_values.len()]; let ord = compare_immutable(&idx_values, &record_values, cursor.index_key_sort_order); if ord.is_ge() { target_pc.to_offset_int() @@ -3751,6 +3757,11 @@ pub fn op_delete( { let mut cursor = state.get_cursor(*cursor_id); let cursor = cursor.as_btree_mut(); + tracing::debug!( + "op_delete(record={:?}, rowid={:?})", + cursor.record(), + cursor.rowid()? + ); return_if_io!(cursor.delete()); } let prev_changes = program.n_change.get(); @@ -3759,6 +3770,71 @@ pub fn op_delete( Ok(InsnFunctionStepResult::Step) } +pub enum OpIdxDeleteState { + Seeking(ImmutableRecord), // First seek row to delete + Deleting, +} +pub fn op_idx_delete( + program: &Program, + state: &mut ProgramState, + insn: &Insn, + pager: &Rc, + mv_store: Option<&Rc>, +) -> Result { + let Insn::IdxDelete { + cursor_id, + start_reg, + num_regs, + } = insn + else { + unreachable!("unexpected Insn {:?}", insn) + }; + loop { + match &state.op_idx_delete_state { + Some(OpIdxDeleteState::Seeking(record)) => { + { + let mut cursor = state.get_cursor(*cursor_id); + let cursor = cursor.as_btree_mut(); + return_if_io!(cursor.seek(SeekKey::IndexKey(&record), SeekOp::EQ)); + tracing::debug!( + "op_idx_delete(seek={}, record={} rowid={:?})", + &record, + cursor.record().as_ref().unwrap(), + cursor.rowid() + ); + if cursor.rowid()?.is_none() { + // If P5 is not zero, then raise an SQLITE_CORRUPT_INDEX error if no matching + // index entry is found. This happens when running an UPDATE or DELETE statement and the + // index entry to be updated or deleted is not found. For some uses of IdxDelete + // (example: the EXCEPT operator) it does not matter that no matching entry is found. + // For those cases, P5 is zero. Also, do not raise this (self-correcting and non-critical) error if in writable_schema mode. + return Err(LimboError::Corrupt(format!( + "IdxDelete: no matching index entry found for record {:?}", + record + ))); + } + } + state.op_idx_delete_state = Some(OpIdxDeleteState::Deleting); + } + Some(OpIdxDeleteState::Deleting) => { + { + let mut cursor = state.get_cursor(*cursor_id); + let cursor = cursor.as_btree_mut(); + return_if_io!(cursor.delete()); + } + let n_change = program.n_change.get(); + program.n_change.set(n_change + 1); + state.pc += 1; + return Ok(InsnFunctionStepResult::Step); + } + None => { + let record = make_record(&state.registers, start_reg, num_regs); + state.op_idx_delete_state = Some(OpIdxDeleteState::Seeking(record)); + } + } + } +} + pub fn op_idx_insert( program: &Program, state: &mut ProgramState, diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index eadb5a0d9..fbdeaa6c2 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -1037,6 +1037,19 @@ pub fn insn_to_str( 0, "".to_string(), ), + Insn::IdxDelete { + cursor_id, + start_reg, + num_regs, + } => ( + "IdxDelete", + *cursor_id as i32, + *start_reg as i32, + *num_regs as i32, + OwnedValue::build_text(""), + 0, + "".to_string(), + ), Insn::NewRowid { cursor, rowid_reg, diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 6f310f746..fc64f59d1 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -650,6 +650,12 @@ pub enum Insn { cursor_id: CursorID, }, + IdxDelete { + start_reg: usize, + num_regs: usize, + cursor_id: CursorID, + }, + NewRowid { cursor: CursorID, // P1 rowid_reg: usize, // P2 Destination register to store the new rowid @@ -961,6 +967,7 @@ impl Insn { Insn::Once { .. } => execute::op_once, Insn::NotFound { .. } => execute::op_not_found, Insn::Affinity { .. } => execute::op_affinity, + Insn::IdxDelete { .. } => execute::op_idx_delete, } } } diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 45b23c538..9652dafef 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -43,7 +43,7 @@ use crate::CheckpointStatus; #[cfg(feature = "json")] use crate::json::JsonCacheCell; use crate::{Connection, MvStore, Result, TransactionState}; -use execute::{InsnFunction, InsnFunctionStepResult}; +use execute::{InsnFunction, InsnFunctionStepResult, OpIdxDeleteState}; use rand::{ distributions::{Distribution, Uniform}, @@ -257,6 +257,7 @@ pub struct ProgramState { halt_state: Option, #[cfg(feature = "json")] json_cache: JsonCacheCell, + op_idx_delete_state: Option, } impl ProgramState { @@ -280,6 +281,7 @@ impl ProgramState { halt_state: None, #[cfg(feature = "json")] json_cache: JsonCacheCell::new(), + op_idx_delete_state: None, } } diff --git a/fuzz/fuzz_targets/cast_real.rs b/fuzz/fuzz_targets/cast_real.rs index 65f550ec8..4ef4ab2ba 100644 --- a/fuzz/fuzz_targets/cast_real.rs +++ b/fuzz/fuzz_targets/cast_real.rs @@ -1,5 +1,6 @@ #![no_main] use libfuzzer_sys::{fuzz_target, Corpus}; +use limbo_core::numeric::StrToF64; use std::error::Error; fn do_fuzz(text: String) -> Result> { @@ -10,8 +11,11 @@ fn do_fuzz(text: String) -> Result> { })? }; - let actual = limbo_core::numeric::atof(&text) - .map(|(non_nan, _)| f64::from(non_nan)) + let actual = limbo_core::numeric::str_to_f64(&text) + .map(|v| { + let (StrToF64::Fractional(non_nan) | StrToF64::Decimal(non_nan)) = v; + f64::from(non_nan) + }) .unwrap_or(0.0); assert_eq!(expected, actual); diff --git a/tests/integration/query_processing/test_write_path.rs b/tests/integration/query_processing/test_write_path.rs index e948ed5d1..9c6107d58 100644 --- a/tests/integration/query_processing/test_write_path.rs +++ b/tests/integration/query_processing/test_write_path.rs @@ -461,3 +461,128 @@ fn test_insert_after_big_blob() -> anyhow::Result<()> { Ok(()) } + +#[test_log::test] +#[ignore = "this takes too long :)"] +fn test_write_delete_with_index() -> anyhow::Result<()> { + let _ = env_logger::try_init(); + + maybe_setup_tracing(); + + let tmp_db = TempDatabase::new_with_rusqlite("CREATE TABLE test (x PRIMARY KEY);"); + let conn = tmp_db.connect_limbo(); + + let list_query = "SELECT * FROM test"; + let max_iterations = 1000; + for i in 0..max_iterations { + println!("inserting {} ", i); + if (i % 100) == 0 { + let progress = (i as f64 / max_iterations as f64) * 100.0; + println!("progress {:.1}%", progress); + } + let insert_query = format!("INSERT INTO test VALUES ({})", i); + match conn.query(insert_query) { + Ok(Some(ref mut rows)) => loop { + match rows.step()? { + StepResult::IO => { + tmp_db.io.run_once()?; + } + StepResult::Done => break, + _ => unreachable!(), + } + }, + Ok(None) => {} + Err(err) => { + eprintln!("{}", err); + } + }; + } + for i in 0..max_iterations { + println!("deleting {} ", i); + if (i % 100) == 0 { + let progress = (i as f64 / max_iterations as f64) * 100.0; + println!("progress {:.1}%", progress); + } + let delete_query = format!("delete from test where x={}", i); + match conn.query(delete_query) { + Ok(Some(ref mut rows)) => loop { + match rows.step()? { + StepResult::IO => { + tmp_db.io.run_once()?; + } + StepResult::Done => break, + _ => unreachable!(), + } + }, + Ok(None) => {} + Err(err) => { + eprintln!("{}", err); + } + }; + println!("listing after deleting {} ", i); + let mut current_read_index = i + 1; + match conn.query(list_query) { + Ok(Some(ref mut rows)) => loop { + match rows.step()? { + StepResult::Row => { + let row = rows.row().unwrap(); + let first_value = row.get::<&OwnedValue>(0).expect("missing id"); + let id = match first_value { + limbo_core::OwnedValue::Integer(i) => *i as i32, + limbo_core::OwnedValue::Float(f) => *f as i32, + _ => unreachable!(), + }; + assert_eq!(current_read_index, id); + current_read_index += 1; + } + StepResult::IO => { + tmp_db.io.run_once()?; + } + StepResult::Interrupt => break, + StepResult::Done => break, + StepResult::Busy => { + panic!("Database is busy"); + } + } + }, + Ok(None) => {} + Err(err) => { + eprintln!("{}", err); + } + } + for i in i + 1..max_iterations { + // now test with seek + match conn.query(format!("select * from test where x = {}", i)) { + Ok(Some(ref mut rows)) => loop { + match rows.step()? { + StepResult::Row => { + let row = rows.row().unwrap(); + let first_value = row.get::<&OwnedValue>(0).expect("missing id"); + let id = match first_value { + limbo_core::OwnedValue::Integer(i) => *i as i32, + limbo_core::OwnedValue::Float(f) => *f as i32, + _ => unreachable!(), + }; + assert_eq!(i, id); + break; + } + StepResult::IO => { + tmp_db.io.run_once()?; + } + StepResult::Interrupt => break, + StepResult::Done => break, + StepResult::Busy => { + panic!("Database is busy"); + } + } + }, + Ok(None) => {} + Err(err) => { + eprintln!("{}", err); + } + } + } + } + + Ok(()) +}