mirrors/limbo
1
0
Fork 0
mirror of https://github.com/tursodatabase/limbo.git synced 2025-08-04 18:18:03 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 13

update and delete fixes

Browse source
This commit is contained in:
Jussi Saurio 2025-06-06 13:08:21 +03:00
parent d81f5f67bd
commit 844461d20b
5 changed files with 98 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

1
.github/workflows/rust.yml vendored
View file

@ -67,6 +67,7 @@ jobs:
test-limbo:
runs-on: blacksmith-4vcpu-ubuntu-2404
timeout-minutes: 20
steps:
- name: Install cargo-c
env:

72
core/storage/btree.rs
View file

@ -176,7 +176,9 @@ enum DeleteState {
cell_idx: usize,
original_child_pointer: Option<u32>,
},
CheckNeedsBalancing,
CheckNeedsBalancing {
rightmost_cell_was_dropped: bool,
},
WaitForBalancingToComplete {
target_key: DeleteSavepoint,
},
@ -1117,8 +1119,8 @@ impl BTreeCursor {
}
loop {
let mem_page_rc = self.stack.top();
tracing::trace!(
"current_before_advance id={} cell={}",
tracing::debug!(
"next: current_before_advance id={} cell={}",
mem_page_rc.get().get().id,
self.stack.current_cell_index()
);
@ -1142,8 +1144,8 @@ impl BTreeCursor {
// Important to advance only after loading the page in order to not advance > 1 times
self.stack.advance();
let cell_idx = self.stack.current_cell_index() as usize;
tracing::trace!(
"current id={} cell={}",
tracing::debug!(
"next:current id={} cell={}",
mem_page_rc.get().get().id,
cell_idx
);
@ -1619,7 +1621,12 @@ impl BTreeCursor {
assert!(self.mv_cursor.is_none());
let iter_dir = seek_op.iteration_direction();
if matches!(self.seek_state, CursorSeekState::Start { .. }) {
if matches!(
self.seek_state,
CursorSeekState::Start { .. }
| CursorSeekState::MovingBetweenPages { .. }
| CursorSeekState::InteriorPageBinarySearch { .. }
) {
// No need for another move_to_root. Move_to already moves to root
return_if_io!(self.move_to(SeekKey::TableRowId(rowid), seek_op));
let page = self.stack.top();
@ -1733,7 +1740,12 @@ impl BTreeCursor {
key: &ImmutableRecord,
seek_op: SeekOp,
) -> Result<CursorResult<bool>> {
if matches!(self.seek_state, CursorSeekState::Start { .. }) {
if matches!(
self.seek_state,
CursorSeekState::Start { .. }
| CursorSeekState::MovingBetweenPages { .. }
| CursorSeekState::InteriorPageBinarySearch { .. }
) {
// No need for another move_to_root. Move_to already moves to root
return_if_io!(self.move_to(SeekKey::IndexKey(key), seek_op));
let CursorSeekState::FoundLeaf { eq_seen } = &self.seek_state else {
@ -1785,7 +1797,10 @@ impl BTreeCursor {
moving_up_to_parent,
} = &self.seek_state
else {
unreachable!("we must be in a leaf binary search state");
unreachable!(
"we must be in a leaf binary search state, got: {:?}",
self.seek_state
);
};
if moving_up_to_parent.get() {
@ -2019,7 +2034,15 @@ impl BTreeCursor {
// 5. We scan the leaf cells in the leaf page until we find the cell whose rowid is equal to the rowid we are looking for.
// This cell contains the actual data we are looking for.
// 6. If we find the cell, we return the record. Otherwise, we return an empty result.
if matches!(self.seek_state, CursorSeekState::Start { .. }) {
// If we are at the beginning/end of seek state, start a new move from the root.
if matches!(
self.seek_state,
CursorSeekState::LeafPageBinarySearch { .. }
) {
self.seek_state = CursorSeekState::Start;
}
if matches!(self.seek_state, CursorSeekState::Start) {
self.move_to_root();
}
@ -4082,7 +4105,6 @@ impl BTreeCursor {
// find_cell() iterates the leaf page from left to right to find the insertion point anyway, so we don't care
// which cell we are in as long as we are on the right page.
// FIXME: find_cell() should not use linear search because it's slow.
self.seek_state = CursorSeekState::Start;
match key {
BTreeKey::IndexKey(_) => {
return_if_io!(self.move_to(
@ -4193,6 +4215,12 @@ impl BTreeCursor {
));
}
tracing::debug!(
"DeleteState::FindCell: page_id: {}, cell_idx: {}",
page.get().id,
cell_idx
);
let cell = contents.cell_get(
cell_idx,
payload_overflow_threshold_max(
@ -4231,6 +4259,8 @@ impl BTreeCursor {
let page = page.get();
let contents = page.get_contents();
let is_last_cell = cell_idx == contents.cell_count().saturating_sub(1);
let delete_info = self.state.mut_delete_info().unwrap();
if !contents.is_leaf() {
delete_info.state = DeleteState::InteriorNodeReplacement {
@ -4242,7 +4272,9 @@ impl BTreeCursor {
drop_cell(contents, cell_idx, self.usable_space() as u16)?;
let delete_info = self.state.mut_delete_info().unwrap();
delete_info.state = DeleteState::CheckNeedsBalancing;
delete_info.state = DeleteState::CheckNeedsBalancing {
rightmost_cell_was_dropped: is_last_cell,
};
}
}
@ -4319,10 +4351,14 @@ impl BTreeCursor {
drop_cell(parent_contents, cell_idx, self.usable_space() as u16)?;
let delete_info = self.state.mut_delete_info().unwrap();
delete_info.state = DeleteState::CheckNeedsBalancing;
delete_info.state = DeleteState::CheckNeedsBalancing {
rightmost_cell_was_dropped: false, // TODO: when is this true?
};
}
DeleteState::CheckNeedsBalancing => {
DeleteState::CheckNeedsBalancing {
rightmost_cell_was_dropped,
} => {
let page = self.stack.top();
return_if_locked_maybe_load!(self.pager, page);
@ -4332,9 +4368,15 @@ impl BTreeCursor {
let needs_balancing = self.stack.has_parent()
&& free_space as usize * 3 > self.usable_space() * 2;
if rightmost_cell_was_dropped {
// If we drop a cell in the middle, e.g. our current index is 2 and we drop 'c' from [a,b,c,d,e], then we don't need to retreat index,
// because index 2 is still going to be the right place [a,b,D,e]
// but:
// If we drop the rightmost cell, e.g. our current index is 4 and we drop 'e' from [a,b,c,d,e], then we need to retreat index,
// because index 4 is now pointing beyond the last cell [a,b,c,d] _ <-- index 4
self.stack.retreat();
}
if needs_balancing {
// FIXME(pere): cell index must be updated before calling `rowid` or
// `record`
let target_key = if page.is_index() {
let record = match return_if_io!(self.record()) {
Some(record) => record.clone(),

15
core/translate/emitter.rs
View file

@ -870,9 +870,20 @@ 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 {
if let Some(index_cursor) = program.resolve_cursor_id_safe(&CursorKey::index(
plan.table_references
.joined_tables()
.first()
.unwrap()
.internal_id,
index.clone(),
)) {
// Don't reopen index if it was already opened as the iteration cursor for this update plan.
let record_reg = program.alloc_register();
index_cursors.push((index_cursor, record_reg));
continue;
}
let index_cursor = program.alloc_cursor_id(CursorType::BTreeIndex(index.clone()));
program.emit_insn(Insn::OpenWrite {
cursor_id: index_cursor,

21
core/translate/optimizer/mod.rs
View file

@ -98,7 +98,7 @@ fn optimize_delete_plan(plan: &mut DeletePlan, schema: &Schema) -> Result<()> {
Ok(())
}
fn optimize_update_plan(plan: &mut UpdatePlan, schema: &Schema) -> Result<()> {
fn optimize_update_plan(plan: &mut UpdatePlan, _schema: &Schema) -> Result<()> {
rewrite_exprs_update(plan)?;
if let ConstantConditionEliminationResult::ImpossibleCondition =
eliminate_constant_conditions(&mut plan.where_clause)?
@ -106,13 +106,18 @@ fn optimize_update_plan(plan: &mut UpdatePlan, schema: &Schema) -> Result<()> {
plan.contains_constant_false_condition = true;
return Ok(());
}
let _ = optimize_table_access(
&mut plan.table_references,
&schema.indexes,
&mut plan.where_clause,
&mut plan.order_by,
&mut None,
)?;
// FIXME: don't use indexes for update right now because it's not safe to traverse an index
// while also updating the same table, things go wrong.
// e.g. in 'explain update t set x=x+5 where x > 10;' where x is an indexed column,
// sqlite first creates an ephemeral index to store the current values so the tree traversal
// doesn't get messed up while updating.
// let _ = optimize_table_access(
// &mut plan.table_references,
// &schema.indexes,
// &mut plan.where_clause,
// &mut plan.order_by,
// &mut None,
// )?;
Ok(())
}

23
core/vdbe/execute.rs
View file

@ -1345,11 +1345,16 @@ pub fn op_column(
unreachable!("unexpected Insn {:?}", insn)
};
if let Some((index_cursor_id, table_cursor_id)) = state.deferred_seeks[*cursor_id].take() {
let deferred_seek = {
let deferred_seek = 'd: {
let rowid = {
let mut index_cursor = state.get_cursor(index_cursor_id);
let index_cursor = index_cursor.as_btree_mut();
return_if_io!(index_cursor.rowid())
match index_cursor.rowid()? {
CursorResult::IO => {
break 'd Some((index_cursor_id, table_cursor_id));
}
CursorResult::Ok(rowid) => rowid,
}
};
let mut table_cursor = state.get_cursor(table_cursor_id);
let table_cursor = table_cursor.as_btree_mut();
@ -1904,11 +1909,16 @@ pub fn op_row_id(
unreachable!("unexpected Insn {:?}", insn)
};
if let Some((index_cursor_id, table_cursor_id)) = state.deferred_seeks[*cursor_id].take() {
let deferred_seek = {
let deferred_seek = 'd: {
let rowid = {
let mut index_cursor = state.get_cursor(index_cursor_id);
let index_cursor = index_cursor.as_btree_mut();
let record = return_if_io!(index_cursor.record());
let record = match index_cursor.record()? {
CursorResult::IO => {
break 'd Some((index_cursor_id, table_cursor_id));
}
CursorResult::Ok(record) => record,
};
let record = record.as_ref().unwrap();
let rowid = record.get_values().last().unwrap();
match rowid {
@ -3903,11 +3913,6 @@ pub fn op_delete(
{
let mut cursor = state.get_cursor(*cursor_id);
let cursor = cursor.as_btree_mut();
tracing::debug!(
"op_delete(rowid ={:?}, record={:?})",
cursor.rowid()?,
cursor.record()?,
);
return_if_io!(cursor.delete());
}
let prev_changes = program.n_change.get();