aljaz/turso
1
0
Fork 0
mirror of https://github.com/aljazceru/turso.git synced 2026-02-05 00:04:23 +01:00
Code Issues Packages Projects Releases Wiki Activity

btree: unify table&index seek page boundary handling

Browse Source 
PR #2065 fixed a bug with table btree seeks concerning boundaries
of leaf pages.

The issue was that if we were e.g. looking for the first key greater than
(GT) 100, we always assumed the key would either be found on the left child
page of a given divider (e.g. divider 102), which is incorrect. #2065 has more
discussion and documentation about this, so read that one for more context.

Anyway:

We already had similar handling for index btrees, but it was baked into
the `BTreeCursor` struct's seek handling itself, whereas #2065 handled this
on the VDBE side.

This PR unifies this handling for both table and index btrees by always doing
the additional cursor advancement in the VDBE.

Unfortunately, since indexes may also need to do an additional advance when they
are looking for an exact match, this resulted in a bigger refactor than anticipated,
since there are quite a few VDBE instructions that may perform a seek, e.g.:
`IdxInsert`, `IdxDelete`, `Found`, `NotFound`, `NoConflict`.

All of these can potentially end up in a similar situation where the cursor needs
one more advance after the initial seek.

For this reason, I have extracted a common VDBE helper `fn seek_internal()` which
all the interested VDBE instructions will call to delegate their seek logic.
This commit is contained in:
Jussi Saurio
2025-07-14 15:20:59 +03:00
parent 90532eabdf
commit 553396e9ca
3 changed files with 451 additions and 379 deletions
Download Patch File Download Diff File Expand all files Collapse all files

136
core/storage/btree.rs
View File

