From 692f0413eb641e113115b588c170d9a5d69a137e Mon Sep 17 00:00:00 2001 From: Krishna Vishal Date: Thu, 26 Jun 2025 11:33:31 +0530 Subject: [PATCH] Stash --- core/storage/btree.rs | 726 +++++++++++++++++++++++++++++++++++++----- core/types.rs | 13 +- core/vdbe/execute.rs | 21 +- 3 files changed, 679 insertions(+), 81 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 9f7c57542..d975bbe4f 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -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> { + // 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> { 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> { + 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() { diff --git a/core/types.rs b/core/types.rs index 35834204e..b99a95db4 100644 --- a/core/types.rs +++ b/core/types.rs @@ -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 + 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) -> ImmutableRecord { let registers: Vec = values.into_iter().map(Register::Value).collect(); - ImmutableRecord::from_registers(®isters) + ImmutableRecord::from_registers(®isters, registers.len()) } fn create_index_info(num_cols: usize, sort_orders: Vec) -> IndexKeyInfo { diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 993ce20f0..820395765 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -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(), );