From d2ef39f352b520a2f3d6d0ef3b1bb02ed661d496 Mon Sep 17 00:00:00 2001 From: Ilja Heitlager Date: Sun, 30 Aug 2026 14:23:24 +0200 Subject: [PATCH 1/2] perf: route UPDATE range predicates through an index-seek scan (#666) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_indexed_range's SELECT fast path (range_scan.rs) already matched oracle's SeekGT+Next idiom with no per-row bound check. The real gap (confirmed against real sqlite3 EXPLAIN, not just EXPLAIN QUERY PLAN) was that UPDATE never used an index at all for `WHERE col >/>=/ ~107ms (~28% faster, cargo bench --bench crud). DELETE's equivalent fast path was prototyped but reverted: for a highly-selective predicate on a small table, the required two-pass materialization (an unavoidable random-order re-seek per matched row, since this engine's cursors can't safely self-mutate mid-scan) cost more than the full-scan it replaced. Real sqlite3 avoids this because its cursors support save/restore across writes — a materially larger, separate change. Follow-up: #666 DELETE fast path blocked on cursor save/restore. spend: ~2x the issue's original "small" estimate — the premise (fold an existing per-row bound-check into the seek) didn't hold once checked against real sqlite3 EXPLAIN; the actual gap (UPDATE/DELETE never seeking at all) took full-fix-sized work for one of the two statements. Co-Authored-By: Claude Sonnet 5 --- src/codegen/select.rs | 1 + src/codegen/select/range_scan.rs | 151 +++++++++++++++++++ src/codegen/stmt/update.rs | 232 +++++++++++++++++++++++++----- tests/unit/codegen_update_test.rs | 120 +++++++++++++++- 4 files changed, 465 insertions(+), 39 deletions(-) diff --git a/src/codegen/select.rs b/src/codegen/select.rs index 15a0fd42..34868ea2 100644 --- a/src/codegen/select.rs +++ b/src/codegen/select.rs @@ -209,6 +209,7 @@ pub(crate) use entry::{ }; pub(crate) use joins::compile_select_joined_scan; pub(crate) use limit_scan::{is_rowid_reference, top_level_equality_operands}; +pub(crate) use range_scan::try_compile_range_row_seek; #[cfg(test)] #[allow(clippy::unwrap_used, clippy::indexing_slicing)] diff --git a/src/codegen/select/range_scan.rs b/src/codegen/select/range_scan.rs index ca8ea2b0..8a6a3ede 100644 --- a/src/codegen/select/range_scan.rs +++ b/src/codegen/select/range_scan.rs @@ -406,6 +406,157 @@ where Ok(true) } +/// #666: `UPDATE`/`DELETE`'s row-scan equivalent of +/// [`try_compile_forward_comparison_seek`]/[`try_compile_between_seek`], +/// stripped of `SELECT`-only concerns (`DISTINCT`, LIMIT/OFFSET, +/// projection) — recognizes the same two WHERE shapes (`col >/>=/( + em: &mut Emitter, + reg: &mut RegAlloc, + where_expr: &Expr, + schema: &TableSchema, + scope: &Scope, + index_cursor: i32, + end_label: Label, + sink: &mut F, +) -> Result +where + F: FnMut(&mut Emitter, &mut RegAlloc, i32, Label) -> Result<(), CodegenError>, +{ + if let ExprKind::Between { + expr, + lo, + hi, + negated: false, + } = &where_expr.kind + { + let Some(col_name) = where_col(expr) else { + return Ok(false); + }; + if !is_supported_operand(lo) || !is_supported_operand(hi) { + return Ok(false); + } + let Some(index_position) = find_leading_index(schema, col_name) else { + return Ok(false); + }; + let Some(index) = schema.indexes.get(index_position) else { + return Ok(false); + }; + let affinity = column_affinity(schema, col_name); + if !operand_matches_column_affinity(lo, affinity) + || !operand_matches_column_affinity(hi, affinity) + { + return Ok(false); + } + let leading_collation = index + .columns + .first() + .map_or(Collation::Binary, |c| c.collation); + + open_index_cursor(em, index, index_cursor)?; + let lo_reg = compile_value(em, reg, scope, lo)?; + let hi_reg = compile_value(em, reg, scope, hi)?; + + let seek_addr = em.emit(Instruction::with_p4( + Opcode::SeekIndexGE, + index_cursor, + 0, + lo_reg, + P4::SeekKey(vec![leading_collation]), + )); + em.patch_p2(seek_addr, end_label); + + let loop_start = em.new_label(); + em.place(loop_start); + + let stop_addr = em.emit(Instruction::with_p4( + Opcode::IdxCompareGT, + index_cursor, + 0, + hi_reg, + P4::SeekKey(vec![leading_collation]), + )); + em.patch_p2(stop_addr, end_label); + + let row_skip = em.new_label(); + sink(em, reg, index_cursor, row_skip)?; + em.place(row_skip); + let next_addr = em.emit(Instruction::new(Opcode::IdxNext, index_cursor, 0, 0)); + em.patch_p2(next_addr, loop_start); + return Ok(true); + } + + let Some((col_name, operand, inclusive)) = as_forward_comparison(where_expr) else { + return Ok(false); + }; + if !is_supported_operand(operand) { + return Ok(false); + } + let Some(index_position) = find_leading_index(schema, col_name) else { + return Ok(false); + }; + let Some(index) = schema.indexes.get(index_position) else { + return Ok(false); + }; + let affinity = column_affinity(schema, col_name); + if !operand_matches_column_affinity(operand, affinity) { + return Ok(false); + } + let leading_collation = index + .columns + .first() + .map_or(Collation::Binary, |c| c.collation); + + open_index_cursor(em, index, index_cursor)?; + let bound_reg = compile_value(em, reg, scope, operand)?; + + let seek_addr = em.emit(Instruction::with_p4( + Opcode::SeekIndexGE, + index_cursor, + 0, + bound_reg, + P4::SeekKey(vec![leading_collation]), + )); + em.patch_p2(seek_addr, end_label); + + if !inclusive { + let skip_start = em.new_label(); + em.place(skip_start); + let past_bound = em.new_label(); + let gt_addr = em.emit(Instruction::with_p4( + Opcode::IdxCompareGT, + index_cursor, + 0, + bound_reg, + P4::SeekKey(vec![leading_collation]), + )); + em.patch_p2(gt_addr, past_bound); + let skip_next_addr = em.emit(Instruction::new(Opcode::IdxNext, index_cursor, 0, 0)); + em.patch_p2(skip_next_addr, skip_start); + let exhausted_addr = em.emit(Instruction::new(Opcode::Goto, 0, 0, 0)); + em.patch_p2(exhausted_addr, end_label); + em.place(past_bound); + } + + let loop_start = em.new_label(); + em.place(loop_start); + let row_skip = em.new_label(); + sink(em, reg, index_cursor, row_skip)?; + em.place(row_skip); + let next_addr = em.emit(Instruction::new(Opcode::IdxNext, index_cursor, 0, 0)); + em.patch_p2(next_addr, loop_start); + Ok(true) +} + /// The maximum Unicode scalar value, `char::MAX` (U+10FFFF) — see this /// module's doc comment for why appending it to a literal prefix gives a /// safe strict upper bound for `LIKE 'prefix%'`/`GLOB 'prefix*'`. diff --git a/src/codegen/stmt/update.rs b/src/codegen/stmt/update.rs index 6e9ab321..2fb3a3fc 100644 --- a/src/codegen/stmt/update.rs +++ b/src/codegen/stmt/update.rs @@ -42,12 +42,14 @@ use crate::codegen::expr::{column_index, compile_cond, compile_value, emit_colum use crate::codegen::index_maintenance::{ emit_index_key_ops, emit_index_key_ops_from_regs, open_index_cursors, valid_table_root_page, }; -use crate::codegen::select::{is_rowid_reference, top_level_equality_operands, CodegenError}; +use crate::codegen::select::{ + is_rowid_reference, top_level_equality_operands, try_compile_range_row_seek, CodegenError, +}; use crate::codegen::stmt::insert::{ - cached_create_table, column_plans, emit_constraint_violation, SQLITE_CONSTRAINT_CHECK, - SQLITE_CONSTRAINT_NOTNULL, + cached_create_table, column_plans, emit_constraint_violation, ColumnPlan, + SQLITE_CONSTRAINT_CHECK, SQLITE_CONSTRAINT_NOTNULL, }; -use crate::codegen::{CondTargets, Emitter, NullTarget, RegAlloc, Target}; +use crate::codegen::{CondTargets, Emitter, Label, NullTarget, RegAlloc, Scope, Target}; use crate::parser::ast::{ ConflictAction, Expr, ExprKind, Literal, ParamKind, TableConstraint, Update, }; @@ -162,9 +164,9 @@ pub fn compile_update_with_catalog( // #336: on a seek, `row_skip` and `end_label` are the same target — // there's exactly one candidate row, so "skip this row" (a // constraint violation under `OR IGNORE`) and "no more rows" both - // mean "we're done". On the ordinary scan, they differ as usual: - // `row_skip` continues the loop, `end_label` exits it. - let (loop_start, row_skip) = if let Some(operand) = rowid_seek_operand { + // mean "we're done". On the ordinary/range-seek scans, they differ + // as usual: `row_skip` continues the loop, `end_label` exits it. + if let Some(operand) = rowid_seek_operand { let value_reg = compile_value(&mut em, &mut reg, &scope, operand)?; let seek_addr = em.emit(Instruction::new( Opcode::SeekRowid, @@ -173,7 +175,132 @@ pub fn compile_update_with_catalog( value_reg, )); em.patch_p2(seek_addr, end_label); - (None, end_label) + emit_update_row_body( + &mut em, + &mut reg, + schema, + &scope, + &plans, + &table_checks, + &check_schema, + action, + rowid_alias, + &assigned, + end_label, + )?; + em.place(end_label); + em.emit(Instruction::new(Opcode::Halt, 0, 0, 0)); + return Ok(em.finish()); + } + + // #666: an index-seek range scan (`WHERE col >/>=/, + assigned: &[Option<&Expr>], + row_skip: Label, +) -> Result<(), CodegenError> { // Every value the new row needs — including a possibly-reassigned // rowid — is read from the cursor's *current* row before `Delete` // below clears it (`cursor::delete` sets `state.current = None`). let rowid_reg = match rowid_alias.and_then(|idx| assigned.get(idx).copied().flatten()) { - Some(expr) => compile_value(&mut em, &mut reg, &scope, expr)?, + Some(expr) => compile_value(em, reg, scope, expr)?, None => { let r = reg.alloc(); em.emit(Instruction::new(Opcode::Rowid, TABLE_CURSOR, r, 0)); @@ -214,10 +387,10 @@ pub fn compile_update_with_catalog( continue; } let r = match expr { - Some(expr) => compile_value(&mut em, &mut reg, &scope, expr)?, + Some(expr) => compile_value(em, reg, scope, expr)?, None => { let r = reg.alloc(); - emit_column_read(&mut em, schema, TABLE_CURSOR, idx, r)?; + emit_column_read(em, schema, TABLE_CURSOR, idx, r)?; r } }; @@ -242,7 +415,7 @@ pub fn compile_update_with_catalog( em.goto(ok); em.place(violation); emit_constraint_violation( - &mut em, + em, action, SQLITE_CONSTRAINT_NOTNULL, format!( @@ -286,9 +459,9 @@ pub fn compile_update_with_catalog( let violation = em.new_label(); let ok = em.new_label(); compile_cond( - &mut em, - &mut reg, - &crate::codegen::Scope::single(&check_schema, CHECK_CURSOR), + em, + reg, + &crate::codegen::Scope::single(check_schema, CHECK_CURSOR), expr, CondTargets { on_true: Target::Fallthrough, @@ -299,7 +472,7 @@ pub fn compile_update_with_catalog( em.goto(ok); em.place(violation); emit_constraint_violation( - &mut em, + em, action, SQLITE_CONSTRAINT_CHECK, format!("CHECK constraint failed: {}", schema.name), @@ -328,8 +501,8 @@ pub fn compile_update_with_catalog( // Old index entries are read from the cursor's still-current // (pre-`Delete`) row — must happen before `Delete` clears it. emit_index_key_ops( - &mut em, - &mut reg, + em, + reg, schema, TABLE_CURSOR, FIRST_INDEX_CURSOR, @@ -347,23 +520,8 @@ pub fn compile_update_with_catalog( // The new row's values are already sitting in `col_regs`/ // `rowid_reg` — build index keys from those directly instead of // seeking `TABLE_CURSOR` back onto the just-written row. - emit_index_key_ops_from_regs( - &mut em, - &mut reg, - schema, - &col_regs, - rowid_reg, - FIRST_INDEX_CURSOR, - )?; + emit_index_key_ops_from_regs(em, reg, schema, &col_regs, rowid_reg, FIRST_INDEX_CURSOR)?; } - if let Some(loop_start) = loop_start { - em.place(row_skip); - let next_addr = em.emit(Instruction::new(Opcode::Next, TABLE_CURSOR, 0, 0)); - em.patch_p2(next_addr, loop_start); - } - - em.place(end_label); - em.emit(Instruction::new(Opcode::Halt, 0, 0, 0)); - Ok(em.finish()) + Ok(()) } diff --git a/tests/unit/codegen_update_test.rs b/tests/unit/codegen_update_test.rs index 4aba8565..0b3f7aa6 100644 --- a/tests/unit/codegen_update_test.rs +++ b/tests/unit/codegen_update_test.rs @@ -15,6 +15,7 @@ //! `decode_record`, mirroring `tests/unit/codegen_insert_test.rs`'s harness. use std::path::{Path, PathBuf}; +use std::process::Command; use std::sync::atomic::{AtomicU64, Ordering}; use sqlite_rs::btree::TableCursor; @@ -23,9 +24,9 @@ use sqlite_rs::header::DatabaseHeader; use sqlite_rs::pager::Pager; use sqlite_rs::parser::{parse_insert, parse_update, ParseOutcome}; use sqlite_rs::record::{decode_record, TextEncoding, Value}; -use sqlite_rs::schema::TableSchema; +use sqlite_rs::schema::{read_schema, TableSchema}; use sqlite_rs::vdbe::{execute_with_writable_db, ExecError}; -use sqlite_rs::vfs::{UnixVfs, Vfs}; +use sqlite_rs::vfs::{UnixVfs, Vfs, VfsPageSource}; fn scratch_db(label: &str) -> PathBuf { static COUNTER: AtomicU64 = AtomicU64::new(0); @@ -458,3 +459,118 @@ fn check_violation_halts_and_leaves_the_row_unchanged() { vec![(1, vec![Value::Integer(5)])] ); } + +// --------------------------------------------------------------- +// #666: index-seek range scan (`WHERE col >/>=/ TableSchema { + let vfs = UnixVfs; + let file = vfs.open_read(db).unwrap(); + let mut header_buf = [0u8; 100]; + file.read_at(&mut header_buf, 0).unwrap(); + let header = DatabaseHeader::parse(&header_buf).unwrap(); + let source = VfsPageSource::open(&vfs, db, header.page_size).unwrap(); + let mut cursor = TableCursor::new(source, &header, 1); + let schemas = read_schema(&mut cursor, header.text_encoding).unwrap(); + schemas + .into_iter() + .find(|s| s.name == table) + .unwrap_or_else(|| panic!("no schema for table {table}")) +} + +fn range_seek_fixture(label: &str) -> (PathBuf, DatabaseHeader, u32, TableSchema) { + let db = std::env::temp_dir().join(format!( + "sqlite-rs-codegen-update-range-seek-{label}-{}.db", + std::process::id() + )); + std::fs::remove_file(&db).ok(); + seed_via_sqlite3( + &db, + "CREATE TABLE t(id INTEGER, val INTEGER); \ + CREATE INDEX idx_val ON t(val); \ + INSERT INTO t VALUES (1, 5), (2, 10), (3, 15), (4, 20), (5, 25);", + ); + let schema = indexed_table_schema(&db, "t"); + let vfs = UnixVfs; + let file = vfs.open_read(&db).unwrap(); + let mut header_buf = [0u8; 100]; + file.read_at(&mut header_buf, 0).unwrap(); + let header = DatabaseHeader::parse(&header_buf).unwrap(); + let page_size = header.page_size; + (db, header, page_size, schema) +} + +/// #666: `WHERE val > lit` against a leading-indexed column compiles to +/// an index seek (`SeekIndexGE`), not a full `Rewind`/`Next` scan. +#[test] +fn range_predicate_update_compiles_to_index_seek() { + let (_db, _header, _page_size, schema) = range_seek_fixture("compile-check"); + let update = match parse_update("UPDATE t SET id = id + 1 WHERE val > 15") { + ParseOutcome::Accepted(u) => *u, + other => panic!("failed to parse: {other:?}"), + }; + let program = compile_update(&update, &schema).unwrap(); + let rows = sqlite_rs::vdbe::explain(&program); + assert!( + rows.iter().any(|r| r.opcode == "SeekIndexGE"), + "expected SeekIndexGE in the compiled program: {rows:?}" + ); + assert!( + !rows.iter().any(|r| r.opcode == "Rewind" && r.p1 == 0), + "range-predicate update must not also emit a full scan of the table cursor: {rows:?}" + ); +} + +#[test] +fn range_predicate_update_touches_only_matching_rows() { + let (db, header, page_size, schema) = range_seek_fixture("exec-gt"); + run_update( + &db, + &header, + page_size, + "UPDATE t SET id = id + 100 WHERE val > 15", + &schema, + ) + .unwrap(); + + let got = rows(&db, &header, page_size, schema.root_page); + let mut ids: Vec = got + .into_iter() + .map(|(_, values)| match &values[0] { + Value::Integer(n) => *n, + other => panic!("expected INTEGER, got {other:?}"), + }) + .collect(); + ids.sort_unstable(); + assert_eq!(ids, vec![1, 2, 3, 104, 105]); +} + +#[test] +fn between_predicate_update_touches_only_matching_rows() { + let (db, header, page_size, schema) = range_seek_fixture("exec-between"); + run_update( + &db, + &header, + page_size, + "UPDATE t SET id = id + 100 WHERE val BETWEEN 10 AND 20", + &schema, + ) + .unwrap(); + + let got = rows(&db, &header, page_size, schema.root_page); + let mut ids: Vec = got + .into_iter() + .map(|(_, values)| match &values[0] { + Value::Integer(n) => *n, + other => panic!("expected INTEGER, got {other:?}"), + }) + .collect(); + ids.sort_unstable(); + assert_eq!(ids, vec![1, 5, 102, 103, 104]); +} From bb84ef3e3a18d4556308b788cc8e946df5b6359d Mon Sep 17 00:00:00 2001 From: Ilja Heitlager Date: Sun, 30 Aug 2026 14:25:34 +0200 Subject: [PATCH 2/2] docs: fold #666 changelog entry into untagged 0.18.9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0.18.9 isn't tagged yet, matching #663's own fold-back (e01fccf3) — no reason to cut 0.18.10 for this fix. --- CHANGELOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17268b44..b0a918b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,24 @@ All notable changes to sqlite-rs. Format follows [Keep a Changelog](https://keep *different*, already-on-disk row's stale entries) are unaffected — they have no such register run to reuse (#663). +- `UPDATE ... WHERE col >/>=/