diff --git a/core/storage/btree.rs b/core/storage/btree.rs index bc7c4afe2..7ecaf9463 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -339,6 +339,24 @@ enum OverflowState { Done, } +/// Holds a Record or RowId, so that these can be transformed into a SeekKey to restore +/// cursor position to its previous location. +enum CursorContext { + TableRowId(u64), + + /// If we are in an index tree we can then reuse this field to save + /// our cursor information + IndexKeyRowId(ImmutableRecord), +} + +/// In the future, we may expand these general validity states +enum CursorValidState { + /// Cursor is pointing a to an existing location/cell in the Btree + Valid, + /// Cursor may be pointing to a non-existent location/cell. This can happen after balancing operations + RequireSeek, +} + pub struct BTreeCursor { /// The multi-version cursor that is used to read and write to the database file. mv_cursor: Option>>, @@ -367,6 +385,10 @@ pub struct BTreeCursor { pub index_key_sort_order: IndexKeySortOrder, /// Maintain count of the number of records in the btree. Used for the `Count` opcode count: usize, + /// Stores the cursor context before rebalancing so that a seek can be done later + context: Option, + /// Store whether the Cursor is in a valid state. Meaning if it is pointing to a valid cell index or not + valid_state: CursorValidState, } impl BTreeCursor { @@ -393,6 +415,8 @@ impl BTreeCursor { empty_record: Cell::new(true), index_key_sort_order: IndexKeySortOrder::default(), count: 0, + context: None, + valid_state: CursorValidState::Valid, } } @@ -756,8 +780,9 @@ impl BTreeCursor { let mem_page = mem_page_rc.get(); let contents = mem_page.contents.as_ref().unwrap(); + let cell_count = contents.cell_count(); - if cell_idx == contents.cell_count() { + if cell_count == 0 || cell_idx == cell_count { // do rightmost let has_parent = self.stack.has_parent(); match contents.rightmost_pointer() { @@ -3444,6 +3469,7 @@ impl BTreeCursor { } pub fn next(&mut self) -> Result> { + let _ = self.restore_context()?; let rowid = return_if_io!(self.get_next_record(None)); self.rowid.replace(rowid); self.empty_record.replace(rowid.is_none()); @@ -3590,7 +3616,9 @@ impl BTreeCursor { DeleteState::FindCell => { let page = self.stack.top(); let mut cell_idx = self.stack.current_cell_index() as usize; - cell_idx -= 1; + if cell_idx > 0 { + cell_idx -= 1; + } let contents = page.get().contents.as_ref().unwrap(); if cell_idx >= contents.cell_count() { @@ -3748,13 +3776,14 @@ impl BTreeCursor { write_info.state = WriteState::BalanceStart; delete_info.balance_write_info = Some(write_info); } - delete_info.state = DeleteState::WaitForBalancingToComplete { target_key } } else { self.stack.retreat(); self.state = CursorState::None; return Ok(CursorResult::Ok(())); } + // Only reaches this function call if state = DeleteState::WaitForBalancingToComplete + self.save_context(); } DeleteState::WaitForBalancingToComplete { target_key } => { @@ -4337,6 +4366,50 @@ impl BTreeCursor { } } } + + /// Save cursor context, to be restored later + pub fn save_context(&mut self) { + if let Some(rowid) = self.rowid.get() { + self.valid_state = CursorValidState::RequireSeek; + match self.stack.top().get_contents().page_type() { + PageType::TableInterior | PageType::TableLeaf => { + self.context = Some(CursorContext::TableRowId(rowid)); + } + PageType::IndexInterior | PageType::IndexLeaf => { + self.context = Some(CursorContext::IndexKeyRowId( + self.reusable_immutable_record + .borrow() + .as_ref() + .unwrap() + .clone(), + )); + } + } + } + } + + /// If context is defined, restore it and set it None on success + fn restore_context(&mut self) -> Result> { + if self.context.is_none() || !matches!(self.valid_state, CursorValidState::RequireSeek) { + return Ok(CursorResult::Ok(())); + } + let ctx = self.context.take().unwrap(); + let seek_key = match ctx { + CursorContext::TableRowId(rowid) => SeekKey::TableRowId(rowid), + CursorContext::IndexKeyRowId(ref record) => SeekKey::IndexKey(record), + }; + let res = self.seek(seek_key, SeekOp::EQ)?; + match res { + CursorResult::Ok(_) => { + self.valid_state = CursorValidState::Valid; + Ok(CursorResult::Ok(())) + } + CursorResult::IO => { + self.context = Some(ctx); + Ok(CursorResult::IO) + } + } + } } #[cfg(debug_assertions)] @@ -4453,13 +4526,21 @@ impl PageStack { /// We usually advance after going traversing a new page fn advance(&self) { let current = self.current(); - tracing::trace!("pagestack::advance {}", self.cell_indices.borrow()[current],); + tracing::trace!( + "pagestack::advance {}, cell_indices={:?}", + self.cell_indices.borrow()[current], + self.cell_indices + ); self.cell_indices.borrow_mut()[current] += 1; } fn retreat(&self) { let current = self.current(); - tracing::trace!("pagestack::retreat {}", self.cell_indices.borrow()[current]); + tracing::trace!( + "pagestack::retreat {}, cell_indices={:?}", + self.cell_indices.borrow()[current], + self.cell_indices + ); self.cell_indices.borrow_mut()[current] -= 1; } @@ -4477,7 +4558,7 @@ impl PageStack { fn set_cell_index(&self, idx: i32) { let current = self.current(); - self.cell_indices.borrow_mut()[current] = idx + self.cell_indices.borrow_mut()[current] = idx; } fn has_parent(&self) -> bool { diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 6557ca169..7c48f92ad 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -6,12 +6,13 @@ use std::sync::Arc; use limbo_sqlite3_parser::ast::{self}; +use crate::error::SQLITE_CONSTRAINT_PRIMARYKEY; 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::{CursorType, ProgramBuilder}; -use crate::vdbe::insn::{IdxInsertFlags, RegisterOrLiteral}; +use crate::vdbe::insn::{CmpInsFlags, IdxInsertFlags, RegisterOrLiteral}; use crate::vdbe::{insn::Insn, BranchOffset}; use crate::{Result, SymbolTable}; @@ -485,6 +486,7 @@ fn emit_delete_insns( program.emit_insn(Insn::OpenWrite { cursor_id: index_cursor_id, root_page: RegisterOrLiteral::Literal(index.root_page), + name: index.name.clone(), }); let num_regs = index.columns.len() + 1; let start_reg = program.alloc_registers(num_regs); @@ -591,6 +593,7 @@ fn emit_program_for_update( // Open indexes for update. let mut index_cursors = Vec::with_capacity(plan.indexes_to_update.len()); // TODO: do not reopen if there is table reference using it. + for index in &plan.indexes_to_update { let index_cursor = program.alloc_cursor_id( Some(index.table_name.clone()), @@ -599,8 +602,10 @@ fn emit_program_for_update( program.emit_insn(Insn::OpenWrite { cursor_id: index_cursor, root_page: RegisterOrLiteral::Literal(index.root_page), + name: index.name.clone(), }); - index_cursors.push(index_cursor); + let record_reg = program.alloc_register(); + index_cursors.push((index_cursor, record_reg)); } open_loop( program, @@ -641,7 +646,7 @@ fn emit_update_insns( plan: &UpdatePlan, t_ctx: &TranslateCtx, program: &mut ProgramBuilder, - index_cursors: Vec, + index_cursors: Vec<(usize, usize)>, ) -> crate::Result<()> { let table_ref = &plan.table_references.first().unwrap(); let loop_labels = t_ctx.labels_main_loop.first().unwrap(); @@ -700,11 +705,44 @@ fn emit_update_insns( cursor_id, dest: beg, }); - // if no rowid, we're done - program.emit_insn(Insn::IsNull { - reg: beg, - target_pc: t_ctx.label_main_loop_end.unwrap(), - }); + + // Check if rowid was provided (through INTEGER PRIMARY KEY as a rowid alias) + + let rowid_alias_index = { + let rowid_alias_index = table_ref.columns().iter().position(|c| c.is_rowid_alias); + if let Some(index) = rowid_alias_index { + plan.set_clauses.iter().position(|(idx, _)| *idx == index) + } else { + None + } + }; + let rowid_set_clause_reg = if rowid_alias_index.is_some() { + Some(program.alloc_register()) + } else { + None + }; + let has_user_provided_rowid = rowid_alias_index.is_some(); + + let check_rowid_not_exists_label = if has_user_provided_rowid { + Some(program.allocate_label()) + } else { + None + }; + + if has_user_provided_rowid { + program.emit_insn(Insn::NotExists { + cursor: cursor_id, + rowid_reg: beg, + target_pc: check_rowid_not_exists_label.unwrap(), + }); + } else { + // if no rowid, we're done + program.emit_insn(Insn::IsNull { + reg: beg, + target_pc: t_ctx.label_main_loop_end.unwrap(), + }); + } + if is_virtual { program.emit_insn(Insn::Copy { src_reg: beg, @@ -740,61 +778,40 @@ fn emit_update_insns( )?; } - // Update indexes first. Columns that are updated will be translated from an expression and those who aren't modified will be - // read from table. Mutiple value index key could be updated partially. - for (index, index_cursor) in plan.indexes_to_update.iter().zip(index_cursors) { - let index_record_reg_count = index.columns.len() + 1; - let index_record_reg_start = program.alloc_registers(index_record_reg_count); - for (idx, column) in index.columns.iter().enumerate() { - if let Some((_, expr)) = plan.set_clauses.iter().find(|(i, _)| *i == idx) { + // we scan a column at a time, loading either the column's values, or the new value + // from the Set expression, into registers so we can emit a MakeRecord and update the row. + let start = if is_virtual { beg + 2 } else { beg + 1 }; + for (idx, table_column) in table_ref.columns().iter().enumerate() { + let target_reg = start + idx; + if let Some((_, expr)) = plan.set_clauses.iter().find(|(i, _)| *i == idx) { + if has_user_provided_rowid + && (table_column.primary_key || table_column.is_rowid_alias) + && !is_virtual + { + let rowid_set_clause_reg = rowid_set_clause_reg.unwrap(); translate_expr( program, Some(&plan.table_references), expr, - index_record_reg_start + idx, + rowid_set_clause_reg, &t_ctx.resolver, )?; - } else { - program.emit_insn(Insn::Column { - cursor_id: cursor_id, - column: column.pos_in_table, - dest: index_record_reg_start + idx, + + program.emit_insn(Insn::MustBeInt { + reg: rowid_set_clause_reg, }); + + program.emit_null(target_reg, None); + } else { + translate_expr( + program, + Some(&plan.table_references), + expr, + target_reg, + &t_ctx.resolver, + )?; } - } - program.emit_insn(Insn::RowId { - cursor_id: cursor_id, - dest: index_record_reg_start + index.columns.len(), - }); - let index_record_reg = program.alloc_register(); - program.emit_insn(Insn::MakeRecord { - start_reg: index_record_reg_start, - count: index_record_reg_count, - dest_reg: index_record_reg, - }); - program.emit_insn(Insn::IdxInsert { - cursor_id: index_cursor, - record_reg: index_record_reg, - unpacked_start: Some(index_record_reg_start), - unpacked_count: Some(index_record_reg_count as u16), - flags: IdxInsertFlags::new(), - }); - } - // we scan a column at a time, loading either the column's values, or the new value - // from the Set expression, into registers so we can emit a MakeRecord and update the row. - let start = if is_virtual { beg + 2 } else { beg + 1 }; - for idx in 0..table_ref.columns().len() { - let target_reg = start + idx; - if let Some((_, expr)) = plan.set_clauses.iter().find(|(i, _)| *i == idx) { - translate_expr( - program, - Some(&plan.table_references), - expr, - target_reg, - &t_ctx.resolver, - )?; } else { - let table_column = table_ref.table.columns().get(idx).unwrap(); let column_idx_in_index = index.as_ref().and_then(|(idx, _)| { idx.columns .iter() @@ -829,6 +846,86 @@ fn emit_update_insns( } } } + + for (index, (idx_cursor_id, record_reg)) in plan.indexes_to_update.iter().zip(&index_cursors) { + if !index.unique { + continue; + } + + let num_cols = index.columns.len(); + // allocate scratch registers for the index columns plus rowid + let idx_start_reg = program.alloc_registers(num_cols + 1); + + let rowid_reg = beg; + let idx_cols_start_reg = beg + 1; + + // copy each index column from the table's column registers into these scratch regs + for (i, col) in index.columns.iter().enumerate() { + // copy from the table's column register over to the index's scratch register + + program.emit_insn(Insn::Copy { + src_reg: idx_cols_start_reg + col.pos_in_table, + dst_reg: idx_start_reg + i, + amount: 0, + }); + } + // last register is the rowid + program.emit_insn(Insn::Copy { + src_reg: rowid_reg, + dst_reg: idx_start_reg + num_cols, + amount: 0, + }); + + program.emit_insn(Insn::MakeRecord { + start_reg: idx_start_reg, + count: num_cols + 1, + dest_reg: *record_reg, + index_name: Some(index.name.clone()), + }); + + let constraint_check = program.allocate_label(); + program.emit_insn(Insn::NoConflict { + cursor_id: *idx_cursor_id, + target_pc: constraint_check, + record_reg: idx_start_reg, + num_regs: num_cols, + }); + + let column_names = index.columns.iter().enumerate().fold( + String::with_capacity(50), + |mut accum, (idx, col)| { + if idx > 0 { + accum.push_str(", "); + } + accum.push_str(&table_ref.table.get_name()); + accum.push('.'); + accum.push_str(&col.name); + + accum + }, + ); + + let idx_rowid_reg = program.alloc_register(); + program.emit_insn(Insn::IdxRowId { + cursor_id: *idx_cursor_id, + dest: idx_rowid_reg, + }); + + program.emit_insn(Insn::Eq { + lhs: rowid_reg, + rhs: idx_rowid_reg, + target_pc: constraint_check, + flags: CmpInsFlags::default(), // TODO: not sure what type of comparison flag is needed + }); + + program.emit_insn(Insn::Halt { + err_code: SQLITE_CONSTRAINT_PRIMARYKEY, // TODO: distinct between primary key and unique index for error code + description: column_names, + }); + + program.preassign_label_to_next_insn(constraint_check); + } + if let Some(btree_table) = table_ref.btree() { if btree_table.is_strict { program.emit_insn(Insn::TypeCheck { @@ -838,15 +935,102 @@ fn emit_update_insns( table_reference: Rc::clone(&btree_table), }); } + + if has_user_provided_rowid { + let record_label = program.allocate_label(); + let idx = rowid_alias_index.unwrap(); + let target_reg = rowid_set_clause_reg.unwrap(); + program.emit_insn(Insn::Eq { + lhs: target_reg, + rhs: beg, + target_pc: record_label, + flags: CmpInsFlags::default(), + }); + + program.emit_insn(Insn::NotExists { + cursor: cursor_id, + rowid_reg: target_reg, + target_pc: record_label, + }); + + program.emit_insn(Insn::Halt { + err_code: SQLITE_CONSTRAINT_PRIMARYKEY, + description: format!( + "{}.{}", + table_ref.table.get_name(), + &table_ref + .columns() + .get(idx) + .unwrap() + .name + .as_ref() + .map_or("", |v| v) + ), + }); + + program.preassign_label_to_next_insn(record_label); + } + let record_reg = program.alloc_register(); program.emit_insn(Insn::MakeRecord { start_reg: start, count: table_ref.columns().len(), dest_reg: record_reg, + index_name: None, }); + + if has_user_provided_rowid { + program.emit_insn(Insn::NotExists { + cursor: cursor_id, + rowid_reg: beg, + target_pc: check_rowid_not_exists_label.unwrap(), + }); + } + + // For each index -> insert + for (index, (idx_cursor_id, record_reg)) in plan.indexes_to_update.iter().zip(index_cursors) + { + 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, + column: column_index.pos_in_table, + dest: start_reg + reg_offset, + }); + }); + + program.emit_insn(Insn::RowId { + cursor_id, + dest: start_reg + num_regs - 1, + }); + + program.emit_insn(Insn::IdxDelete { + start_reg, + num_regs, + cursor_id: idx_cursor_id, + }); + + program.emit_insn(Insn::IdxInsert { + cursor_id: idx_cursor_id, + record_reg: record_reg, + unpacked_start: Some(start), + unpacked_count: Some((index.columns.len() + 1) as u16), + flags: IdxInsertFlags::new(), + }); + } + + program.emit_insn(Insn::Delete { cursor_id }); + program.emit_insn(Insn::Insert { cursor: cursor_id, - key_reg: beg, + key_reg: rowid_set_clause_reg.unwrap_or(beg), record_reg, flag: 0, table_name: table_ref.identifier.clone(), @@ -869,5 +1053,10 @@ fn emit_update_insns( }) } // TODO(pthorpe): handle RETURNING clause + + if let Some(label) = check_rowid_not_exists_label { + program.preassign_label_to_next_insn(label); + } + Ok(()) } diff --git a/core/translate/index.rs b/core/translate/index.rs index 4ab8486dd..30e96f897 100644 --- a/core/translate/index.rs +++ b/core/translate/index.rs @@ -100,6 +100,7 @@ pub fn translate_create_index( program.emit_insn(Insn::OpenWrite { cursor_id: sqlite_schema_cursor_id, root_page: RegisterOrLiteral::Literal(sqlite_table.root_page), + name: sqlite_table.name.clone(), }); let sql = create_idx_stmt_to_sql(&tbl_name, &idx_name, unique_if_not_exists, &columns); emit_schema_entry( @@ -164,6 +165,7 @@ pub fn translate_create_index( start_reg, count: columns.len() + 1, dest_reg: record_reg, + index_name: Some(idx_name.clone()), }); program.emit_insn(Insn::SorterInsert { cursor_id: sorter_cursor_id, @@ -181,6 +183,7 @@ pub fn translate_create_index( program.emit_insn(Insn::OpenWrite { cursor_id: btree_cursor_id, root_page: RegisterOrLiteral::Register(root_page_reg), + name: idx_name.clone(), }); let sorted_loop_start = program.allocate_label(); @@ -375,6 +378,7 @@ pub fn translate_drop_index( program.emit_insn(Insn::OpenWrite { cursor_id: sqlite_schema_cursor_id, root_page: RegisterOrLiteral::Literal(sqlite_table.root_page), + name: sqlite_table.name.clone(), }); let loop_start_label = program.allocate_label(); diff --git a/core/translate/insert.rs b/core/translate/insert.rs index d162f2c2e..1c3169b4c 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -177,6 +177,7 @@ pub fn translate_insert( program.emit_insn(Insn::OpenWrite { cursor_id, root_page: RegisterOrLiteral::Literal(root_page), + name: table_name.0.clone(), }); // Main loop @@ -192,6 +193,7 @@ pub fn translate_insert( program.emit_insn(Insn::OpenWrite { cursor_id, root_page: RegisterOrLiteral::Literal(root_page), + name: table_name.0.clone(), }); populate_column_registers( @@ -209,6 +211,7 @@ pub fn translate_insert( program.emit_insn(Insn::OpenWrite { cursor_id: idx_cursor.2, root_page: idx_cursor.1.into(), + name: idx_cursor.0.clone(), }); } // Common record insertion logic for both single and multiple rows @@ -289,7 +292,7 @@ pub fn translate_insert( _ => (), } - for index_col_mapping in index_col_mappings.iter() { + for index_col_mapping in index_col_mappings { // find which cursor we opened earlier for this index let idx_cursor_id = idx_cursors .iter() @@ -318,17 +321,18 @@ pub fn translate_insert( amount: 0, }); + let index = schema + .get_index(&table_name.0, &index_col_mapping.idx_name) + .expect("index should be present"); + let record_reg = program.alloc_register(); program.emit_insn(Insn::MakeRecord { start_reg: idx_start_reg, count: num_cols + 1, dest_reg: record_reg, + index_name: Some(index_col_mapping.idx_name), }); - let index = schema - .get_index(&table_name.0, &index_col_mapping.idx_name) - .expect("index should be present"); - if index.unique { let label_idx_insert = program.allocate_label(); program.emit_insn(Insn::NoConflict { @@ -384,6 +388,7 @@ pub fn translate_insert( start_reg: column_registers_start, count: num_cols, dest_reg: record_register, + index_name: None, }); program.emit_insn(Insn::Insert { diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index bf30b8508..f10523f16 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -110,6 +110,7 @@ pub fn init_loop( cursor_id: table_cursor_id .expect("table cursor is always opened in OperationMode::DELETE"), root_page: root_page.into(), + name: btree.name.clone(), }); } (OperationMode::UPDATE, Table::BTree(btree)) => { @@ -118,11 +119,13 @@ pub fn init_loop( cursor_id: table_cursor_id .expect("table cursor is always opened in OperationMode::UPDATE"), root_page: root_page.into(), + name: btree.name.clone(), }); if let Some(index_cursor_id) = index_cursor_id { program.emit_insn(Insn::OpenWrite { cursor_id: index_cursor_id, root_page: index.as_ref().unwrap().root_page.into(), + name: index.as_ref().unwrap().name.clone(), }); } } @@ -148,6 +151,7 @@ pub fn init_loop( program.emit_insn(Insn::OpenWrite { cursor_id: table_cursor_id, root_page: table.table.get_root_page().into(), + name: table.table.get_name().to_string(), }); } _ => { @@ -174,6 +178,7 @@ pub fn init_loop( cursor_id: index_cursor_id .expect("index cursor is always opened in Seek with index"), root_page: index.root_page.into(), + name: index.name.clone(), }); } _ => { @@ -1222,6 +1227,7 @@ fn emit_autoindex( start_reg: ephemeral_cols_start_reg, count: num_regs_to_reserve, dest_reg: record_reg, + index_name: Some(index.name.clone()), }); program.emit_insn(Insn::IdxInsert { cursor_id: index_cursor_id, diff --git a/core/translate/order_by.rs b/core/translate/order_by.rs index 6c8f4d90a..488a8cd82 100644 --- a/core/translate/order_by.rs +++ b/core/translate/order_by.rs @@ -247,6 +247,7 @@ pub fn sorter_insert( start_reg, count: column_count, dest_reg: record_reg, + index_name: None, }); program.emit_insn(Insn::SorterInsert { cursor_id, diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 8fcb8abb8..7c0b3d7cc 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -120,6 +120,7 @@ pub fn translate_create_table( program.emit_insn(Insn::OpenWrite { cursor_id: sqlite_schema_cursor_id, root_page: 1usize.into(), + name: tbl_name.name.0.clone(), }); // Add the table entry to sqlite_schema @@ -236,6 +237,7 @@ pub fn emit_schema_entry( start_reg: type_reg, count: 5, dest_reg: record_reg, + index_name: None, }); program.emit_insn(Insn::Insert { @@ -563,6 +565,7 @@ pub fn translate_create_virtual_table( start_reg: args_start, count: args_vec.len(), dest_reg: args_record_reg, + index_name: None, }); Some(args_record_reg) } else { @@ -582,6 +585,7 @@ pub fn translate_create_virtual_table( program.emit_insn(Insn::OpenWrite { cursor_id: sqlite_schema_cursor_id, root_page: 1usize.into(), + name: table_name.clone(), }); let sql = create_vtable_body_to_str(&vtab, vtab_module.clone()); @@ -661,6 +665,7 @@ pub fn translate_drop_table( program.emit_insn(Insn::OpenWrite { cursor_id: sqlite_schema_cursor_id, root_page: 1usize.into(), + name: tbl_name.name.0.clone(), }); // 1. Remove all entries from the schema table related to the table we are dropping, except for triggers diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index d6ab6ba4d..62e953813 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1444,6 +1444,7 @@ pub fn op_make_record( start_reg, count, dest_reg, + .. } = insn else { unreachable!("unexpected Insn {:?}", insn) diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index ff5892ddb..7ac6e397d 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -529,20 +529,25 @@ pub fn insn_to_str( start_reg, count, dest_reg, - } => ( - "MakeRecord", - *start_reg as i32, - *count as i32, - *dest_reg as i32, - OwnedValue::build_text(""), - 0, - format!( - "r[{}]=mkrec(r[{}..{}])", - dest_reg, - start_reg, - start_reg + count - 1, - ), - ), + index_name, + } => { + let for_index = index_name.as_ref().map(|name| format!("; for {}", name)); + ( + "MakeRecord", + *start_reg as i32, + *count as i32, + *dest_reg as i32, + OwnedValue::build_text(""), + 0, + format!( + "r[{}]=mkrec(r[{}..{}]){}", + dest_reg, + start_reg, + start_reg + count - 1, + for_index.unwrap_or("".to_string()) + ), + ) + } Insn::ResultRow { start_reg, count } => ( "ResultRow", *start_reg as i32, @@ -1081,15 +1086,22 @@ pub fn insn_to_str( target_pc, record_reg, num_regs, - } => ( - "NoConflict", - *cursor_id as i32, - target_pc.to_debug_int(), - *record_reg as i32, - OwnedValue::build_text(&format!("{num_regs}")), - 0, - format!("key=r[{}]", record_reg), - ), + } => { + let key = if *num_regs > 0 { + format!("key=r[{}..{}]", record_reg, record_reg + num_regs - 1) + } else { + format!("key=r[{}]", record_reg) + }; + ( + "NoConflict", + *cursor_id as i32, + target_pc.to_debug_int(), + *record_reg as i32, + OwnedValue::build_text(&format!("{num_regs}")), + 0, + key, + ) + } Insn::NotExists { cursor, rowid_reg, @@ -1122,6 +1134,7 @@ pub fn insn_to_str( Insn::OpenWrite { cursor_id, root_page, + name, .. } => ( "OpenWrite", @@ -1133,7 +1146,7 @@ pub fn insn_to_str( 0, OwnedValue::build_text(""), 0, - "".to_string(), + format!("root={}; {}", root_page, name), ), Insn::Copy { src_reg, diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 587695d30..876ab7930 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -82,7 +82,7 @@ impl IdxInsertFlags { } #[derive(Clone, Copy, Debug)] -pub enum RegisterOrLiteral { +pub enum RegisterOrLiteral { Register(usize), Literal(T), } @@ -93,6 +93,15 @@ impl From for RegisterOrLiteral { } } +impl std::fmt::Display for RegisterOrLiteral { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Literal(lit) => lit.fmt(f), + Self::Register(reg) => reg.fmt(f), + } + } +} + #[derive(Description, Debug)] pub enum Insn { /// Initialize the program state and jump to the given PC. @@ -364,6 +373,7 @@ pub enum Insn { start_reg: usize, // P1 count: usize, // P2 dest_reg: usize, // P3 + index_name: Option, }, /// Emit a row of results. @@ -699,6 +709,7 @@ pub enum Insn { OpenWrite { cursor_id: CursorID, root_page: RegisterOrLiteral, + name: String, }, Copy { diff --git a/flake.nix b/flake.nix index 31f19412d..e262c09c4 100644 --- a/flake.nix +++ b/flake.nix @@ -24,10 +24,17 @@ lib = pkgs.lib; + # Custom SQLite package with debug enabled + sqlite-debug = pkgs.sqlite.overrideAttrs (oldAttrs: rec { + name = "sqlite-debug-${oldAttrs.version}"; + configureFlags = oldAttrs.configureFlags ++ [ "--enable-debug" ]; + dontStrip = true; + separateDebugInfo = true; + }); + cargoArtifacts = craneLib.buildDepsOnly { src = ./.; pname = "limbo"; - stritcDeps = true; nativeBuildInputs = with pkgs; [ python3 ]; }; @@ -58,7 +65,7 @@ devShells.default = with pkgs; mkShell { nativeBuildInputs = [ clang - sqlite + sqlite-debug # Use debug-enabled SQLite gnumake tcl python3 @@ -77,4 +84,4 @@ }; } ); -} +} \ No newline at end of file diff --git a/testing/cli_tests/constraint.py b/testing/cli_tests/constraint.py index 65758745b..36181fa67 100644 --- a/testing/cli_tests/constraint.py +++ b/testing/cli_tests/constraint.py @@ -230,12 +230,24 @@ class Table(BaseModel): return f"INSERT INTO {self.name} VALUES ({vals});" + # These statements should always cause a constraint error as there is no where clause here + def generate_update(self) -> str: + vals = [ + f"{col.name} = {col.col_type.generate(fake)}" + for col in self.columns + if col.primary_key + ] + vals = ", ".join(vals) + + return f"UPDATE {self.name} SET {vals};" + class ConstraintTest(BaseModel): table: Table db_path: str = "testing/constraint.db" insert_stmts: list[str] insert_errors: list[str] + update_errors: list[str] def run( self, @@ -258,6 +270,14 @@ class ConstraintTest(BaseModel): str(len(self.insert_stmts)), ) + for update_stmt in self.update_errors: + limbo.run_test_fn( + update_stmt, + lambda val: "Runtime error: UNIQUE constraint failed" in val, + ) + + # TODO: When we implement rollbacks, have a test here to assure the values did not change + def validate_with_expected(result: str, expected: str): return (expected in result, expected) @@ -281,8 +301,17 @@ def generate_test(col_amount: int, primary_keys: int) -> ConstraintTest: table = Table(columns=cols, name=fake.word()) insert_stmts = [table.generate_insert() for _ in range(col_amount)] + + update_errors = [] + if len(insert_stmts) > 1: + # TODO: As we have no rollback we just generate one update statement + update_errors = [table.generate_update()] + return ConstraintTest( - table=table, insert_stmts=insert_stmts, insert_errors=insert_stmts + table=table, + insert_stmts=insert_stmts, + insert_errors=insert_stmts, + update_errors=update_errors, ) @@ -296,8 +325,14 @@ def custom_test_1() -> ConstraintTest: "INSERT INTO users VALUES (1, 'alice');", "INSERT INTO users VALUES (2, 'bob');", ] + update_stmts = [ + "UPDATE users SET id = 2, username = 'bob' WHERE id == 1;", + ] return ConstraintTest( - table=table, insert_stmts=insert_stmts, insert_errors=insert_stmts + table=table, + insert_stmts=insert_stmts, + insert_errors=insert_stmts, + update_errors=update_stmts, )