@@ -463,9 +463,6 @@ pub enum CursorSeekState {
/// 1. We have not seen an EQ during the traversal
/// 2. We are looking for an exact match ([SeekOp::GE] or [SeekOp::LE] with eq_only: true)
eq_seen: Cell<bool>,
/// Indicates when we have not not found a value in leaf and now will look in the next/prev record.
/// This value is only used for indexbtree
moving_up_to_parent: Cell<bool>,
},
}
@@ -1291,15 +1288,7 @@ impl BTreeCursor {
self.tablebtree_seek(rowid, op)
}
SeekKey::IndexKey(index_key) => {
match self.indexbtree_seek(index_key, op) {
Ok(CursorResult::Ok(found)) => Ok(CursorResult::Ok(if found {
SeekResult::Found
} else {
SeekResult::NotFound
})),
Ok(CursorResult::IO) => Ok(CursorResult::IO),
Err(err) => Err(err),
}
self.indexbtree_seek(index_key, op)
}
});
self.valid_state = CursorValidState::Valid;
@@ -1725,7 +1714,6 @@ impl BTreeCursor {
min_cell_idx,
max_cell_idx,
nearest_matching_cell,
moving_up_to_parent: Cell::new(false),
eq_seen: Cell::new(false), // not relevant for table btrees
};
}
@@ -1828,7 +1816,7 @@ impl BTreeCursor {
&mut self,
key: &ImmutableRecord,
seek_op: SeekOp,
) -> Result<CursorResult<bool>> {
) -> Result<CursorResult<SeekResult>> {
let key_values = key.get_values();
let index_info_default = IndexKeyInfo::default();
let index_info = *self.index_key_info.as_ref().unwrap_or(&index_info_default);
@@ -1861,15 +1849,7 @@ impl BTreeCursor {
let contents = page.get().contents.as_ref().unwrap();
let cell_count = contents.cell_count();
if cell_count == 0 {
self.stack.set_cell_index(0);
match seek_op.iteration_direction() {
IterationDirection::Forwards => {
return self.next();
}
IterationDirection::Backwards => {
return self.prev();
}
}
return Ok(CursorResult::Ok(SeekResult::NotFound));
}
let min = Cell::new(0);
@@ -1883,7 +1863,6 @@ impl BTreeCursor {
min_cell_idx: min,
max_cell_idx: max,
nearest_matching_cell,
moving_up_to_parent: Cell::new(false),
eq_seen: Cell::new(eq_seen),
};
}
@@ -1893,7 +1872,6 @@ impl BTreeCursor {
max_cell_idx,
nearest_matching_cell,
eq_seen,
moving_up_to_parent,
} = &self.seek_state
else {
unreachable!(
@@ -1902,47 +1880,6 @@ impl BTreeCursor {
);
};
if moving_up_to_parent.get() {
let page = self.stack.top();
return_if_locked_maybe_load!(self.pager, page);
let page = page.get();
let contents = page.get().contents.as_ref().unwrap();
let cur_cell_idx = self.stack.current_cell_index() as usize;
let cell = contents.cell_get(cur_cell_idx, self.usable_space())?;
let BTreeCell::IndexInteriorCell(IndexInteriorCell {
payload,
first_overflow_page,
payload_size,
..
}) = &cell
else {
unreachable!("unexpected cell type: {:?}", cell);
};
if let Some(next_page) = first_overflow_page {
return_if_io!(self.process_overflow_read(payload, *next_page, *payload_size))
} else {
self.get_immutable_record_or_create()
.as_mut()
.unwrap()
.invalidate();
self.get_immutable_record_or_create()
.as_mut()
.unwrap()
.start_serialization(payload);
self.record_cursor.borrow_mut().invalidate();
};
let (_, found) = self.compare_with_current_record(
key_values.as_slice(),
seek_op,
&record_comparer,
&index_info,
);
moving_up_to_parent.set(false);
return Ok(CursorResult::Ok(found));
}
let page = self.stack.top();
return_if_locked_maybe_load!(self.pager, page);
let page = page.get();
@@ -1957,56 +1894,20 @@ impl BTreeCursor {
if min > max {
if let Some(nearest_matching_cell) = nearest_matching_cell.get() {
self.stack.set_cell_index(nearest_matching_cell as i32);
return Ok(CursorResult::Ok(true));
return Ok(CursorResult::Ok(SeekResult::Found));
} else {
// We have now iterated over all cells in the leaf page and found no match.
// Unlike tables, indexes store payloads in interior cells as well. self.move_to() always moves to a leaf page, so there are cases where we need to
// move back up to the parent interior cell and get the next record from there to perform a correct seek.
// an example of how this can occur:
//
// we do an index seek for key K with cmp = SeekOp::GT, meaning we want to seek to the first key that is greater than K.
// in self.move_to(), we encounter an interior cell with key K' = K+2, and move the left child page, which is a leaf page.
// the reason we move to the left child page is that we know that in an index, all keys in the left child page are less than K' i.e. less than K+2,
// meaning that the left subtree may contain a key greater than K, e.g. K+1. however, it is possible that it doesn't, in which case the correct
// next key is K+2, which is in the parent interior cell.
//
// In the seek() method, once we have landed in the leaf page and find that there is no cell with a key greater than K,
// if we were to return Ok(CursorResult::Ok((None, None))), self.record would be None, which is incorrect, because we already know
// that there is a record with a key greater than K (K' = K+2) in the parent interior cell. Hence, we need to move back up the tree
// and get the next matching record from there.
//
// However we only do this if either of the following is true:
// - We have seen an EQ match up in the tree in an interior node
// - Or, we are not looking for an exact match.
// Similar logic as in tablebtree_seek(), but for indexes.
// The difference is that since index keys are not necessarily unique, we need to TryAdvance
// even when eq_only=true and we have seen an EQ match up in the tree in an interior node.
if seek_op.eq_only() && !eq_seen.get() {
return Ok(CursorResult::Ok(false));
}
match iter_dir {
IterationDirection::Forwards => {
if !moving_up_to_parent.get() {
moving_up_to_parent.set(true);
self.stack.set_cell_index(cell_count as i32);
}
let next_res = return_if_io!(self.next());
if !next_res {
return Ok(CursorResult::Ok(false));
}
// FIXME: optimize this in case record can be read directly
return Ok(CursorResult::IO);
}
IterationDirection::Backwards => {
if !moving_up_to_parent.get() {
moving_up_to_parent.set(true);
self.stack.set_cell_index(-1);
}
let prev_res = return_if_io!(self.prev());
if !prev_res {
return Ok(CursorResult::Ok(false));
}
// FIXME: optimize this in case record can be read directly
return Ok(CursorResult::IO);
}
return Ok(CursorResult::Ok(SeekResult::NotFound));
}
// set cursor to the position where which would hold the op-boundary if it were present
self.stack.set_cell_index(match &seek_op {
SeekOp::GT | SeekOp::GE { .. } => cell_count as i32,
SeekOp::LT | SeekOp::LE { .. } => 0,
});
return Ok(CursorResult::Ok(SeekResult::TryAdvance));
};
}
@@ -7164,10 +7065,11 @@ mod tests {
pager.deref(),
)
.unwrap();
assert!(
matches!(exists, SeekResult::Found),
"key {key:?} is not found"
);
let mut found = matches!(exists, SeekResult::Found);
if matches!(exists, SeekResult::TryAdvance) {
found = run_until_done(|| cursor.next(), pager.deref()).unwrap();
}
assert!(found, "key {key:?} is not found");
}
// Check that key count is right
cursor.move_to_root().unwrap();

690
core/vdbe/execute.rs
View File

@@ -2392,11 +2392,22 @@ pub fn op_deferred_seek(
Ok(InsnFunctionStepResult::Step)
}
/// Separate enum for seek key to avoid lifetime issues
/// with using [SeekKey] - OpSeekState always owns the key,
/// unless it's [OpSeekKey::IndexKeyFromRegister] in which case the record
/// is owned by the program state's registers and we store the register number.
#[derive(Debug)]
pub enum OpSeekKey {
TableRowId(i64),
IndexKeyOwned(ImmutableRecord),
IndexKeyFromRegister(usize),
}
pub enum OpSeekState {
/// Initial state
Start,
/// Position cursor with seek operation with (rowid, op) search parameters
Seek { rowid: i64, op: SeekOp },
Seek { key: OpSeekKey, op: SeekOp },
/// Advance cursor (with [BTreeCursor::next]/[BTreeCursor::prev] methods) which was
/// positioned after [OpSeekState::Seek] state if [BTreeCursor::seek] returned [SeekResult::TryAdvance]
Advance { op: SeekOp },
@@ -2411,56 +2422,52 @@ pub fn op_seek(
pager: &Rc<Pager>,
mv_store: Option<&Rc<MvStore>>,
) -> Result<InsnFunctionStepResult> {
let result = op_seek_internal(program, state, insn, pager, mv_store);
if let Err(_) | Ok(InsnFunctionStepResult::Step) = &result {
state.op_seek_state = OpSeekState::Start;
}
result
}
pub fn op_seek_internal(
program: &Program,
state: &mut ProgramState,
insn: &Insn,
pager: &Rc<Pager>,
mv_store: Option<&Rc<MvStore>>,
) -> Result<InsnFunctionStepResult> {
let (Insn::SeekGE {
cursor_id,
start_reg,
num_regs,
target_pc,
is_index,
..
}
| Insn::SeekGT {
cursor_id,
start_reg,
num_regs,
target_pc,
is_index,
}
| Insn::SeekLE {
cursor_id,
start_reg,
num_regs,
target_pc,
is_index,
..
}
| Insn::SeekLT {
cursor_id,
start_reg,
num_regs,
target_pc,
is_index,
}) = insn
else {
unreachable!("unexpected Insn {:?}", insn)
let (cursor_id, is_index, record_source, target_pc) = match insn {
Insn::SeekGE {
cursor_id,
is_index,
start_reg,
num_regs,
target_pc,
..
}
| Insn::SeekLE {
cursor_id,
is_index,
start_reg,
num_regs,
target_pc,
..
}
| Insn::SeekGT {
cursor_id,
is_index,
start_reg,
num_regs,
target_pc,
..
}
| Insn::SeekLT {
cursor_id,
is_index,
start_reg,
num_regs,
target_pc,
..
} => (
cursor_id,
*is_index,
RecordSource::Unpacked {
start_reg: *start_reg,
num_regs: *num_regs,
},
target_pc,
),
_ => unreachable!("unexpected Insn {:?}", insn),
};
assert!(
target_pc.is_offset(),
"target_pc should be an offset, is: {target_pc:?}"
"op_seek: target_pc should be an offset, is: {target_pc:?}"
);
let eq_only = match insn {
Insn::SeekGE { eq_only, .. } | Insn::SeekLE { eq_only, .. } => *eq_only,
@@ -2473,169 +2480,280 @@ pub fn op_seek_internal(
Insn::SeekLT { .. } => SeekOp::LT,
_ => unreachable!("unexpected Insn {:?}", insn),
};
if *is_index {
let seek_result = {
let mut cursor = state.get_cursor(*cursor_id);
let cursor = cursor.as_btree_mut();
let record_from_regs = make_record(&state.registers, start_reg, num_regs);
return_if_io!(cursor.seek(SeekKey::IndexKey(&record_from_regs), op))
};
if !matches!(seek_result, SeekResult::Found) {
state.pc = target_pc.as_offset_int();
} else {
match seek_internal(
program,
state,
pager,
mv_store,
record_source,
*cursor_id,
is_index,
op,
) {
Ok(SeekInternalResult::Found) => {
state.pc += 1;
Ok(InsnFunctionStepResult::Step)
}
return Ok(InsnFunctionStepResult::Step);
Ok(SeekInternalResult::NotFound) => {
state.pc = target_pc.as_offset_int();
Ok(InsnFunctionStepResult::Step)
}
Ok(SeekInternalResult::IO) => Ok(InsnFunctionStepResult::IO),
Err(e) => Err(e),
}
loop {
match &state.op_seek_state {
OpSeekState::Start => {
let original_value = state.registers[*start_reg].get_owned_value().clone();
let mut temp_value = original_value.clone();
}
let conversion_successful = if matches!(temp_value, Value::Text(_)) {
let mut temp_reg = Register::Value(temp_value);
let converted = apply_numeric_affinity(&mut temp_reg, false);
temp_value = temp_reg.get_owned_value().clone();
converted
} else {
true // Non-text values don't need conversion
};
let int_key = extract_int_value(&temp_value);
let lost_precision =
!conversion_successful || !matches!(temp_value, Value::Integer(_));
let actual_op = if lost_precision {
match &temp_value {
Value::Float(f) => {
let int_key_as_float = int_key as f64;
let c = if int_key_as_float > *f {
1
} else if int_key_as_float < *f {
-1
} else {
0
};
#[derive(Debug, PartialEq)]
pub enum SeekInternalResult {
Found,
NotFound,
IO,
}
pub enum RecordSource {
Unpacked { start_reg: usize, num_regs: usize },
Packed { record_reg: usize },
}
match c.cmp(&0) {
std::cmp::Ordering::Less => match op {
SeekOp::LT => SeekOp::LE { eq_only: false }, // (x < 5.1) -> (x <= 5)
SeekOp::GE { .. } => SeekOp::GT, // (x >= 5.1) -> (x > 5)
other => other,
},
std::cmp::Ordering::Greater => match op {
SeekOp::GT => SeekOp::GE { eq_only: false }, // (x > 4.9) -> (x >= 5)
SeekOp::LE { .. } => SeekOp::LT, // (x <= 4.9) -> (x < 5)
other => other,
},
std::cmp::Ordering::Equal => op,
/// Internal function used by many VDBE instructions that need to perform a seek operation.
///
/// Explanation for some of the arguments:
/// - `record_source`: whether the seek key record is already a record (packed) or it will be constructed from registers (unpacked)
/// - `cursor_id`: the cursor id
/// - `is_index`: true if the cursor is an index, false if it is a table
/// - `op`: the [SeekOp] to perform
#[allow(clippy::too_many_arguments)]
pub fn seek_internal(
program: &Program,
state: &mut ProgramState,
pager: &Rc<Pager>,
mv_store: Option<&Rc<MvStore>>,
record_source: RecordSource,
cursor_id: usize,
is_index: bool,
op: SeekOp,
) -> Result<SeekInternalResult> {
/// wrapper so we can use the ? operator and handle errors correctly in this outer function
fn inner(
program: &Program,
state: &mut ProgramState,
pager: &Rc<Pager>,
mv_store: Option<&Rc<MvStore>>,
record_source: RecordSource,
cursor_id: usize,
is_index: bool,
op: SeekOp,
) -> Result<SeekInternalResult> {
loop {
match &state.seek_state {
OpSeekState::Start => {
if is_index {
// FIXME: text-to-numeric conversion should also happen here when applicable (when index column is numeric)
// See below for the table-btree implementation of this
match record_source {
RecordSource::Unpacked {
start_reg,
num_regs,
} => {
let record_from_regs =
make_record(&state.registers, &start_reg, &num_regs);
state.seek_state = OpSeekState::Seek {
key: OpSeekKey::IndexKeyOwned(record_from_regs),
op,
};
}
}
Value::Text(_) | Value::Blob(_) => {
match op {
SeekOp::GT | SeekOp::GE { .. } => {
// No integers are > or >= non-numeric text, jump to target (empty result)
state.pc = target_pc.as_offset_int();
return Ok(InsnFunctionStepResult::Step);
}
SeekOp::LT | SeekOp::LE { .. } => {
// All integers are < or <= non-numeric text
// Move to last position and then use the normal seek logic
state.op_seek_state = OpSeekState::MoveLast;
continue;
}
RecordSource::Packed { record_reg } => {
state.seek_state = OpSeekState::Seek {
key: OpSeekKey::IndexKeyFromRegister(record_reg),
op,
};
}
}
_ => op,
}
} else {
op
};
let rowid = if matches!(original_value, Value::Null) {
match actual_op {
SeekOp::GE { .. } | SeekOp::GT => {
state.pc = target_pc.as_offset_int();
return Ok(InsnFunctionStepResult::Step);
}
SeekOp::LE { .. } | SeekOp::LT => {
// No integers are < NULL, so jump to target
state.pc = target_pc.as_offset_int();
return Ok(InsnFunctionStepResult::Step);
}
}
} else {
int_key
};
state.op_seek_state = OpSeekState::Seek {
rowid,
op: actual_op,
};
continue;
}
OpSeekState::Seek { rowid, op } => {
let seek_result = {
let mut cursor = state.get_cursor(*cursor_id);
let cursor = cursor.as_btree_mut();
return_if_io!(cursor.seek(SeekKey::TableRowId(*rowid), *op))
};
let found = match seek_result {
SeekResult::Found => true,
SeekResult::NotFound => false,
SeekResult::TryAdvance => {
state.op_seek_state = OpSeekState::Advance { op: *op };
};
continue;
}
};
if !found {
state.pc = target_pc.as_offset_int()
} else {
state.pc += 1
let RecordSource::Unpacked {
start_reg,
num_regs,
} = record_source
else {
unreachable!("op_seek: record_source should be Unpacked for table-btree");
};
assert_eq!(num_regs, 1, "op_seek: num_regs should be 1 for table-btree");
let original_value = state.registers[start_reg].get_owned_value().clone();
let mut temp_value = original_value.clone();
let conversion_successful = if matches!(temp_value, Value::Text(_)) {
let mut temp_reg = Register::Value(temp_value);
let converted = apply_numeric_affinity(&mut temp_reg, false);
temp_value = temp_reg.get_owned_value().clone();
converted
} else {
true // Non-text values don't need conversion
};
let int_key = extract_int_value(&temp_value);
let lost_precision =
!conversion_successful || !matches!(temp_value, Value::Integer(_));
let actual_op = if lost_precision {
match &temp_value {
Value::Float(f) => {
let int_key_as_float = int_key as f64;
let c = if int_key_as_float > *f {
1
} else if int_key_as_float < *f {
-1
} else {
0
};
match c.cmp(&0) {
std::cmp::Ordering::Less => match op {
SeekOp::LT => SeekOp::LE { eq_only: false }, // (x < 5.1) -> (x <= 5)
SeekOp::GE { .. } => SeekOp::GT, // (x >= 5.1) -> (x > 5)
other => other,
},
std::cmp::Ordering::Greater => match op {
SeekOp::GT => SeekOp::GE { eq_only: false }, // (x > 4.9) -> (x >= 5)
SeekOp::LE { .. } => SeekOp::LT, // (x <= 4.9) -> (x < 5)
other => other,
},
std::cmp::Ordering::Equal => op,
}
}
Value::Text(_) | Value::Blob(_) => {
match op {
SeekOp::GT | SeekOp::GE { .. } => {
// No integers are > or >= non-numeric text
return Ok(SeekInternalResult::NotFound);
}
SeekOp::LT | SeekOp::LE { .. } => {
// All integers are < or <= non-numeric text
// Move to last position and then use the normal seek logic
state.seek_state = OpSeekState::MoveLast;
continue;
}
}
}
_ => op,
}
} else {
op
};
let rowid = if matches!(original_value, Value::Null) {
match actual_op {
SeekOp::GE { .. } | SeekOp::GT => {
return Ok(SeekInternalResult::NotFound);
}
SeekOp::LE { .. } | SeekOp::LT => {
// No integers are < NULL
return Ok(SeekInternalResult::NotFound);
}
}
} else {
int_key
};
state.seek_state = OpSeekState::Seek {
key: OpSeekKey::TableRowId(rowid),
op: actual_op,
};
continue;
}
return Ok(InsnFunctionStepResult::Step);
}
OpSeekState::Advance { op } => {
let found = {
let mut cursor = state.get_cursor(*cursor_id);
OpSeekState::Seek { key, op } => {
let seek_result = {
let mut cursor = state.get_cursor(cursor_id);
let cursor = cursor.as_btree_mut();
let seek_key = match key {
OpSeekKey::TableRowId(rowid) => SeekKey::TableRowId(*rowid),
OpSeekKey::IndexKeyOwned(record) => SeekKey::IndexKey(record),
OpSeekKey::IndexKeyFromRegister(record_reg) => match &state.registers[*record_reg] {
Register::Record(ref record) => SeekKey::IndexKey(record),
_ => unreachable!("op_seek: record_reg should be a Record register when OpSeekKey::IndexKeyFromRegister is used"),
}
};
match cursor.seek(seek_key, *op)? {
CursorResult::Ok(seek_result) => seek_result,
CursorResult::IO => return Ok(SeekInternalResult::IO),
}
};
let found = match seek_result {
SeekResult::Found => true,
SeekResult::NotFound => false,
SeekResult::TryAdvance => {
state.seek_state = OpSeekState::Advance { op: *op };
continue;
}
};
return Ok(if found {
SeekInternalResult::Found
} else {
SeekInternalResult::NotFound
});
}
OpSeekState::Advance { op } => {
let found = {
let mut cursor = state.get_cursor(cursor_id);
let cursor = cursor.as_btree_mut();
// Seek operation has anchor number which equals to the closed boundary of the range
// (e.g. for >= x - anchor is x, for > x - anchor is x + 1)
//
// Before Advance state, cursor was positioned to the leaf page which should hold the anchor.
// Sometimes this leaf page can have no matching rows, and in this case
// we need to move cursor in the direction of Seek to find record which matches the seek filter
//
// Consider following scenario: Seek [> 666]
// interior page dividers: I1: [ .. 667 .. ]
// / \
// leaf pages: P1[661,665] P2[anything here is GT 666]
// After the initial Seek, cursor will be positioned after the end of leaf page P1 [661, 665]
// because this is potential position for insertion of value 666.
// But as P1 has no row matching Seek criteria - we need to move it to the right
// (and as we at the page boundary, we will move cursor to the next neighbor leaf, which guaranteed to have
// row keys greater than divider, which is greater or equal than anchor)
// this same logic applies for indexes, but the next/prev record is expected to be found in the parent page's
// divider cell.
let result = match op {
SeekOp::GT { .. } | SeekOp::GE { .. } => cursor.next()?,
SeekOp::LT { .. } | SeekOp::LE { .. } => cursor.prev()?,
};
match result {
CursorResult::Ok(found) => found,
CursorResult::IO => return Ok(SeekInternalResult::IO),
}
};
return Ok(if found {
SeekInternalResult::Found
} else {
SeekInternalResult::NotFound
});
}
OpSeekState::MoveLast => {
let mut cursor = state.get_cursor(cursor_id);
let cursor = cursor.as_btree_mut();
// Seek operation has anchor number which equals to the closed boundary of the range
// (e.g. for >= x - anchor is x, for > x - anchor is x + 1)
//
// Before Advance state, cursor was positioned to the leaf page which should hold the anchor.
// Sometimes this leaf page can have no matching rows, and in this case
// we need to move cursor in the direction of Seek to find record which matches the seek filter
//
// Consider following scenario: Seek [> 666]
// interior page dividers: I1: [ .. 667 .. ]
// / \
// leaf pages: P1[661,665] P2[anything here is GT 666]
// After the initial Seek, cursor will be positioned after the end of leaf page P1 [661, 665]
// because this is potential position for insertion of value 666.
// But as P1 has no row matching Seek criteria - we need to move it to the right
// (and as we at the page boundary, we will move cursor to the next neighbor leaf, which guaranteed to have
// row keys greater than divider, which is greater or equal than anchor)
match op {
SeekOp::GT | SeekOp::GE { eq_only: false } => return_if_io!(cursor.next()),
SeekOp::LT | SeekOp::LE { eq_only: false } => return_if_io!(cursor.prev()),
_ => unreachable!("eq_only: true state must be unreachable"),
match cursor.last()? {
CursorResult::Ok(()) => {}
CursorResult::IO => return Ok(SeekInternalResult::IO),
}
};
if !found {
state.pc = target_pc.as_offset_int()
} else {
state.pc += 1
// the MoveLast variant is only used for SeekOp::LT and SeekOp::LE when the seek condition is always true,
// so we have always found what we were looking for.
return Ok(SeekInternalResult::Found);
}
return Ok(InsnFunctionStepResult::Step);
}
OpSeekState::MoveLast => {
{
let mut cursor = state.get_cursor(*cursor_id);
let cursor = cursor.as_btree_mut();
return_if_io!(cursor.last());
}
state.pc += 1;
return Ok(InsnFunctionStepResult::Step);
}
}
}
let result = inner(
program,
state,
pager,
mv_store,
record_source,
cursor_id,
is_index,
op,
);
if !matches!(result, Ok(SeekInternalResult::IO)) {
state.seek_state = OpSeekState::Start;
}
result
}
/// Returns the tie-breaker ordering for SQLite index comparison opcodes.
@@ -4837,7 +4955,7 @@ pub fn op_delete(
#[derive(Debug)]
pub enum OpIdxDeleteState {
Seeking(ImmutableRecord), // First seek row to delete
Seeking,
Verifying,
Deleting,
}
@@ -4857,6 +4975,7 @@ pub fn op_idx_delete(
else {
unreachable!("unexpected Insn {:?}", insn)
};
loop {
tracing::debug!(
"op_idx_delete(cursor_id={}, start_reg={}, num_regs={}, rootpage={}, state={:?})",
@@ -4867,26 +4986,31 @@ pub fn op_idx_delete(
state.op_idx_delete_state
);
match &state.op_idx_delete_state {
Some(OpIdxDeleteState::Seeking(record)) => {
let found = {
let mut cursor = state.get_cursor(*cursor_id);
let cursor = cursor.as_btree_mut();
let seek_result = return_if_io!(
cursor.seek(SeekKey::IndexKey(record), SeekOp::GE { eq_only: true })
);
tracing::debug!(
"op_idx_delete: found={:?}, rootpage={}, key={:?}",
seek_result,
cursor.root_page(),
record
);
matches!(seek_result, SeekResult::Found)
Some(OpIdxDeleteState::Seeking) => {
let found = match seek_internal(
program,
state,
pager,
mv_store,
RecordSource::Unpacked {
start_reg: *start_reg,
num_regs: *num_regs,
},
*cursor_id,
true,
SeekOp::GE { eq_only: true },
) {
Ok(SeekInternalResult::Found) => true,
Ok(SeekInternalResult::NotFound) => false,
Ok(SeekInternalResult::IO) => return Ok(InsnFunctionStepResult::IO),
Err(e) => return Err(e),
};
if !found {
// If P5 is not zero, then raise an SQLITE_CORRUPT_INDEX error if no matching index entry is found
// Also, do not raise this (self-correcting and non-critical) error if in writable_schema mode.
if *raise_error_if_no_matching_entry {
let record = make_record(&state.registers, start_reg, num_regs);
return Err(LimboError::Corrupt(format!(
"IdxDelete: no matching index entry found for record {record:?}"
)));
@@ -4925,8 +5049,7 @@ pub fn op_idx_delete(
return Ok(InsnFunctionStepResult::Step);
}
None => {
let record = make_record(&state.registers, start_reg, num_regs);
state.op_idx_delete_state = Some(OpIdxDeleteState::Seeking(record));
state.op_idx_delete_state = Some(OpIdxDeleteState::Seeking);
}
}
}
@@ -4981,17 +5104,27 @@ pub fn op_idx_insert(
};
return Ok(InsnFunctionStepResult::Step);
}
{
let mut cursor = state.get_cursor(cursor_id);
let cursor = cursor.as_btree_mut();
return_if_io!(cursor.seek(
SeekKey::IndexKey(record_to_insert),
SeekOp::GE { eq_only: true }
));
match seek_internal(
program,
state,
pager,
mv_store,
RecordSource::Packed { record_reg },
cursor_id,
true,
SeekOp::GE { eq_only: true },
)? {
SeekInternalResult::Found => {
state.op_idx_insert_state = OpIdxInsertState::UniqueConstraintCheck;
Ok(InsnFunctionStepResult::Step)
}
SeekInternalResult::NotFound => {
state.op_idx_insert_state = OpIdxInsertState::Insert { moved_before: true };
Ok(InsnFunctionStepResult::Step)
}
SeekInternalResult::IO => Ok(InsnFunctionStepResult::IO),
}
state.op_idx_insert_state = OpIdxInsertState::UniqueConstraintCheck;
Ok(InsnFunctionStepResult::Step)
}
OpIdxInsertState::UniqueConstraintCheck => {
let ignore_conflict = 'i: {
@@ -5239,6 +5372,10 @@ pub fn op_soft_null(
Ok(InsnFunctionStepResult::Step)
}
/// If a matching record is not found in the btree ("no conflict"), jump to the target PC.
/// Otherwise, continue execution.
/// FIXME: create a state machine for op_no_conflict so that it doesn't do the nulls checking
/// multiple times in the case of yielding on IO.
pub fn op_no_conflict(
program: &Program,
state: &mut ProgramState,
@@ -5258,39 +5395,67 @@ pub fn op_no_conflict(
let mut cursor_ref = state.get_cursor(*cursor_id);
let cursor = cursor_ref.as_btree_mut();
let record = if *num_regs == 0 {
match &state.registers[*record_reg] {
Register::Record(r) => r,
_ => {
return Err(LimboError::InternalError(
"NoConflict: exepected a record in the register".into(),
));
}
let record_source = if *num_regs == 0 {
RecordSource::Packed {
record_reg: *record_reg,
}
} else {
&make_record(&state.registers, record_reg, num_regs)
RecordSource::Unpacked {
start_reg: *record_reg,
num_regs: *num_regs,
}
};
// If there is at least one NULL in the index record, there cannot be a conflict so we can immediately jump.
let contains_nulls = record
.get_values()
.iter()
.any(|val| matches!(val, RefValue::Null));
let contains_nulls = match &record_source {
RecordSource::Packed { record_reg } => {
let Register::Record(record) = &state.registers[*record_reg] else {
return Err(LimboError::InternalError(
"NoConflict: expected a record in the register".into(),
));
};
record
.get_values()
.iter()
.any(|val| matches!(val, RefValue::Null))
}
RecordSource::Unpacked {
start_reg,
num_regs,
} => (0..*num_regs).any(|i| {
matches!(
&state.registers[start_reg + i],
Register::Value(Value::Null)
)
}),
};
if contains_nulls {
drop(cursor_ref);
state.pc = target_pc.as_offset_int();
return Ok(InsnFunctionStepResult::Step);
}
let seek_result =
return_if_io!(cursor.seek(SeekKey::IndexKey(record), SeekOp::GE { eq_only: true }));
drop(cursor_ref);
if !matches!(seek_result, SeekResult::Found) {
state.pc = target_pc.as_offset_int();
} else {
state.pc += 1;
match seek_internal(
program,
state,
pager,
mv_store,
record_source,
*cursor_id,
true,
SeekOp::GE { eq_only: true },
)? {
SeekInternalResult::Found => {
state.pc += 1;
Ok(InsnFunctionStepResult::Step)
}
SeekInternalResult::NotFound => {
state.pc = target_pc.as_offset_int();
Ok(InsnFunctionStepResult::Step)
}
SeekInternalResult::IO => Ok(InsnFunctionStepResult::IO),
}
Ok(InsnFunctionStepResult::Step)
}
pub fn op_not_exists(
@@ -6099,25 +6264,30 @@ pub fn op_found(
let not = matches!(insn, Insn::NotFound { .. });
let seek_result = {
let mut cursor = state.get_cursor(*cursor_id);
let cursor = cursor.as_btree_mut();
if *num_regs == 0 {
let record = match &state.registers[*record_reg] {
Register::Record(r) => r,
_ => {
return Err(LimboError::InternalError(
"NotFound: exepected a record in the register".into(),
));
}
};
return_if_io!(cursor.seek(SeekKey::IndexKey(record), SeekOp::GE { eq_only: true }))
} else {
let record = make_record(&state.registers, record_reg, num_regs);
return_if_io!(cursor.seek(SeekKey::IndexKey(&record), SeekOp::GE { eq_only: true }))
let record_source = if *num_regs == 0 {
RecordSource::Packed {
record_reg: *record_reg,
}
} else {
RecordSource::Unpacked {
start_reg: *record_reg,
num_regs: *num_regs,
}
};
let seek_result = match seek_internal(
program,
state,
pager,
mv_store,
record_source,
*cursor_id,
true,
SeekOp::GE { eq_only: true },
) {
Ok(SeekInternalResult::Found) => SeekResult::Found,
Ok(SeekInternalResult::NotFound) => SeekResult::NotFound,
Ok(SeekInternalResult::IO) => return Ok(InsnFunctionStepResult::IO),
Err(e) => return Err(e),
};
let found = matches!(seek_result, SeekResult::Found);

4
core/vdbe/mod.rs
View File

@@ -258,7 +258,7 @@ pub struct ProgramState {
op_new_rowid_state: OpNewRowidState,
op_idx_insert_state: OpIdxInsertState,
op_insert_state: OpInsertState,
op_seek_state: OpSeekState,
seek_state: OpSeekState,
}
impl ProgramState {
@@ -288,7 +288,7 @@ impl ProgramState {
op_new_rowid_state: OpNewRowidState::Start,
op_idx_insert_state: OpIdxInsertState::SeekIfUnique,
op_insert_state: OpInsertState::Insert,
op_seek_state: OpSeekState::Start,
seek_state: OpSeekState::Start,
}
}