diff --git a/CHANGELOG.md b/CHANGELOG.md index ef03e3b..afbc0cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ All notable changes to sqlite-rs. Format follows [Keep a Changelog](https://keep **Versioning policy:** one minor version per completed plan phase — the version number tells the plan's story, sub-steps stay inside a phase. V1 (READ CORE) = 0.1.0 through 0.4.0. *(History note: internal iterations briefly numbered 0.4.0–0.6.0 were renumbered into the phase scheme on 14 Aug 2026, before any tag or publication of those versions existed.)* +## [0.18.9] - 2026-08-30 + +### Fixed + +- `IndexCursor::seek()` scanned linearly from the first entry, fully + decoding every candidate cell until finding one `>= target`: O(n) per + seek. It now does a real O(log n) tree descent, binary-searching each + level's cell array and falling back to the nearest ancestor's + qualifying cell when a descended-into subtree has nothing to offer — + mirroring `TableCursor::seek`'s binary search, which never got applied + to the index-cursor side. Measured ~13.8% faster on an indexed range + scan and ~7% faster on an indexed join (#661). + ## [0.18.8] - 2026-08-29 ### Fixed diff --git a/Cargo.lock b/Cargo.lock index 00e06f7..4f6369c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -781,7 +781,7 @@ checksum = "8ed6a63f02c8539c91a8685a86f4099661ba3da017932f6ebbea6de3f0fa7c90" [[package]] name = "sqlite-rs" -version = "0.18.8" +version = "0.18.9" dependencies = [ "criterion", "md-5", diff --git a/Cargo.toml b/Cargo.toml index 48e5f3a..75ad8ad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sqlite-rs" -version = "0.18.8" +version = "0.18.9" edition = "2021" publish = false license = "Apache-2.0" diff --git a/README.md b/README.md index 082cae1..599e9f2 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ See [.openspec/plan.md](.openspec/plan.md) for the full breakdown and [.openspec ## Status -**Version 0.18.8** — see [CHANGELOG.md](CHANGELOG.md). One minor version per completed plan phase. +**Version 0.18.9** — see [CHANGELOG.md](CHANGELOG.md). One minor version per completed plan phase. | Phase | Version | Status | |-------|---------|--------| diff --git a/src/btree/index.rs b/src/btree/index.rs index 0f70cbc..de24a09 100644 --- a/src/btree/index.rs +++ b/src/btree/index.rs @@ -17,9 +17,9 @@ //! Key comparison (NULL < numeric < text < blob, BINARY collation only — //! Tier 0 scope) is minimal by design, per the originating issue: enough //! ordering to walk in the correct sequence, not a fully general seek. -//! [`IndexCursor::seek`] is a linear scan from the first entry rather -//! than a tree descent, trading O(log n) for a much simpler, harder-to- -//! get-wrong implementation — acceptable at Tier 0 scope. +//! [`IndexCursor::seek`] does a real O(log n) tree descent (#661), +//! binary-searching each level's cell array rather than scanning linearly +//! from the first entry. use std::cmp::Ordering; use std::rc::Rc; @@ -135,22 +135,90 @@ impl IndexCursor

{ /// Returns the first entry (in ascending key order) whose decoded key /// is not less than `target`, or `None` if every entry is less than - /// `target`. A linear scan from the first entry — see the module doc - /// for why that's an intentional Tier 0 simplification. + /// `target`. + /// + /// A real tree descent (#661): at each interior level, binary-searches + /// that page's cell array for the leftmost cell whose key is not less + /// than `target`, then descends into that cell's left child (which + /// may itself hold a qualifying entry smaller than the cell's own + /// key) — recording that cell as a fallback by leaving this frame's + /// `step` positioned just past the descend, exactly as normal + /// [`Self::advance`] traversal would after visiting it. If no cell on + /// a level qualifies, descends into `rightmost` instead. At the leaf, + /// binary-searches for the matching entry directly. Either way, the + /// stack left behind is indistinguishable from one built by ordinary + /// [`Self::first`]/[`Self::next`] traversal up to this point, so a + /// final [`Self::advance`] call yields the right entry — the leaf + /// match if there is one, or (if the leaf has nothing to offer) pops + /// back up to the nearest ancestor's fallback cell, or `None` if + /// nothing on the path ever qualified. This decodes only the O(log n) + /// candidate cells actually inspected per level, not every cell in + /// the tree. + #[allow( + clippy::indexing_slicing, + reason = "top = stack.len() - 1, computed just above from a non-empty stack (just pushed); always in bounds" + )] pub fn seek( &mut self, target: &[Value], encoding: TextEncoding, ) -> Result, BtreeError> { - let mut row = self.first()?; - while let Some(r) = row { - let key = decode_record(&r.payload, encoding)?; + self.stack.clear(); + self.pages_visited = 0; + self.push_page(self.root_page)?; + loop { + let top = self.stack.len().saturating_sub(1); + let (is_interior, num_cells) = { + let f = &self.stack[top]; + (f.is_interior, f.num_cells) + }; + if !is_interior { + let i = self.binary_search_page(top, num_cells, target, encoding, false)?; + self.stack[top].step = i; + break; + } + let i = self.binary_search_page(top, num_cells, target, encoding, true)?; + let child = if i < num_cells { + self.stack[top].step = i.saturating_mul(2).saturating_add(1); + self.read_interior_child(top, i)? + } else { + self.stack[top].step = num_cells.saturating_mul(2).saturating_add(1); + self.stack[top].rightmost + }; + self.push_page(child)?; + } + self.advance() + } + + /// Binary-searches `top`'s cell array (interior or leaf, per + /// `is_interior`) for the leftmost cell index whose decoded key is + /// not less than `target`, or `num_cells` if none qualify. Only the + /// O(log n) cells actually probed get their payload decoded. + fn binary_search_page( + &self, + top: usize, + num_cells: usize, + target: &[Value], + encoding: TextEncoding, + is_interior: bool, + ) -> Result { + let mut lo = 0usize; + let mut hi = num_cells; + while lo < hi { + let mid = lo.saturating_add(hi.saturating_sub(lo) / 2); + let row = if is_interior { + self.decode_interior_entry(top, mid)? + } else { + self.decode_leaf_entry(top, mid)? + }; + let key = decode_record(&row.payload, encoding)?; if compare_keys(&key, target) != Ordering::Less { - return Ok(Some(r)); + hi = mid; + } else { + lo = mid.saturating_add(1); } - row = self.next()?; } - Ok(None) + Ok(lo) } fn read_page(&mut self, page_num: u32) -> Result, BtreeError> { @@ -892,6 +960,67 @@ mod tests { assert_eq!(int(&key[1]), 100); } + /// #661: `seek`'s binary-search tree descent must land on exactly the + /// entry a linear full scan would, for a value between two existing + /// keys (not an exact hit on any stored key — exercises the + /// interior-cell fallback path when a descended-into child's subtree + /// turns out to hold nothing `>= target`) — and the cursor must then + /// be positioned correctly for `next()` to continue in order. + #[test] + fn secondary_index_seek_between_keys_matches_full_scan_and_next_continues() { + let mut scan = open_cursor("index.db", 3); + let mut rows = Vec::new(); + let mut row = scan.first().unwrap(); + while let Some(r) = row { + rows.push(decode_record(&r.payload, TextEncoding::Utf8).unwrap()); + row = scan.next().unwrap(); + } + + // BINARY collation: "row number 15" sorts between "row number 1" + // and "row number 150"/"row number 1500" etc., but isn't itself a + // stored key. + let target = [Value::Text("row number 15".to_string().into())]; + let expect_idx = rows + .iter() + .position(|k| compare_keys(k, &target) != Ordering::Less) + .expect("some row must be >= target"); + + let mut cursor = open_cursor("index.db", 3); + let landed = cursor.seek(&target, TextEncoding::Utf8).unwrap().unwrap(); + let landed_key = decode_record(&landed.payload, TextEncoding::Utf8).unwrap(); + assert_eq!(landed_key, rows[expect_idx]); + + // `next()` from here must continue exactly where a full scan + // would, proving the stack seek() leaves behind is positioned + // like ordinary first()/next() traversal. + for expected in &rows[expect_idx.saturating_add(1)..] { + let next_row = cursor.next().unwrap().expect("more rows expected"); + let next_key = decode_record(&next_row.payload, TextEncoding::Utf8).unwrap(); + assert_eq!(&next_key, expected); + } + assert!(cursor.next().unwrap().is_none()); + } + + #[test] + fn secondary_index_seek_past_every_key_returns_none() { + let mut cursor = open_cursor("index.db", 3); + let target = [Value::Text("zzz-past-everything".to_string().into())]; + assert!(cursor.seek(&target, TextEncoding::Utf8).unwrap().is_none()); + } + + #[test] + fn secondary_index_seek_before_every_key_returns_first_row() { + let mut cursor = open_cursor("index.db", 3); + let target = [Value::Text(String::new().into())]; + let row = cursor.seek(&target, TextEncoding::Utf8).unwrap().unwrap(); + let key = decode_record(&row.payload, TextEncoding::Utf8).unwrap(); + + let mut expect = open_cursor("index.db", 3); + let first = expect.first().unwrap().unwrap(); + let first_key = decode_record(&first.payload, TextEncoding::Utf8).unwrap(); + assert_eq!(key, first_key); + } + /// #52 tagged MC/DC vector (obligation `index_868`, the ordering-check /// decision `idx < expect_order.len() && text(&key[0]) == expect_order[idx]` /// inside `without_rowid_table_is_readable_as_index_btree` below): both