From aaaa1f93c957a84e2fd3db62ab6bcc63f87bd34b Mon Sep 17 00:00:00 2001 From: Ilja Heitlager Date: Sun, 30 Aug 2026 12:53:20 +0200 Subject: [PATCH] fix: read indexed column from index cursor in range-seek fast paths (#664) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The BETWEEN/LIKE-prefix/IN/forward-comparison index-seek fast paths (src/codegen/select/range_scan.rs) all re-read the indexed column from the table row after IdxRowid+SeekRowid, even though that same column's value is already sitting in the index cursor's own key the seek just matched against — one redundant Column decode per row, on top of the table lookup still needed for the other selected columns. emit_matched_row now recognizes a plain bare-column select list (bare_column_names) and substitutes an index-cursor Column read (emit_indexed_column_read, position 0, with the same REAL-affinity fixup emit_column_read applies) for the one column that matches the seek's indexed column. Any other select-list shape (*, computed expressions) falls back to the unchanged emit_row_via_sink path. Verified: EXPLAIN opcode sequence for read_indexed_range (SELECT id, n, x, f, s FROM bench_data WHERE x > 50000) now matches oracle's shape exactly (Column reads x off the index cursor, not the table). Full test suite and oracle-parity corpus pass; spot-checked BETWEEN/IN/*/computed-expression queries byte-for-byte against oracle. Benchmark note: cargo bench --bench crud -- read_indexed_range shows no measurable change at the 1mb fixture (within noise, no 50mb variant exists for this scenario) — the eliminated Column read is a small fraction of per-row cost next to the btree seek/page touch and the other four column reads. Landing as a correctness/opcode-parity fix, not a benchmarked performance win. spend: ~1x estimate (small). Refs #664 Co-Authored-By: Claude Sonnet 5 --- src/codegen/select/range_scan.rs | 95 +++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/src/codegen/select/range_scan.rs b/src/codegen/select/range_scan.rs index 159ec73..ca8ea2b 100644 --- a/src/codegen/select/range_scan.rs +++ b/src/codegen/select/range_scan.rs @@ -161,11 +161,79 @@ fn open_index_cursor( Ok(()) } +/// Every select-list column, when it's a bare unqualified column +/// reference (`ResultColumn::Expr` wrapping `ExprKind::Column{table: +/// None, catalog: None, ..}`) — the shape every fast path in this file +/// is profiled against (`SELECT id, n, x, ... FROM t WHERE ...`), and +/// the only shape [`emit_matched_row`]'s indexed-column substitution +/// (#664) knows how to recognize. `None` for `*`/`tbl.*`, a qualified +/// reference, or any computed expression — [`emit_matched_row`] falls +/// back to the ordinary full-row read via [`emit_row_via_sink`] for +/// those, exactly as before this optimization existed. +fn bare_column_names(select: &Select) -> Option> { + select + .columns + .iter() + .map(|col| match col { + ResultColumn::Expr { + expr: + Expr { + kind: + ExprKind::Column { + table: None, + catalog: None, + name, + }, + .. + }, + .. + } => Some(name.as_str()), + _ => None, + }) + .collect() +} + +/// Reads `idx`'s value from `index_cursor`'s own leading key column +/// (position 0) into `dest`, instead of [`emit_column_read`]'s ordinary +/// read off the table cursor (#664) — every fast path in this file +/// positions its index cursor on an entry whose key already carries the +/// one column the seek matched against, so re-reading that same column +/// a moment later off the table row `IdxRowid`+`SeekRowid` just fetched +/// is a wholly redundant round trip (the table lookup itself still +/// stands: every *other* selected column still only lives in the table +/// row). Mirrors `emit_column_read`'s REAL-affinity fixup (#143) since +/// this is still logically that same schema column, just sourced from a +/// different cursor; the rowid-alias case never arises here — a +/// rowid-alias column is never itself indexed as an ordinary index +/// column, so [`emit_matched_row`] never requests it. +fn emit_indexed_column_read( + em: &mut Emitter, + schema: &TableSchema, + index_cursor: i32, + idx: usize, + dest: i32, +) { + em.emit(Instruction::new(Opcode::Column, index_cursor, 0, dest)); + if schema + .column_types + .get(idx) + .is_some_and(|t| affinity_of(t) == Affinity::Real) + { + em.emit(Instruction::new(Opcode::RealAffinity, dest, 0, 0)); + } +} + /// Emits the shared "fetch the full row and hand it to `sink`" tail /// every fast path in this file uses once the index cursor is /// positioned on a matching entry: `IdxRowid` + `SeekRowid` (jumping to -/// `row_skip` if the table row is somehow missing) + LIMIT/OFFSET guards -/// + [`emit_row_via_sink`]. +/// `row_skip` if the table row is somehow missing), then LIMIT/OFFSET +/// guards, then the row projection. `indexed_col_name` is the column +/// this fast path's seek matched against (already sitting in +/// `index_cursor`'s key) — when every select-list column is a bare +/// reference ([`bare_column_names`]), that one column is read straight +/// from the index cursor instead of the table row (#664); any other +/// select-list shape falls back to [`emit_row_via_sink`]'s ordinary +/// full-row read, unchanged from before this optimization existed. #[allow(clippy::too_many_arguments)] fn emit_matched_row( em: &mut Emitter, @@ -174,6 +242,7 @@ fn emit_matched_row( schema: &TableSchema, cursors: ScanCursors, index_cursor: i32, + indexed_col_name: &str, limit: &Option, row_skip: Label, end_label: Label, @@ -204,6 +273,24 @@ where if let Some(limit) = limit { emit_limit_guard(em, limit, end_label); } + + if let Some(names) = bare_column_names(select) { + let mut first = None; + for name in &names { + let idx = column_index(schema, name).ok_or_else(|| CodegenError::UnknownColumn { + name: (*name).to_string(), + })?; + let r = reg.alloc(); + if name.eq_ignore_ascii_case(indexed_col_name) && schema.rowid_alias != Some(idx) { + emit_indexed_column_read(em, schema, index_cursor, idx, r); + } else { + emit_column_read(em, schema, cursors.table, idx, r)?; + } + first.get_or_insert(r); + } + let first = first.unwrap_or_else(|| reg.alloc()); + return sink(em, reg, first, i32::try_from(names.len()).unwrap_or(0)); + } emit_row_via_sink(em, reg, select, schema, cursors.table, false, catalog, sink) } @@ -305,6 +392,7 @@ where schema, cursors, index_cursor, + col_name, &limit, row_skip, end_label, @@ -473,6 +561,7 @@ where schema, cursors, index_cursor, + col_name, &limit, row_skip, end_label, @@ -597,6 +686,7 @@ where schema, cursors, index_cursor, + col_name, &limit, row_skip, end_label, @@ -738,6 +828,7 @@ where schema, cursors, index_cursor, + col_name, &limit, row_skip, end_label,