diff --git a/src/codegen/select.rs b/src/codegen/select.rs index 34868ea..70279ca 100644 --- a/src/codegen/select.rs +++ b/src/codegen/select.rs @@ -209,7 +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; +pub(crate) use range_scan::{range_seek_index_position, 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 8a6a3ed..f33e832 100644 --- a/src/codegen/select/range_scan.rs +++ b/src/codegen/select/range_scan.rs @@ -992,6 +992,87 @@ where Ok(true) } +/// The `schema.indexes` position of the index a range-seek fast path in +/// this file would pick for `where_expr`, or `None` if `where_expr` +/// doesn't match any of the recognized shapes (`BETWEEN`, `LIKE`/`GLOB` +/// prefix, `IN`, or a forward comparison) — the same +/// shape-recognition/affinity checks `try_compile_range_row_seek` itself +/// applies, factored out so a caller can inspect *which* index would be +/// scanned without actually emitting anything (`update.rs`'s #675 fix +/// uses this to decide whether the `SET` clause touches that index and a +/// two-pass ephemeral-rowid plan is still required). +pub(crate) fn range_seek_index_position(where_expr: &Expr, schema: &TableSchema) -> Option { + match &where_expr.kind { + ExprKind::Between { + expr, + lo, + hi, + negated: false, + } => { + let col_name = where_col(expr)?; + if !is_supported_operand(lo) || !is_supported_operand(hi) { + return None; + } + let index_position = find_leading_index(schema, col_name)?; + let affinity = column_affinity(schema, col_name); + if !operand_matches_column_affinity(lo, affinity) + || !operand_matches_column_affinity(hi, affinity) + { + return None; + } + Some(index_position) + } + ExprKind::Like { + expr, + pattern, + glob, + negated: false, + escape: None, + } => { + let col_name = where_col(expr)?; + let ExprKind::Literal(Literal::Str(pattern_str)) = &pattern.kind else { + return None; + }; + like_literal_prefix(pattern_str, *glob)?; + let index_position = find_leading_index(schema, col_name)?; + if column_affinity(schema, col_name) != Affinity::Text { + return None; + } + Some(index_position) + } + ExprKind::In { + expr, + list, + negated: false, + } => { + if list.is_empty() || !list.iter().all(is_supported_operand) { + return None; + } + let col_name = where_col(expr)?; + let index_position = find_leading_index(schema, col_name)?; + let affinity = column_affinity(schema, col_name); + if !list + .iter() + .all(|v| operand_matches_column_affinity(v, affinity)) + { + return None; + } + Some(index_position) + } + _ => as_forward_comparison(where_expr).and_then(|(col_name, operand, _inclusive)| { + if !is_supported_operand(operand) { + return None; + } + let index_position = find_leading_index(schema, col_name)?; + let affinity = column_affinity(schema, col_name); + if !operand_matches_column_affinity(operand, affinity) { + return None; + } + Some(index_position) + }), + } +} + /// `EXPLAIN QUERY PLAN` reporting for this file's fast paths (#606's /// acceptance criteria: `EXPLAIN QUERY PLAN` must show index usage for /// these query shapes) — reuses the exact same shape-recognition diff --git a/src/codegen/stmt/update.rs b/src/codegen/stmt/update.rs index 2fb3a3f..6785f02 100644 --- a/src/codegen/stmt/update.rs +++ b/src/codegen/stmt/update.rs @@ -43,7 +43,8 @@ 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, try_compile_range_row_seek, CodegenError, + is_rowid_reference, range_seek_index_position, top_level_equality_operands, + try_compile_range_row_seek, CodegenError, }; use crate::codegen::stmt::insert::{ cached_create_table, column_plans, emit_constraint_violation, ColumnPlan, @@ -193,81 +194,154 @@ pub fn compile_update_with_catalog( return Ok(em.finish()); } - // #666: an index-seek range scan (`WHERE col >/>=//>=/ lit` +/// scans), so the compiled plan must skip the ephemeral-rowid +/// materialization entirely and apply the update directly in the index +/// walk. +#[test] +fn range_predicate_update_without_indexed_set_uses_single_pass() { + let (_db, _header, _page_size, schema) = range_seek_fixture("single-pass-compile"); + 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 == "OpenEphemeral"), + "SET column doesn't intersect the scanned index — the two-pass \ + ephemeral-rowid plan should be skipped: {rows:?}" + ); +} + +/// `SET val = ...` *does* touch `idx_val`, the very index `WHERE val > +/// lit` scans — the compiled plan must keep #666's two-pass ephemeral +/// plan (correctness over speed here), and the update must still land +/// correctly on every originally-matching row despite the self-mutating +/// index walk. +#[test] +fn range_predicate_update_on_indexed_column_keeps_two_pass_plan() { + let (db, header, page_size, schema) = range_seek_fixture("two-pass-compile"); + let update = match parse_update("UPDATE t SET val = val + 1 WHERE val > 15") { + ParseOutcome::Accepted(u) => *u, + other => panic!("failed to parse: {other:?}"), + }; + let program = compile_update(&update, &schema).unwrap(); + let rows_eqp = sqlite_rs::vdbe::explain(&program); + assert!( + rows_eqp.iter().any(|r| r.opcode == "OpenEphemeral"), + "SET column intersects the scanned index — the two-pass \ + ephemeral-rowid plan must still be used: {rows_eqp:?}" + ); + + run_update( + &db, + &header, + page_size, + "UPDATE t SET val = val + 1 WHERE val > 15", + &schema, + ) + .unwrap(); + + let got = rows(&db, &header, page_size, schema.root_page); + let mut vals: Vec = got + .into_iter() + .map(|(_, values)| match &values[1] { + Value::Integer(n) => *n, + other => panic!("expected INTEGER, got {other:?}"), + }) + .collect(); + vals.sort_unstable(); + // Originally 5, 10, 15, 20, 25 -- only val > 15 (20, 25) should be + // touched, becoming 21, 26. A self-mutating single-pass walk would + // either skip, re-visit, or double-increment these once the index + // b-tree is rewritten mid-scan. + assert_eq!(vals, vec![5, 10, 15, 21, 26]); +}