From 72cc0fcdcb8f03fc418839661d30b4618fedcfc0 Mon Sep 17 00:00:00 2001 From: pedrocarlo Date: Mon, 12 May 2025 11:29:24 -0300 Subject: [PATCH] fixes and comments --- core/storage/btree.rs | 18 +++-- core/translate/emitter.rs | 136 +++++++++++++++++++------------------- 2 files changed, 80 insertions(+), 74 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index f149fd4c7..11388f457 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -339,6 +339,8 @@ 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), @@ -347,9 +349,11 @@ enum CursorContext { IndexKeyRowId(ImmutableRecord), } -// In the future, we may expand these general validity states +/// 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, } @@ -381,9 +385,9 @@ 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 last record that was seen. + /// 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 + /// Store whether the Cursor is in a valid state. Meaning if it is pointing to a valid cell index or not valid_state: CursorValidState, } @@ -4385,12 +4389,12 @@ impl BTreeCursor { } /// If context is defined, restore it and set it None on success - fn restore_context(&mut self) -> Result> { + fn restore_context(&mut self) -> Result> { if self.context.is_none() || !matches!(self.valid_state, CursorValidState::RequireSeek) { - return Ok(CursorResult::Ok(false)); + return Ok(CursorResult::Ok(())); } let ctx = self.context.as_ref().unwrap(); - let res = match ctx { + let _ = match ctx { CursorContext::TableRowId(rowid) => self.seek(SeekKey::TableRowId(*rowid), SeekOp::EQ), CursorContext::IndexKeyRowId(record) => { // TODO: see how to avoid clone here @@ -4400,7 +4404,7 @@ impl BTreeCursor { }?; self.context = None; self.valid_state = CursorValidState::Valid; - Ok(res) + Ok(CursorResult::Ok(())) } } diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 1196394e0..7c48f92ad 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -848,80 +848,82 @@ fn emit_update_insns( } for (index, (idx_cursor_id, record_reg)) in plan.indexes_to_update.iter().zip(&index_cursors) { - if index.unique { - 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); + if !index.unique { + continue; + } - let rowid_reg = beg; - let idx_cols_start_reg = beg + 1; + 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); - // 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 + 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, + src_reg: idx_cols_start_reg + col.pos_in_table, + dst_reg: idx_start_reg + i, 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); } + // 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() {