This commit is contained in:
Krishna Vishal
2025-06-26 11:33:31 +05:30
parent 0b1ed44c1f
commit 692f0413eb
3 changed files with 679 additions and 81 deletions

View File

@@ -13,7 +13,10 @@ use crate::{
},
translate::{collate::CollationSeq, plan::IterationDirection},
turso_assert,
types::{IndexKeyInfo, IndexKeySortOrder, ParseRecordState, RecordCursor},
types::{
compare_records_generic, find_compare, IndexKeyInfo, IndexKeySortOrder, ParseRecordState,
RecordCompare, RecordCursor,
},
MvCursor,
};
@@ -1442,6 +1445,379 @@ impl BTreeCursor {
}
}
fn compare_with_current_record_debug(
&self,
key: &ImmutableRecord,
seek_op: SeekOp,
record_comparer: &RecordCompare,
index_info: &IndexKeyInfo,
) -> (Ordering, bool) {
let record = self.get_immutable_record();
let record = record.as_ref().unwrap();
let key_values = key.get_values();
// OLD WAY - compare_immutable
let old_cmp = {
let record_values = record.get_values();
let record_slice_equal_number_of_cols = &record_values[..key_values.len()];
compare_immutable(
record_slice_equal_number_of_cols,
key_values.as_slice(),
self.key_sort_order(),
&self.collations,
)
};
// NEW WAY - optimized comparison
let new_cmp =
match record_comparer.compare(record, &key_values, index_info, &self.collations, 0) {
Ok(ordering) => ordering,
Err(e) => {
println!("Optimized record comparison failed: {:?}", e);
// For debugging, let's also try generic comparison
match compare_records_generic(
record,
&key_values,
index_info,
&self.collations,
0,
) {
Ok(ordering) => {
println!("Generic comparison succeeded where optimized failed");
ordering
}
Err(fallback_err) => {
println!("Generic comparison also failed: {:?}", fallback_err);
old_cmp // Use old comparison as final fallback
}
}
}
};
// COMPARE RESULTS
if old_cmp != new_cmp {
println!(
"COMPARISON MISMATCH! Old: {:?}, New: {:?}, Strategy: {:?}, Key: {:?}",
old_cmp, new_cmp, record_comparer, key_values
);
// Log more details for debugging
let record_values = record.get_values();
println!(
"Record values: {:?}, Key values: {:?}, Index info: {:?}",
record_values, key_values, index_info
);
// For now, use the old comparison to maintain correctness while debugging
let cmp = old_cmp;
let found = match seek_op {
SeekOp::GT => cmp.is_gt(),
SeekOp::GE { eq_only: true } => cmp.is_eq(),
SeekOp::GE { eq_only: false } => cmp.is_ge(),
SeekOp::LE { eq_only: true } => cmp.is_eq(),
SeekOp::LE { eq_only: false } => cmp.is_le(),
SeekOp::LT => cmp.is_lt(),
};
return (cmp, found);
}
// Results match, use the new comparison
let cmp = new_cmp;
let found = match seek_op {
SeekOp::GT => cmp.is_gt(),
SeekOp::GE { eq_only: true } => cmp.is_eq(),
SeekOp::GE { eq_only: false } => cmp.is_ge(),
SeekOp::LE { eq_only: true } => cmp.is_eq(),
SeekOp::LE { eq_only: false } => cmp.is_le(),
SeekOp::LT => cmp.is_lt(),
};
(cmp, found)
}
// /// Specialized version of move_to() for index btrees.
// #[instrument(skip(self, index_key), level = Level::TRACE)]
// fn indexbtree_move_to(
// &mut self,
// index_key: &ImmutableRecord,
// cmp: SeekOp,
// ) -> Result<CursorResult<()>> {
// let iter_dir = cmp.iteration_direction();
// let key_values = index_key.get_values();
// let index_info = &self.index_key_info.unwrap();
// let record_comparer = find_compare(&key_values, &index_info, &self.collations);
// println!("record_comparer: {:?}", record_comparer);
// tracing::debug!("Using record comparison strategy: {:?}", record_comparer);
// 'outer: loop {
// 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();
// if contents.is_leaf() {
// let eq_seen = match &self.seek_state {
// CursorSeekState::MovingBetweenPages { eq_seen } => eq_seen.get(),
// _ => false,
// };
// self.seek_state = CursorSeekState::FoundLeaf {
// eq_seen: Cell::new(eq_seen),
// };
// return Ok(CursorResult::Ok(()));
// }
// if matches!(
// self.seek_state,
// CursorSeekState::Start | CursorSeekState::MovingBetweenPages { .. }
// ) {
// let eq_seen = match &self.seek_state {
// CursorSeekState::MovingBetweenPages { eq_seen } => eq_seen.get(),
// _ => false,
// };
// let cell_count = contents.cell_count();
// let min_cell_idx = Cell::new(0);
// let max_cell_idx = Cell::new(cell_count as isize - 1);
// let nearest_matching_cell = Cell::new(None);
// self.seek_state = CursorSeekState::InteriorPageBinarySearch {
// min_cell_idx,
// max_cell_idx,
// nearest_matching_cell,
// eq_seen: Cell::new(eq_seen),
// };
// }
// let CursorSeekState::InteriorPageBinarySearch {
// min_cell_idx,
// max_cell_idx,
// nearest_matching_cell,
// eq_seen,
// } = &self.seek_state
// else {
// unreachable!(
// "we must be in an interior binary search state, got {:?}",
// self.seek_state
// );
// };
// loop {
// let min = min_cell_idx.get();
// let max = max_cell_idx.get();
// if min > max {
// let Some(leftmost_matching_cell) = nearest_matching_cell.get() else {
// self.stack.set_cell_index(contents.cell_count() as i32 + 1);
// match contents.rightmost_pointer() {
// Some(right_most_pointer) => {
// let mem_page = self.read_page(right_most_pointer as usize)?;
// self.stack.push(mem_page);
// self.seek_state = CursorSeekState::MovingBetweenPages {
// eq_seen: Cell::new(eq_seen.get()),
// };
// continue 'outer;
// }
// None => {
// unreachable!(
// "we shall not go back up! The only way is down the slope"
// );
// }
// }
// };
// 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(),
// )?;
// self.stack.set_cell_index(leftmost_matching_cell as i32);
// // we don't advance in case of forward iteration and index tree internal nodes because we will visit this node going up.
// // in backwards iteration, we must retreat because otherwise we would unnecessarily visit this node again.
// // Example:
// // this parent: key 666, and we found the target key in the left child.
// // left child has: key 663, key 664, key 665
// // we need to move to the previous parent (with e.g. key 662) when iterating backwards so that we don't end up back here again.
// if iter_dir == IterationDirection::Backwards {
// self.stack.retreat();
// }
// let BTreeCell::IndexInteriorCell(IndexInteriorCell {
// left_child_page, ..
// }) = &matching_cell
// else {
// unreachable!("unexpected cell type: {:?}", matching_cell);
// };
// let mem_page = self.read_page(*left_child_page as usize)?;
// self.stack.push(mem_page);
// self.seek_state = CursorSeekState::MovingBetweenPages {
// eq_seen: Cell::new(eq_seen.get()),
// };
// continue 'outer;
// }
// 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,
// 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 BTreeCell::IndexInteriorCell(IndexInteriorCell {
// payload,
// payload_size,
// first_overflow_page,
// ..
// }) = &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()
// .start_serialization(payload);
// self.record_cursor.borrow_mut().invalidate();
// };
// let (target_leaf_page_is_in_left_subtree, is_eq) = {
// let record = self.get_immutable_record();
// let record = record.as_ref().unwrap();
// // DEBUG COMPARISON - Compare old vs new method
// let old_cmp = {
// let record_values = record.get_values();
// let record_slice_equal_number_of_cols = &record_values[..key_values.len()];
// compare_immutable(
// record_slice_equal_number_of_cols,
// key_values.as_slice(),
// self.key_sort_order(),
// &self.collations,
// )
// };
// let new_cmp = match record_comparer.compare(
// record,
// &key_values,
// &index_info,
// &self.collations,
// 0,
// ) {
// Ok(ordering) => ordering,
// Err(e) => {
// println!("Optimized record comparison failed: {:?}", e);
// match compare_records_generic(
// record,
// &key_values,
// &index_info,
// &self.collations,
// 0,
// ) {
// Ok(ordering) => {
// println!("Generic comparison succeeded where optimized failed");
// ordering
// }
// Err(fallback_err) => {
// println!("Generic comparison also failed: {:?}", fallback_err);
// old_cmp // Use old comparison as final fallback
// }
// }
// }
// };
// println!("old_cmp: {:?}, new_cmp: {:?}", old_cmp, new_cmp);
// // Check for mismatches
// if old_cmp != new_cmp {
// println!(
// "COMPARISON MISMATCH in indexbtree_move_to! Old: {:?}, New: {:?}, Strategy: {:?}",
// old_cmp, new_cmp, record_comparer);
// let record_values = record.get_values();
// println!(
// "Record values: {:?}, Key values: {:?}, Index info: {:?}",
// record_values, key_values, index_info
// );
// // Use old comparison for correctness while debugging
// let interior_cell_vs_index_key = old_cmp;
// } else {
// // Use new comparison if they match
// let interior_cell_vs_index_key = new_cmp;
// }
// let interior_cell_vs_index_key =
// if old_cmp != new_cmp { old_cmp } else { new_cmp };
// // in sqlite btrees left child pages have <= keys.
// // in general, in forwards iteration we want to find the first key that matches the seek condition.
// // in backwards iteration we want to find the last key that matches the seek condition.
// //
// // Logic table for determining if target leaf page is in left subtree.
// // For index b-trees this is a bit more complicated since the interior cells contain payloads (the key is the payload).
// // and for non-unique indexes there might be several cells with the same key.
// //
// // Forwards iteration (looking for first match in tree):
// // OP | Current Cell vs Seek Key | Action? | Explanation
// // GT | > | go left | First > key could be exactly this one, or in left subtree
// // GT | = or < | go right | First > key must be in right subtree
// // GE | > | go left | First >= key could be exactly this one, or in left subtree
// // GE | = | go left | First >= key could be exactly this one, or in left subtree
// // GE | < | go right | First >= key must be in right subtree
// //
// // Backwards iteration (looking for last match in tree):
// // OP | Current Cell vs Seek Key | Action? | Explanation
// // LE | > | go left | Last <= key must be in left subtree
// // LE | = | go right | Last <= key is either this one, or somewhere to the right of this one. So we need to go right to make sure
// // LE | < | go right | Last <= key must be in right subtree
// // LT | > | go left | Last < key must be in left subtree
// // LT | = | go left | Last < key must be in left subtree since we want strictly less than
// // LT | < | go right | Last < key could be exactly this one, or in right subtree
// //
// // No iteration (point query):
// // EQ | > | go left | First = key must be in left subtree
// // EQ | = | go left | First = key could be exactly this one, or in left subtree
// // EQ | < | go right | First = key must be in right subtree
// (
// match cmp {
// SeekOp::GT => interior_cell_vs_index_key.is_gt(),
// SeekOp::GE { .. } => interior_cell_vs_index_key.is_ge(),
// SeekOp::LE { .. } => interior_cell_vs_index_key.is_gt(),
// SeekOp::LT => interior_cell_vs_index_key.is_ge(),
// },
// interior_cell_vs_index_key.is_eq(),
// )
// };
// if is_eq {
// eq_seen.set(true);
// }
// if target_leaf_page_is_in_left_subtree {
// nearest_matching_cell.set(Some(cur_cell_idx as usize));
// max_cell_idx.set(cur_cell_idx - 1);
// } else {
// min_cell_idx.set(cur_cell_idx + 1);
// }
// }
// }
// }
/// Specialized version of move_to() for index btrees.
#[instrument(skip(self, index_key), level = Level::INFO)]
fn indexbtree_move_to(
@@ -1450,11 +1826,27 @@ impl BTreeCursor {
cmp: SeekOp,
) -> Result<CursorResult<()>> {
let iter_dir = cmp.iteration_direction();
let key_values = index_key.get_values();
let index_info = &self.index_key_info.unwrap();
let record_comparer = find_compare(&key_values, &index_info, &self.collations);
println!("=== INDEXBTREE_MOVE_TO START ===");
println!("Searching for: {:?}", key_values);
println!("record_comparer: {:?}", record_comparer);
tracing::debug!("Using record comparison strategy: {:?}", record_comparer);
'outer: loop {
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();
println!(
"Current page: {}, is_leaf: {}",
page.get().id,
contents.is_leaf()
);
if contents.is_leaf() {
let eq_seen = match &self.seek_state {
CursorSeekState::MovingBetweenPages { eq_seen } => eq_seen.get(),
@@ -1463,9 +1855,15 @@ impl BTreeCursor {
self.seek_state = CursorSeekState::FoundLeaf {
eq_seen: Cell::new(eq_seen),
};
println!("=== REACHED LEAF PAGE ===");
return Ok(CursorResult::Ok(()));
}
println!(
"Processing interior page with {} cells",
contents.cell_count()
);
if matches!(
self.seek_state,
CursorSeekState::Start | CursorSeekState::MovingBetweenPages { .. }
@@ -1503,11 +1901,16 @@ impl BTreeCursor {
loop {
let min = min_cell_idx.get();
let max = max_cell_idx.get();
println!("Binary search: min={}, max={}", min, max);
if min > max {
println!("Binary search complete");
let Some(leftmost_matching_cell) = nearest_matching_cell.get() else {
println!("No matching cell found, going to rightmost child");
self.stack.set_cell_index(contents.cell_count() as i32 + 1);
match contents.rightmost_pointer() {
Some(right_most_pointer) => {
println!("Going to rightmost child page: {}", right_most_pointer);
let mem_page = self.read_page(right_most_pointer as usize)?;
self.stack.push(mem_page);
self.seek_state = CursorSeekState::MovingBetweenPages {
@@ -1525,12 +1928,7 @@ impl BTreeCursor {
let matching_cell =
contents.cell_get(leftmost_matching_cell, self.usable_space())?;
self.stack.set_cell_index(leftmost_matching_cell as i32);
// we don't advance in case of forward iteration and index tree internal nodes because we will visit this node going up.
// in backwards iteration, we must retreat because otherwise we would unnecessarily visit this node again.
// Example:
// this parent: key 666, and we found the target key in the left child.
// left child has: key 663, key 664, key 665
// we need to move to the previous parent (with e.g. key 662) when iterating backwards so that we don't end up back here again.
if iter_dir == IterationDirection::Backwards {
self.stack.retreat();
}
@@ -1541,6 +1939,7 @@ impl BTreeCursor {
unreachable!("unexpected cell type: {:?}", matching_cell);
};
println!("Going to left child page: {}", left_child_page);
let mem_page = self.read_page(*left_child_page as usize)?;
self.stack.push(mem_page);
self.seek_state = CursorSeekState::MovingBetweenPages {
@@ -1549,7 +1948,8 @@ impl BTreeCursor {
continue 'outer;
}
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 cur_cell_idx = (min + max) >> 1;
println!("Examining cell at index: {}", cur_cell_idx);
self.stack.set_cell_index(cur_cell_idx as i32);
let cell = contents.cell_get(cur_cell_idx as usize, self.usable_space())?;
let BTreeCell::IndexInteriorCell(IndexInteriorCell {
@@ -1571,57 +1971,43 @@ impl BTreeCursor {
.start_serialization(payload);
self.record_cursor.borrow_mut().invalidate();
};
let (target_leaf_page_is_in_left_subtree, is_eq) = {
let record = self.get_immutable_record();
let record = record.as_ref().unwrap();
let record_values = record.get_values();
let record_slice_equal_number_of_cols =
&record_values[..index_key.get_values().len()];
let interior_cell_vs_index_key = compare_immutable(
record_slice_equal_number_of_cols,
index_key.get_values().as_slice(),
self.key_sort_order(),
&self.collations,
);
// in sqlite btrees left child pages have <= keys.
// in general, in forwards iteration we want to find the first key that matches the seek condition.
// in backwards iteration we want to find the last key that matches the seek condition.
//
// Logic table for determining if target leaf page is in left subtree.
// For index b-trees this is a bit more complicated since the interior cells contain payloads (the key is the payload).
// and for non-unique indexes there might be several cells with the same key.
//
// Forwards iteration (looking for first match in tree):
// OP | Current Cell vs Seek Key | Action? | Explanation
// GT | > | go left | First > key could be exactly this one, or in left subtree
// GT | = or < | go right | First > key must be in right subtree
// GE | > | go left | First >= key could be exactly this one, or in left subtree
// GE | = | go left | First >= key could be exactly this one, or in left subtree
// GE | < | go right | First >= key must be in right subtree
//
// Backwards iteration (looking for last match in tree):
// OP | Current Cell vs Seek Key | Action? | Explanation
// LE | > | go left | Last <= key must be in left subtree
// LE | = | go right | Last <= key is either this one, or somewhere to the right of this one. So we need to go right to make sure
// LE | < | go right | Last <= key must be in right subtree
// LT | > | go left | Last < key must be in left subtree
// LT | = | go left | Last < key must be in left subtree since we want strictly less than
// LT | < | go right | Last < key could be exactly this one, or in right subtree
//
// No iteration (point query):
// EQ | > | go left | First = key must be in left subtree
// EQ | = | go left | First = key could be exactly this one, or in left subtree
// EQ | < | go right | First = key must be in right subtree
(
match cmp {
SeekOp::GT => interior_cell_vs_index_key.is_gt(),
SeekOp::GE { .. } => interior_cell_vs_index_key.is_ge(),
SeekOp::LE { .. } => interior_cell_vs_index_key.is_gt(),
SeekOp::LT => interior_cell_vs_index_key.is_ge(),
},
interior_cell_vs_index_key.is_eq(),
)
// USE OLD COMPARISON ONLY (temporarily disable optimization)
let interior_cell_vs_index_key = {
let record_values = record.get_values();
let record_slice_equal_number_of_cols = &record_values[..key_values.len()];
compare_immutable(
record_slice_equal_number_of_cols,
key_values.as_slice(),
self.key_sort_order(),
&self.collations,
)
};
println!(
"Interior cell comparison: key={:?} vs record={:?} => {:?}",
key_values,
record.get_values(),
interior_cell_vs_index_key
);
let target_is_left = match cmp {
SeekOp::GT => interior_cell_vs_index_key.is_gt(),
SeekOp::GE { .. } => interior_cell_vs_index_key.is_ge(),
SeekOp::LE { .. } => interior_cell_vs_index_key.is_gt(),
SeekOp::LT => interior_cell_vs_index_key.is_ge(),
};
println!(
"Navigation decision: seek_op={:?}, cell_vs_key={:?}, go_left={}",
cmp, interior_cell_vs_index_key, target_is_left
);
(target_is_left, interior_cell_vs_index_key.is_eq())
};
if is_eq {
@@ -1629,9 +2015,15 @@ impl BTreeCursor {
}
if target_leaf_page_is_in_left_subtree {
println!(
"Going left: setting nearest_matching_cell={}, max={}",
cur_cell_idx,
cur_cell_idx - 1
);
nearest_matching_cell.set(Some(cur_cell_idx as usize));
max_cell_idx.set(cur_cell_idx - 1);
} else {
println!("Going right: setting min={}", cur_cell_idx + 1);
min_cell_idx.set(cur_cell_idx + 1);
}
}
@@ -1766,6 +2158,15 @@ impl BTreeCursor {
key: &ImmutableRecord,
seek_op: SeekOp,
) -> Result<CursorResult<bool>> {
let key_values = key.get_values();
let index_info = &self.index_key_info.unwrap();
let record_comparer = find_compare(&key_values, &index_info, &self.collations);
tracing::debug!(
"Using record comparison strategy for seek: {:?}",
record_comparer
);
if matches!(
self.seek_state,
CursorSeekState::Start
@@ -1856,7 +2257,8 @@ impl BTreeCursor {
self.record_cursor.borrow_mut().invalidate();
};
let (_, found) = self.compare_with_current_record(key, seek_op);
let (_, found) =
self.compare_with_current_record(key, seek_op, &record_comparer, &index_info);
moving_up_to_parent.set(false);
return Ok(CursorResult::Ok(found));
}
@@ -1951,7 +2353,8 @@ impl BTreeCursor {
self.record_cursor.borrow_mut().invalidate();
};
let (cmp, found) = self.compare_with_current_record(key, seek_op);
let (cmp, found) =
self.compare_with_current_record(key, seek_op, &record_comparer, &index_info);
if found {
nearest_matching_cell.set(Some(cur_cell_idx as usize));
match iter_dir {
@@ -1979,25 +2382,159 @@ impl BTreeCursor {
}
}
// fn compare_with_current_record(
// &self,
// key: &ImmutableRecord,
// seek_op: SeekOp,
// record_comparer: &RecordCompare,
// index_info: &IndexKeyInfo,
// ) -> (Ordering, bool) {
// let cmp = {
// let record = self.get_immutable_record();
// let record = record.as_ref().unwrap();
// tracing::debug!(?record);
// let key_values = key.get_values();
// record_comparer
// .compare(record, &key_values, index_info, &self.collations, 0)
// .unwrap()
// };
// let old_cmp = {
// let record = self.get_immutable_record();
// let record = record.as_ref().unwrap();
// let record_values = record.get_values();
// let key_values = key.get_values();
// let record_slice_equal_number_of_cols = &record_values[..key_values.len()];
// compare_immutable(
// record_slice_equal_number_of_cols,
// key_values.as_slice(),
// self.key_sort_order(),
// &self.collations,
// )
// };
// println!("oldcmp: {:?}, newcmp: {:?}", old_cmp, cmp);
// let found = match seek_op {
// SeekOp::GT => cmp.is_gt(),
// SeekOp::GE { eq_only: true } => old_cmp.is_eq(),
// SeekOp::GE { eq_only: false } => old_cmp.is_ge(),
// SeekOp::LE { eq_only: true } => old_cmp.is_eq(),
// SeekOp::LE { eq_only: false } => old_cmp.is_le(),
// SeekOp::LT => old_cmp.is_lt(),
// };
// (old_cmp, found)
// }
fn compare_with_current_record(
&self,
key: &ImmutableRecord,
seek_op: SeekOp,
record_comparer: &RecordCompare,
index_info: &IndexKeyInfo,
) -> (Ordering, bool) {
let cmp = {
let record = self.get_immutable_record();
let record = record.as_ref().unwrap();
tracing::debug!(?record);
let record_values = record.get_values();
let key_values = key.get_values();
let record_slice_equal_number_of_cols = &record_values[..key_values.len()];
compare_immutable(
record_slice_equal_number_of_cols,
key_values.as_slice(),
self.key_sort_order(),
&self.collations,
)
};
let record = self.get_immutable_record();
let record = record.as_ref().unwrap();
tracing::debug!(?record);
let key_values = key.get_values();
// Debug: Print the actual values being compared
let record_values = record.get_values();
let record_slice_equal_number_of_cols = &record_values[..key_values.len()];
println!("=== COMPARISON DEBUG ===");
println!("Key values: {:?}", key_values);
println!("Record values (full): {:?}", record_values);
println!(
"Record values (slice): {:?}",
record_slice_equal_number_of_cols
);
// If we're dealing with integers, show the actual integer values
if !key_values.is_empty() && !record_slice_equal_number_of_cols.is_empty() {
match (&key_values[0], &record_slice_equal_number_of_cols[0]) {
(RefValue::Integer(key_int), RefValue::Integer(record_int)) => {
println!("Integer comparison: key={}, record={}", key_int, record_int);
}
_ => {
println!(
"Non-integer comparison: key={:?}, record={:?}",
key_values[0], record_slice_equal_number_of_cols[0]
);
}
}
}
// NEW WAY - optimized comparison
let new_cmp =
match record_comparer.compare(record, &key_values, index_info, &self.collations, 0) {
Ok(ordering) => ordering,
Err(e) => {
println!("Optimized record comparison failed: {:?}", e);
// For debugging, let's also try generic comparison
match compare_records_generic(
record,
&key_values,
index_info,
&self.collations,
0,
) {
Ok(ordering) => {
println!("Generic comparison succeeded where optimized failed");
ordering
}
Err(fallback_err) => {
println!("Generic comparison also failed: {:?}", fallback_err);
// Final fallback to compare_immutable
compare_immutable(
record_slice_equal_number_of_cols,
key_values.as_slice(),
self.key_sort_order(),
&self.collations,
)
}
}
}
};
// OLD WAY - compare_immutable (for debugging)
let old_cmp = compare_immutable(
record_slice_equal_number_of_cols,
key_values.as_slice(),
self.key_sort_order(),
&self.collations,
);
// Debug output
if old_cmp != new_cmp {
println!(
"COMPARISON MISMATCH! oldcmp: {:?}, newcmp: {:?}",
old_cmp, new_cmp
);
// Use old comparison for correctness while debugging
let cmp = old_cmp;
let found = match seek_op {
SeekOp::GT => cmp.is_gt(),
SeekOp::GE { eq_only: true } => cmp.is_eq(),
SeekOp::GE { eq_only: false } => cmp.is_ge(),
SeekOp::LE { eq_only: true } => cmp.is_eq(),
SeekOp::LE { eq_only: false } => cmp.is_le(),
SeekOp::LT => cmp.is_lt(),
};
println!("Using old comparison - found: {}", found);
return (cmp, found);
} else {
println!("oldcmp: {:?}, newcmp: {:?}", old_cmp, new_cmp);
}
// Use the new comparison consistently
let cmp = new_cmp;
let found = match seek_op {
SeekOp::GT => cmp.is_gt(),
SeekOp::GE { eq_only: true } => cmp.is_eq(),
@@ -2006,6 +2543,13 @@ impl BTreeCursor {
SeekOp::LE { eq_only: false } => cmp.is_le(),
SeekOp::LT => cmp.is_lt(),
};
println!(
"Final result - cmp: {:?}, found: {}, seek_op: {:?}",
cmp, found, seek_op
);
println!("========================");
(cmp, found)
}
@@ -6744,6 +7288,46 @@ mod tests {
(pager, page2.get().get().id, db, conn)
}
#[test]
fn debug_single_comparison() {
// Create a simple test case to isolate the comparison issue
let key = Value::Integer(0);
let record_values = vec![RefValue::Integer(0)]; // This should be equal
// Test the comparison directly
let index_info = IndexKeyInfo {
sort_order: IndexKeySortOrder::default(),
has_rowid: true,
num_cols: 1,
};
let collations = vec![CollationSeq::Binary];
let comparer = find_compare(&record_values, &index_info, &collations);
println!("Strategy selected: {:?}", comparer);
// Create an ImmutableRecord from the value
let register = Register::Value(Value::Integer(0));
let immutable_record = ImmutableRecord::from_registers(&[register], 1);
let result = comparer.compare(
&immutable_record,
&record_values,
&index_info,
&collations,
0,
);
println!("Comparison result: {:?}", result);
// Also test with compare_immutable for comparison
let old_result = compare_immutable(
&record_values,
&record_values,
index_info.sort_order,
&collations,
);
println!("Old comparison result: {:?}", old_result);
}
#[test]
#[ignore]
pub fn btree_insert_fuzz_ex() {

View File

@@ -860,9 +860,12 @@ impl ImmutableRecord {
}
}
pub fn from_registers(registers: &[Register]) -> Self {
let mut values = Vec::with_capacity(registers.len());
let mut serials = Vec::with_capacity(registers.len());
pub fn from_registers<'a>(
registers: impl IntoIterator<Item = &'a Register> + Copy,
len: usize,
) -> Self {
let mut values = Vec::with_capacity(len);
let mut serials = Vec::with_capacity(len);
let mut size_header = 0;
let mut size_values = 0;
@@ -1615,7 +1618,7 @@ fn compare_records_string(
}
}
fn compare_records_generic(
pub fn compare_records_generic(
serialized: &ImmutableRecord,
unpacked: &[RefValue],
index_info: &IndexKeyInfo,
@@ -2139,7 +2142,7 @@ mod tests {
fn create_record(values: Vec<Value>) -> ImmutableRecord {
let registers: Vec<Register> = values.into_iter().map(Register::Value).collect();
ImmutableRecord::from_registers(&registers)
ImmutableRecord::from_registers(&registers, registers.len())
}
fn create_index_info(num_cols: usize, sort_orders: Vec<SortOrder>) -> IndexKeyInfo {

View File

@@ -8,7 +8,7 @@ use crate::storage::pager::CreateBTreeFlags;
use crate::storage::wal::DummyWAL;
use crate::storage::{self, header_accessor};
use crate::translate::collate::CollationSeq;
use crate::types::{ImmutableRecord, Text};
use crate::types::{compare_immutable_for_testing, ImmutableRecord, Text};
use crate::util::normalize_ident;
use crate::{
error::{
@@ -2339,7 +2339,7 @@ pub fn op_idx_ge(
let idx_values = &idx_values[..record_values.len()];
let ord = compare_immutable(
idx_values,
record_values,
record_values.as_slice(),
cursor.key_sort_order(),
cursor.collations(),
);
@@ -2403,7 +2403,7 @@ pub fn op_idx_le(
let idx_values = &idx_values[..record_values.len()];
let ord = compare_immutable(
idx_values,
record_values,
record_values.as_slice(),
cursor.key_sort_order(),
cursor.collations(),
);
@@ -2442,6 +2442,7 @@ pub fn op_idx_gt(
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);
dbg!("here");
let pc = if let Some(ref idx_record) = return_if_io!(cursor.record()) {
// Compare against the same number of values
let idx_values = idx_record.get_values();
@@ -2449,16 +2450,26 @@ pub fn op_idx_gt(
let idx_values = &idx_values[..record_values.len()];
let ord = compare_immutable(
idx_values,
record_values,
record_values.as_slice(),
cursor.key_sort_order(),
cursor.collations(),
);
let ord_debug = compare_immutable_for_testing(
idx_values,
record_values.as_slice(),
cursor.key_sort_order(),
cursor.collations(),
);
println!("idx_gt== old={:?}, debug={:?}", ord, ord_debug);
if ord.is_gt() {
target_pc.as_offset_int()
} else {
state.pc + 1
}
} else {
println!("we are taking the else branch");
target_pc.as_offset_int()
};
pc
@@ -2495,7 +2506,7 @@ pub fn op_idx_lt(
let idx_values = &idx_values[..record_values.len()];
let ord = compare_immutable(
idx_values,
record_values,
record_values.as_slice(),
cursor.key_sort_order(),
cursor.collations(),
);