From 2495d15b96495562213ae14e8d09cfe0b7b77443 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Wed, 16 Apr 2025 13:11:49 +0300 Subject: [PATCH 01/16] Index insert fuzz --- core/storage/btree.rs | 86 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index a8167fd17..0b8226017 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -5159,8 +5159,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, @@ -5678,6 +5680,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(); @@ -5731,6 +5808,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); From b7970a286d77808bd01ce1c4e40ab4d103b82990 Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Tue, 22 Apr 2025 11:50:51 +0200 Subject: [PATCH 02/16] implement IdxDelete clippy revert op_idx_ge changes fmt fmt again rever op_idx_gt changes --- core/translate/delete.rs | 6 + core/translate/emitter.rs | 53 +++++++- core/translate/plan.rs | 2 + core/vdbe/execute.rs | 41 +++++- core/vdbe/explain.rs | 13 ++ core/vdbe/insn.rs | 7 + .../query_processing/test_write_path.rs | 125 ++++++++++++++++++ 7 files changed, 239 insertions(+), 8 deletions(-) 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 5b12e4375..d1e2fdce9 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}; @@ -375,7 +378,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)?; @@ -393,6 +402,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(); @@ -408,11 +418,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, }); @@ -433,7 +444,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/plan.rs b/core/translate/plan.rs index 48ce4c854..d25e1837c 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/vdbe/execute.rs b/core/vdbe/execute.rs index de871f54c..1177af3b7 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1810,11 +1810,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, + _ => todo!(), + } }; 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 +2075,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() @@ -3759,6 +3764,34 @@ pub fn op_delete( Ok(InsnFunctionStepResult::Step) } +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) + }; + let record = make_record(&state.registers, start_reg, num_regs); + { + let mut cursor = state.get_cursor(*cursor_id); + let cursor = cursor.as_btree_mut(); + return_if_io!(cursor.seek(SeekKey::IndexKey(&record), SeekOp::EQ)); + return_if_io!(cursor.delete()); + } + let prev_changes = program.n_change.get(); + program.n_change.set(prev_changes + 1); + state.pc += 1; + Ok(InsnFunctionStepResult::Step) +} + pub fn op_idx_insert( program: &Program, state: &mut ProgramState, @@ -3766,7 +3799,6 @@ pub fn op_idx_insert( pager: &Rc, mv_store: Option<&Rc>, ) -> Result { - dbg!("op_idx_insert_"); if let Insn::IdxInsert { cursor_id, record_reg, @@ -3807,7 +3839,6 @@ pub fn op_idx_insert( } }; - dbg!(moved_before); // Start insertion of row. This might trigger a balance procedure which will take care of moving to different pages, // therefore, we don't want to seek again if that happens, meaning we don't want to return on io without moving to the following opcode // because it could trigger a movement to child page after a balance root which will leave the current page as the root page. diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 96afc5d17..a6db7be05 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 56f44bd2b..295b41a2d 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 @@ -948,6 +954,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/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(()) +} From 3ba5c2349fb1f48a047090f05fa57273d2656368 Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Thu, 24 Apr 2025 16:23:20 +0200 Subject: [PATCH 03/16] add corrupt error if no matching record found for idxdelete a --- core/vdbe/execute.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 1177af3b7..bc91c9841 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1815,7 +1815,7 @@ pub fn op_row_id( let rowid = record.get_values().last().unwrap(); match rowid { RefValue::Integer(rowid) => *rowid as u64, - _ => todo!(), + _ => unreachable!(), } }; let mut table_cursor = state.get_cursor(table_cursor_id); @@ -3784,10 +3784,22 @@ pub fn op_idx_delete( let mut cursor = state.get_cursor(*cursor_id); let cursor = cursor.as_btree_mut(); return_if_io!(cursor.seek(SeekKey::IndexKey(&record), SeekOp::EQ)); + + 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 + ))); + } return_if_io!(cursor.delete()); } - let prev_changes = program.n_change.get(); - program.n_change.set(prev_changes + 1); + let n_change = program.n_change.get(); + program.n_change.set(n_change + 1); state.pc += 1; Ok(InsnFunctionStepResult::Step) } From a7537be2b6dd9e283d93f544ba3874ec0c70508a Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Sat, 26 Apr 2025 09:38:03 +0300 Subject: [PATCH 04/16] antithesis-tests: Fix accounts to be at least one --- antithesis-tests/bank-test/first_setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From e8bc3086f2fa7d8654ae399c848485a91006a0a4 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Sat, 26 Apr 2025 10:53:31 +0300 Subject: [PATCH 05/16] antithesis-tests: Fix generate_random_value() on older Python versions --- antithesis-tests/stress-composer/utils.py | 27 +++++++++++------------ 1 file changed, 13 insertions(+), 14 deletions(-) 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 From 23ae5a27c473b142e45cea1966d72af18f35f2a8 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Sat, 26 Apr 2025 11:32:24 +0300 Subject: [PATCH 06/16] btree/tablebtree_move_to: micro-optimizations --- core/storage/btree.rs | 29 +++++++---------------------- core/storage/sqlite3_ondisk.rs | 17 +++++++++++++++++ core/types.rs | 1 + 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index d191227a4..02c60363d 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1043,18 +1043,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,15 +1054,11 @@ 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; } @@ -1089,7 +1075,6 @@ impl BTreeCursor { } } let cur_cell_idx = (min + max) / 2; - self.stack.set_cell_index(cur_cell_idx as i32); 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, diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index 8e091ef64..315d01460 100644 --- a/core/storage/sqlite3_ondisk.rs +++ b/core/storage/sqlite3_ondisk.rs @@ -617,6 +617,23 @@ 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 { + 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 { diff --git a/core/types.rs b/core/types.rs index b2a7a053b..5d86f97f3 100644 --- a/core/types.rs +++ b/core/types.rs @@ -1402,6 +1402,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, From 6d3c63fb012605f2eddfcbb752bad8b224589c2c Mon Sep 17 00:00:00 2001 From: Anton Harniakou Date: Sat, 26 Apr 2025 12:04:37 +0300 Subject: [PATCH 07/16] Add the .indexes command --- cli/app.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++ cli/commands/args.rs | 6 +++++ cli/commands/mod.rs | 7 ++++-- cli/input.rs | 2 ++ 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/cli/app.rs b/cli/app.rs index 8212ce8c7..196face0a 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -634,6 +634,11 @@ impl<'a> Limbo<'a> { 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, @@ -909,6 +914,55 @@ impl<'a> Limbo<'a> { 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!( 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 (;). From 5060f1a1fa937aa712accfd44d9aa8f463373129 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Sat, 26 Apr 2025 12:12:09 +0300 Subject: [PATCH 08/16] don't use integer division in binary search halfpoint calculation --- core/storage/btree.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 02c60363d..d42d5f95a 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1074,7 +1074,7 @@ impl BTreeCursor { } } } - 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. 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, @@ -1186,7 +1186,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, @@ -1340,7 +1340,7 @@ impl BTreeCursor { return Ok(CursorResult::Ok(Some(cell_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_rowid = contents.cell_table_leaf_read_rowid(cur_cell_idx as usize)?; @@ -1509,7 +1509,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( From e46c01928c25f4c61c869d2da9b1bd0ff8759de5 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Sat, 26 Apr 2025 12:59:19 +0300 Subject: [PATCH 09/16] antithesis: Enable Rust backtraces again --- Dockerfile.antithesis | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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"] From ac1bc17ea45f918e0cc37683cc11da85cc0fce1b Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Sat, 26 Apr 2025 13:41:30 +0300 Subject: [PATCH 10/16] btree/tablebtree_seek: remove some more useless calls to set_cell_index() --- core/storage/btree.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index d42d5f95a..46c6ef9f2 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1306,7 +1306,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( @@ -1335,13 +1334,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) >> 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_rowid = contents.cell_table_leaf_read_rowid(cur_cell_idx as usize)?; let cmp = cell_rowid.cmp(&rowid); @@ -1383,7 +1385,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))); } From 75c6678a06a57029c0de80521ad35d8c03094cd1 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Sat, 26 Apr 2025 14:47:45 +0300 Subject: [PATCH 11/16] sqlite3_ondisk: use debug asserts for cell_table_interior_read... funcs --- core/storage/sqlite3_ondisk.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index 315d01460..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; @@ -620,7 +620,7 @@ impl PageContent { /// 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 { - 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; @@ -637,7 +637,7 @@ impl PageContent { /// 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; From 46d45a6bf49f1d31bc9c244569b5f9583ca11fff Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Sat, 26 Apr 2025 14:47:56 +0300 Subject: [PATCH 12/16] don't recompute cell_count --- core/storage/btree.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 46c6ef9f2..bc3e450c8 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1062,7 +1062,7 @@ impl BTreeCursor { 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)?; From 33d230771f52bf21502c1a51dda849ce5a2fc10e Mon Sep 17 00:00:00 2001 From: Piotr Rzysko Date: Mon, 28 Apr 2025 08:35:04 +0200 Subject: [PATCH 13/16] Save history on exit --- cli/app.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cli/app.rs b/cli/app.rs index d214d139f..82fa34b63 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) => { @@ -1008,12 +1010,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() + } +} From 3e70cc3b68479be0034ce1051c31c13c8abefc80 Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Mon, 28 Apr 2025 11:33:46 +0300 Subject: [PATCH 14/16] fix: old name --- fuzz/fuzz_targets/cast_real.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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); From a30241ca9150cf37e64937cc789c78355aebeee5 Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Mon, 28 Apr 2025 12:45:23 +0300 Subject: [PATCH 15/16] Add state machine for op_idx_delete + DeleteState simplification DeleteState had a bit too many unnecessary states so I removed them. Usually we care about having a different state when I/O is triggered requiring a state to be stored for later. Furthermore, there was a bug with op_idx_delete where if balance is triggered, op_idx_delete wouldn't be re-entrant. So a state machine was added to prevent that from happening. --- core/storage/btree.rs | 94 ++++++++++++------------------------------- core/types.rs | 20 +++++++++ core/vdbe/execute.rs | 73 +++++++++++++++++++++++---------- core/vdbe/mod.rs | 4 +- 4 files changed, 100 insertions(+), 91 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 36b39eb15..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)] @@ -3530,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()); @@ -3554,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 @@ -3646,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; } } @@ -3724,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); @@ -3764,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(); @@ -3806,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(), @@ -3830,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(())); } diff --git a/core/types.rs b/core/types.rs index 5d86f97f3..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(); diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index f76bc5add..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, @@ -3756,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(); @@ -3764,6 +3770,10 @@ 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, @@ -3779,29 +3789,50 @@ pub fn op_idx_delete( else { unreachable!("unexpected Insn {:?}", insn) }; - let record = make_record(&state.registers, start_reg, num_regs); - { - let mut cursor = state.get_cursor(*cursor_id); - let cursor = cursor.as_btree_mut(); - return_if_io!(cursor.seek(SeekKey::IndexKey(&record), SeekOp::EQ)); - - 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 - ))); + 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)); + } } - return_if_io!(cursor.delete()); } - let n_change = program.n_change.get(); - program.n_change.set(n_change + 1); - state.pc += 1; - Ok(InsnFunctionStepResult::Step) } pub fn op_idx_insert( 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, } } From 51d43074f3fa474fcfb6b0d91965fe398b5d7c30 Mon Sep 17 00:00:00 2001 From: meteorgan Date: Tue, 29 Apr 2025 22:34:20 +0800 Subject: [PATCH 16/16] Support literal-value current_time, current_date and current_timestamp --- core/translate/expr.rs | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) 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!(),