diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a35d73e..5bc3750b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,35 @@ 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.)* +## [Unreleased] + +### Fixed + +- `rowid`/`_rowid_`/`oid` resolved in a `WHERE` clause but not in a + result-list projection (`Scope::resolve` had no pseudo-column + awareness, unlike the `UPDATE`/`DELETE` seek path's + `is_rowid_reference`). Now resolves as a pseudo-column on rowid + tables — a declared column of that name still shadows it, `INTEGER + PRIMARY KEY` remains the existing alias case, and `WITHOUT ROWID` + tables reject it with the same unknown-column error the oracle gives. + Also fixes the same reference combined with `ORDER BY` and an alias + in one statement, where the post-sort pseudo cursor previously tried + to re-issue `Rowid` against itself. `GROUP BY rowid` (and `_rowid_`/ + `oid`) had the identical pseudo-cursor problem in + `compile_grouped_scan`'s sort-then-group pass 2 — both the group-key + comparison and a bare `rowid` in the result list or `HAVING` now read + back a materialized field instead of re-issuing `Rowid` against a + cursor that can't answer it (#708). Two silent wrong answers in the + same family are fixed alongside it: an aggregate with no `GROUP BY` + (`SELECT rowid, count(*) FROM t`) took a fast path whose synthetic + per-group record has no slot for the pseudo-column and so projected an + empty value instead of the rowid — that path now defers to + `compile_grouped_scan`; and a table with a *declared* column named + `rowid` projected the hidden rowid instead of the column's own value + once an `ORDER BY` put the read behind a pseudo cursor, because the + projection fell through to the pseudo-column sentinel even though a + declared column of that name shadows it (#708). + ## [0.18.10] - 2026-08-31 ### Fixed diff --git a/Cargo.toml b/Cargo.toml index 99457437..edda6891 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -140,6 +140,10 @@ path = "tests/unit/codegen_expr_test.rs" name = "codegen_select" path = "tests/unit/codegen_select_test.rs" +[[test]] +name = "rowid_projection" +path = "tests/unit/rowid_projection_test.rs" + [[test]] name = "codegen_insert" path = "tests/unit/codegen_insert_test.rs" diff --git a/src/codegen.rs b/src/codegen.rs index 177185c8..e4807e48 100644 --- a/src/codegen.rs +++ b/src/codegen.rs @@ -561,7 +561,9 @@ impl Scope { .ok_or_else(|| select::CodegenError::UnknownColumn { name: name.to_string(), })?; - let idx = expr::column_index(&binding.schema, name).unwrap_or(0); + let idx = expr::column_index(&binding.schema, name) + .or_else(|| expr::rowid_pseudo_column_index(&binding.schema, name)) + .unwrap_or(0); Ok((binding.cursor, idx, &binding.schema, binding.forced_null)) } @@ -593,16 +595,18 @@ impl Scope { .ok_or_else(|| select::CodegenError::UnknownColumn { name: format!("{table}.{name}"), })?; - expr::column_index(&binding.schema, name).ok_or_else(|| { - select::CodegenError::UnknownColumn { + expr::column_index(&binding.schema, name) + .or_else(|| expr::rowid_pseudo_column_index(&binding.schema, name)) + .ok_or_else(|| select::CodegenError::UnknownColumn { name: format!("{table}.{name}"), - } - })?; + })?; return Ok(idx); } let mut found: Option = None; for (i, binding) in self.tables.iter().enumerate() { - if expr::column_index(&binding.schema, name).is_some() { + if expr::column_index(&binding.schema, name).is_some() + || expr::rowid_pseudo_column_index(&binding.schema, name).is_some() + { if found.is_some() { return Err(select::CodegenError::AmbiguousColumn { name: name.to_string(), diff --git a/src/codegen/expr.rs b/src/codegen/expr.rs index 14e95ee1..35541b34 100644 --- a/src/codegen/expr.rs +++ b/src/codegen/expr.rs @@ -24,7 +24,7 @@ mod cond; mod value; -pub(crate) use cond::{column_index, compile_cond, ensure_label}; +pub(crate) use cond::{column_index, compile_cond, ensure_label, rowid_pseudo_column_index}; pub(crate) use value::{ collation_of, compile_value, emit_column_read, expr_affinity, expr_collation, is_aggregate_call, }; diff --git a/src/codegen/expr/cond.rs b/src/codegen/expr/cond.rs index a439442c..510bc17e 100644 --- a/src/codegen/expr/cond.rs +++ b/src/codegen/expr/cond.rs @@ -23,6 +23,28 @@ pub(crate) fn column_index(schema: &TableSchema, name: &str) -> Option { .position(|c| c.eq_ignore_ascii_case(name)) } +/// #708: `rowid`/`_rowid_`/`oid` as a pseudo-column on a rowid table — +/// resolves to a sentinel index one past `schema.columns`'s end, which +/// [`super::value::emit_column_read`] recognizes and reads via +/// `Opcode::Rowid` off the live cursor, same as a genuine +/// `rowid_alias` hit. Only consulted when [`column_index`] has already +/// failed, so a declared column of that name always wins (shadowing). +/// `WITHOUT ROWID` tables have no rowid at all, so this always misses +/// for them, and callers fall through to the ordinary "unknown column" +/// error — matching the oracle. +pub(crate) fn rowid_pseudo_column_index(schema: &TableSchema, name: &str) -> Option { + if schema.without_rowid { + return None; + } + if name.eq_ignore_ascii_case("rowid") + || name.eq_ignore_ascii_case("_rowid_") + || name.eq_ignore_ascii_case("oid") + { + return Some(schema.columns.len()); + } + None +} + /// #581: a rough, static (no `ANALYZE` data needed) cost class for an /// expression, used only to order `AND`/`OR` operands cheapest-first so /// short-circuit evaluation skips the pricier side more often — never diff --git a/src/codegen/expr/value.rs b/src/codegen/expr/value.rs index 75a779a5..f2e333bf 100644 --- a/src/codegen/expr/value.rs +++ b/src/codegen/expr/value.rs @@ -27,7 +27,11 @@ pub(crate) fn emit_column_read( idx: usize, dest: i32, ) -> Result<(), CodegenError> { - if schema.rowid_alias == Some(idx) { + if schema.rowid_alias == Some(idx) || idx == schema.columns.len() { + // `idx == schema.columns.len()` is #708's `rowid`/`_rowid_`/`oid` + // pseudo-column sentinel (see `expr::rowid_pseudo_column_index`) + // — one past the last declared column, so it never collides + // with a real index. em.emit(Instruction::new(Opcode::Rowid, cursor, dest, 0)); return Ok(()); } diff --git a/src/codegen/select.rs b/src/codegen/select.rs index 70279ca5..ebb38b04 100644 --- a/src/codegen/select.rs +++ b/src/codegen/select.rs @@ -17,7 +17,7 @@ use crate::codegen::expr::{ collation_of, column_index, compile_cond, compile_value, emit_column_read, expr_affinity, - expr_collation, is_aggregate_call, + expr_collation, is_aggregate_call, rowid_pseudo_column_index, }; use crate::codegen::{ p4_coll_seq, CondTargets, Emitter, Label, RegAlloc, Scope, TableBinding, Target, diff --git a/src/codegen/select/aggregate.rs b/src/codegen/select/aggregate.rs index 605b3690..298b6df8 100644 --- a/src/codegen/select/aggregate.rs +++ b/src/codegen/select/aggregate.rs @@ -321,6 +321,18 @@ where if aggs.iter().any(|(_, _, _, distinct)| *distinct) { return Ok(false); } + // #708 follow-up: this fast path's synthetic per-group record holds + // one field per *declared* schema column and nothing else, so it has + // no slot for the bare `rowid`/`_rowid_`/`oid` pseudo-column — yet + // the projection still asks for `rowid_pseudo_column_index`'s + // sentinel (one past the last column), which reads off the end of + // the record and projects an empty value instead of the rowid. + // Decline and let `compile_grouped_scan`'s implicit-whole-table-group + // path handle it: it materializes the field (see + // `grouped_scan_needs_rowid_field`) and reads it back with `Column`. + if grouped_scan_needs_rowid_field(select, schema) { + return Ok(false); + } let table_scope = Scope::single(schema, cursors.table).with_catalog(catalog.to_vec()); // #322: hoist any uncorrelated WHERE-clause subquery once, up @@ -431,6 +443,7 @@ where schema, catalog, &snapshot_regs, + None, &agg_slots, limit.as_ref(), end_label, @@ -682,6 +695,72 @@ fn walk_expr_for_column_refs( } } +/// #708 follow-up: is `expr` exactly a bare, unshadowed +/// `rowid`/`_rowid_`/`oid` reference — the narrow shape +/// [`order_by_target_for_expr`] already routes to `OrderByTarget::Expr` +/// since it has no real schema column index of its own. `schema` is the +/// original table schema (not [`compact_schema`]'s narrowed one), so a +/// declared column named `rowid` correctly wins (shadowing) via +/// `column_index` before `rowid_pseudo_column_index` is even consulted. +fn is_bare_rowid_pseudo_column(expr: &Expr, schema: &TableSchema) -> bool { + matches!( + &expr.kind, + ExprKind::Column { + name, + table: None, + catalog: None, + } if crate::codegen::expr::column_index(schema, name).is_none() + && crate::codegen::expr::rowid_pseudo_column_index(schema, name).is_some() + ) +} + +/// Same bare-reference check as [`is_bare_rowid_pseudo_column`], but +/// walking through `Paren`/`Collate`/`Unary`/`Binary` wrappers (matching +/// `accum::substitute_rowid_pseudo_column`'s own traversal) to answer +/// whether `expr` references the pseudo-column *anywhere*, e.g. `HAVING +/// rowid > 1`'s `rowid` is nested one level inside a `Binary`. +fn expr_references_rowid_pseudo_column(expr: &Expr, schema: &TableSchema) -> bool { + if is_bare_rowid_pseudo_column(expr, schema) { + return true; + } + match &expr.kind { + ExprKind::Paren(inner) + | ExprKind::Collate { expr: inner, .. } + | ExprKind::Unary { expr: inner, .. } => expr_references_rowid_pseudo_column(inner, schema), + ExprKind::Binary { lhs, rhs, .. } => { + expr_references_rowid_pseudo_column(lhs, schema) + || expr_references_rowid_pseudo_column(rhs, schema) + } + _ => false, + } +} + +/// #708 follow-up: does [`compile_grouped_scan`]'s pass 1 need to +/// materialize the bare rowid pseudo-column into the sort record? Needed +/// whenever `GROUP BY`, a result column, or `HAVING` references it — +/// otherwise there is no field for it anywhere in the compacted record +/// (unlike a real `schema` column, `needed_order` never contains it). +/// `WITHOUT ROWID` never needs it: there is no rowid to materialize, and +/// [`crate::codegen::expr::rowid_pseudo_column_index`] already reports +/// unknown-column for it, same as main. +fn grouped_scan_needs_rowid_field(select: &Select, schema: &TableSchema) -> bool { + if schema.without_rowid { + return false; + } + select + .group_by + .iter() + .any(|e| expr_references_rowid_pseudo_column(e, schema)) + || select.columns.iter().any(|col| match col { + ResultColumn::Expr { expr, .. } => expr_references_rowid_pseudo_column(expr, schema), + ResultColumn::Star | ResultColumn::TableStar { .. } => false, + }) + || select + .having + .as_ref() + .is_some_and(|h| expr_references_rowid_pseudo_column(h, schema)) +} + /// #239: `GROUP BY` / `HAVING`. Strategy mirrors real SQLite's /// sort-then-group `select.c` shape rather than a hash table, since the /// `Sorter*` opcode family this compiler already has for `ORDER BY` @@ -778,6 +857,11 @@ where if let Some(outer) = outer_scope { pseudo_scope = pseudo_scope.with_outer(outer.clone()); } + // #708 follow-up: whether a bare rowid/`_rowid_`/`oid` pseudo-column + // reference anywhere in `GROUP BY`/`select.columns`/`HAVING` needs a + // materialized field in the sort record — see + // `grouped_scan_needs_rowid_field`'s doc for why. + let needs_rowid_field = grouped_scan_needs_rowid_field(select, schema); // Pass 1: buffer every WHERE-matching row's needed column values // (#665: only the columns `needed_order` names, not the full row), @@ -810,6 +894,21 @@ where } let first = compile_row_values_compact(em, reg, schema, &needed_order, cursors.table)?; + // #708 follow-up: materialize the bare rowid pseudo-column right + // after the compacted schema-column block — `cursors.table` is a + // real table cursor here, so `Opcode::Rowid` is valid (unlike pass + // 2's `cursors.pseudo`, which can't answer it at all). Its field + // position (`needed_order.len()`, i.e. right after `first`) is + // reused below by both a rowid `GROUP BY` key and, in pass 2, by the + // group's snapshot/key re-reads — the same materialize-once, + // read-back-with-`Column` shape `compile_sorted_scan` established + // for `ORDER BY rowid` (#718). + let rowid_field_index = needs_rowid_field.then(|| { + let r = reg.alloc(); + em.emit(Instruction::new(Opcode::Rowid, cursors.table, r, 0)); + usize::try_from(r.saturating_sub(first)).unwrap_or(0) + }); + let mut sort_keys = Vec::with_capacity(group_targets.len()); for (expr, target) in select.group_by.iter().zip(&group_targets) { let index = match target { @@ -817,6 +916,12 @@ where // `select.group_by` references (`columns_needed_for_projection`), // so `idx` always has a compacted position. OrderByTarget::Column(idx) => compact_of.get(*idx).copied().flatten().unwrap_or(0), + OrderByTarget::Expr(e) if is_bare_rowid_pseudo_column(e, schema) => { + // Already materialized above — reuse it rather than + // recompiling (which would emit a second, redundant + // `Rowid` read against the same real cursor). + rowid_field_index.unwrap_or(0) + } OrderByTarget::Expr(e) => { let r = compile_value(em, reg, &table_scope, e)?; usize::try_from(r.saturating_sub(first)).unwrap_or(0) @@ -892,6 +997,15 @@ where for &r in &snapshot_regs { em.emit(Instruction::new(Opcode::Null, 0, r, 0)); } + // #708 follow-up: the bare rowid pseudo-column's own persistent + // "arbitrary row" snapshot register, alongside `snapshot_regs` — + // kept separate rather than folded into that array since it has no + // corresponding `schema.columns`/`compact_of` entry to zip against. + let rowid_snapshot_reg = rowid_field_index.map(|_| { + let r = reg.alloc(); + em.emit(Instruction::new(Opcode::Null, 0, r, 0)); + r + }); // Aggregate-context slots (`Vm::agg_contexts`) are a disjoint table // from the register file, addressed by their own small integer // space — a bare 0-based counter here, not `reg.alloc()`. @@ -945,6 +1059,26 @@ where read_pseudo_column(em, &pseudo_schema, cursors.pseudo, compact_idx, r)?; Ok(r) } + OrderByTarget::Expr(_) if is_bare_rowid_pseudo_column(expr, schema) => { + // #708 follow-up: `compile_value(&pseudo_scope, ...)` + // would resolve this against the pass-2 pseudo cursor, + // which can't answer `Opcode::Rowid` at all (that's the + // regression this ticket fixes) — read the field pass 1 + // already materialized instead, same as + // `read_pseudo_column` does for `OrderByTarget::Column` + // just above. + let r = reg.alloc(); + let idx = rowid_field_index.unwrap_or(0); + em.emit(Instruction::new( + Opcode::Column, + cursors.pseudo, + i32::try_from(idx).map_err(|_| CodegenError::Unsupported { + reason: format!("column index {idx} does not fit in a P2 operand"), + })?, + r, + )); + Ok(r) + } OrderByTarget::Expr(_) => compile_value(em, reg, &pseudo_scope, expr), }) .collect::>()?; @@ -995,6 +1129,7 @@ where schema, catalog, &snapshot_regs, + rowid_snapshot_reg, &agg_slots, limit.as_ref(), end_label, @@ -1034,6 +1169,20 @@ where read_pseudo_column(em, &pseudo_schema, cursors.pseudo, compact_idx, dest)?; } } + // #708 follow-up: the bare rowid pseudo-column's own "arbitrary row" + // snapshot, same shape as `snapshot_regs`'s loop just above — read + // straight off pass 1's materialized field (`rowid_field_index`), + // never recomputed via `Opcode::Rowid` against `cursors.pseudo`. + if let (Some(idx), Some(dest)) = (rowid_field_index, rowid_snapshot_reg) { + em.emit(Instruction::new( + Opcode::Column, + cursors.pseudo, + i32::try_from(idx).map_err(|_| CodegenError::Unsupported { + reason: format!("column index {idx} does not fit in a P2 operand"), + })?, + dest, + )); + } let after_accumulate = em.new_label(); let goto_after_accumulate = em.emit(Instruction::new(Opcode::Goto, 0, 0, 0)); em.patch_p2(goto_after_accumulate, after_accumulate); @@ -1070,6 +1219,7 @@ where schema, catalog, &snapshot_regs, + rowid_snapshot_reg, &agg_slots, limit.as_ref(), end_label, @@ -1331,6 +1481,7 @@ where schema, catalog, &snapshot_regs, + None, &agg_slots, limit.as_ref(), end_label, @@ -1374,6 +1525,7 @@ where schema, catalog, &snapshot_regs, + None, &agg_slots, limit.as_ref(), end_label, diff --git a/src/codegen/select/aggregate/accum.rs b/src/codegen/select/aggregate/accum.rs index 9fbea571..b07c136b 100644 --- a/src/codegen/select/aggregate/accum.rs +++ b/src/codegen/select/aggregate/accum.rs @@ -202,6 +202,65 @@ pub(in crate::codegen::select) fn substitute_aggregates( } } +/// #708 follow-up (GROUP BY rowid): rewrites a bare, unshadowed +/// `rowid`/`_rowid_`/`oid` reference into a `Column` reference to +/// [`flush_group`]'s synthetic `__rowid` field — the same idea as +/// [`substitute_aggregates`]'s aggregate-call rewrite, just for the +/// pseudo-column [`compile_grouped_scan`] materialized into the sort +/// record instead. `schema` is the *original* table schema (not the +/// synthetic one), so `column_index` sees any real declared `rowid` +/// column and correctly leaves it alone (shadowing). +pub(in crate::codegen::select) fn substitute_rowid_pseudo_column( + expr: &Expr, + schema: &TableSchema, +) -> Expr { + if let ExprKind::Column { + name, + table: None, + catalog: None, + } = &expr.kind + { + if crate::codegen::expr::column_index(schema, name).is_none() + && crate::codegen::expr::rowid_pseudo_column_index(schema, name).is_some() + { + return Expr { + kind: ExprKind::Column { + table: None, + catalog: None, + name: "__rowid".to_string(), + }, + span: expr.span, + }; + } + } + let kind = match &expr.kind { + ExprKind::Paren(inner) => { + ExprKind::Paren(Box::new(substitute_rowid_pseudo_column(inner, schema))) + } + ExprKind::Collate { + expr: inner, + collation, + } => ExprKind::Collate { + expr: Box::new(substitute_rowid_pseudo_column(inner, schema)), + collation: collation.clone(), + }, + ExprKind::Unary { op, expr: inner } => ExprKind::Unary { + op: *op, + expr: Box::new(substitute_rowid_pseudo_column(inner, schema)), + }, + ExprKind::Binary { op, lhs, rhs } => ExprKind::Binary { + op: *op, + lhs: Box::new(substitute_rowid_pseudo_column(lhs, schema)), + rhs: Box::new(substitute_rowid_pseudo_column(rhs, schema)), + }, + other => other.clone(), + }; + Expr { + kind, + span: expr.span, + } +} + /// Pseudo-cursor-safe single-column read: like `emit_column_read`, but /// aware that `cursor` re-reads an already-materialized record (so the /// rowid-alias column is an ordinary field within it, not something @@ -339,10 +398,13 @@ pub(in crate::codegen::select) fn emit_agg_step( /// Finalizes and emits one grouped output row via `sink`, applying /// `HAVING`/`LIMIT`/`OFFSET` exactly as the ungrouped scans do. Builds a /// synthetic record — the group's snapshot column values (from the last -/// row seen) followed by each aggregate's finalized value — and opens a -/// fresh pseudo cursor over it, so `select.columns`/`having` (with -/// aggregate calls rewritten to reference the synthetic record's -/// trailing fields via [`substitute_aggregates`]) compile through the +/// row seen), the group's bare rowid pseudo-column value (#708 follow-up, +/// when `rowid_reg` is given), then each aggregate's finalized value — +/// and opens a fresh pseudo cursor over it, so `select.columns`/`having` +/// (with aggregate calls rewritten to reference the synthetic record's +/// trailing fields via [`substitute_aggregates`], and a bare +/// `rowid`/`_rowid_`/`oid` reference rewritten to the synthetic `__rowid` +/// field via [`substitute_rowid_pseudo_column`]) compile through the /// ordinary `compile_row_values`/`compile_cond` machinery unchanged. #[allow(clippy::too_many_arguments)] pub(in crate::codegen::select) fn flush_group( @@ -352,6 +414,7 @@ pub(in crate::codegen::select) fn flush_group( schema: &TableSchema, catalog: &[TableSchema], snapshot_regs: &[i32], + rowid_reg: Option, agg_slots: &[AggSlot], limit: Option<&LimitState>, end_label: Label, @@ -363,8 +426,14 @@ where let synthetic_names: Vec = (0..agg_slots.len()).map(|i| format!("__agg{i}")).collect(); let mut synthetic_columns = schema.columns.clone(); + if rowid_reg.is_some() { + synthetic_columns.push("__rowid".to_string()); + } synthetic_columns.extend(synthetic_names.iter().cloned()); let mut synthetic_types = schema.column_types.clone(); + if rowid_reg.is_some() { + synthetic_types.push(String::new()); + } synthetic_types.extend(synthetic_names.iter().map(|_| String::new())); let synthetic_schema = TableSchema { unresolved_autoindex: false, @@ -381,17 +450,28 @@ where rowid_alias: None, }; - // Allocate one fresh, contiguous register per snapshot/aggregate + // Allocate one fresh, contiguous register per snapshot/rowid/aggregate // field up front — `reg.alloc()` bump-allocates sequentially, so as // long as nothing else allocates in between, `dests` is guaranteed // contiguous for `MakeRecord`. - let synthetic_count = snapshot_regs.len().saturating_add(agg_slots.len()); + let rowid_width = usize::from(rowid_reg.is_some()); + let synthetic_count = snapshot_regs + .len() + .saturating_add(rowid_width) + .saturating_add(agg_slots.len()); let dests: Vec = (0..synthetic_count).map(|_| reg.alloc()).collect(); let synthetic_first = dests.first().copied().unwrap_or_else(|| reg.alloc()); for (&snap, &dest) in snapshot_regs.iter().zip(&dests) { em.emit(Instruction::new(Opcode::Copy, snap, dest, 0)); } - let agg_dests = dests.get(snapshot_regs.len()..).unwrap_or(&[]); + if let Some(rreg) = rowid_reg { + if let Some(&dest) = dests.get(snapshot_regs.len()) { + em.emit(Instruction::new(Opcode::Copy, rreg, dest, 0)); + } + } + let agg_dests = dests + .get(snapshot_regs.len().saturating_add(rowid_width)..) + .unwrap_or(&[]); for (agg, &dest) in agg_slots.iter().zip(agg_dests) { // `avg()`'s sum/count division now happens inside // `crate::vdbe::aggregate::finalize` — `AggFinal` just reads @@ -423,7 +503,12 @@ where let flush_scope = Scope::single(&synthetic_schema, flush_cursor).with_catalog(catalog.to_vec()); let skip_label = em.new_label(); if let Some(having) = &select.having { - let rewritten = substitute_aggregates(having, agg_slots, &synthetic_names); + let having = if rowid_reg.is_some() { + substitute_rowid_pseudo_column(having, schema) + } else { + having.clone() + }; + let rewritten = substitute_aggregates(&having, agg_slots, &synthetic_names); compile_cond( em, reg, @@ -443,10 +528,17 @@ where .columns .iter() .map(|col| match col { - ResultColumn::Expr { expr, alias } => ResultColumn::Expr { - expr: substitute_aggregates(expr, agg_slots, &synthetic_names), - alias: alias.clone(), - }, + ResultColumn::Expr { expr, alias } => { + let expr = if rowid_reg.is_some() { + substitute_rowid_pseudo_column(expr, schema) + } else { + expr.clone() + }; + ResultColumn::Expr { + expr: substitute_aggregates(&expr, agg_slots, &synthetic_names), + alias: alias.clone(), + } + } other => other.clone(), }) .collect(); diff --git a/src/codegen/select/aggregate/hash.rs b/src/codegen/select/aggregate/hash.rs index a0acd7ad..e26a807e 100644 --- a/src/codegen/select/aggregate/hash.rs +++ b/src/codegen/select/aggregate/hash.rs @@ -257,6 +257,7 @@ where schema, catalog, &snapshot_regs, + None, &agg_slots, limit.as_ref(), end_label, diff --git a/src/codegen/select/limit_scan.rs b/src/codegen/select/limit_scan.rs index 720d5b73..9bad78be 100644 --- a/src/codegen/select/limit_scan.rs +++ b/src/codegen/select/limit_scan.rs @@ -948,6 +948,32 @@ where Ok(()) } +/// #708: does `select` project a bare `rowid`/`_rowid_`/`oid` +/// pseudo-column reference (not a declared column, and not buried in a +/// compound expression — same narrow scope as the existing +/// `rowid_alias` special case in `projection.rs`)? If so, +/// `compile_sorted_scan`'s pass 1 must materialize it into the sorted +/// record — it has no field there otherwise, since pass 1's +/// schema-column block only ever iterates `schema.columns`. +fn select_projects_rowid_pseudo_column(select: &Select, schema: &TableSchema) -> bool { + select.columns.iter().any(|col| match col { + ResultColumn::Expr { + expr: + Expr { + kind: + ExprKind::Column { + name, + table: None, + catalog: None, + }, + .. + }, + .. + } => crate::codegen::expr::rowid_pseudo_column_index(schema, name).is_some(), + _ => false, + }) +} + #[allow(clippy::too_many_arguments)] pub(super) fn compile_sorted_scan( em: &mut Emitter, @@ -1075,6 +1101,18 @@ where catalog, )?; + // #708: materialize the bare rowid pseudo-column, if projected, + // immediately after the schema-column block — landing at record + // field index `schema.columns.len()`, the same sentinel + // `expr::rowid_pseudo_column_index` hands out, so pass 2's + // `projection.rs` special case can read it back with a plain + // `Column` op against the post-sort pseudo cursor instead of + // re-issuing `Rowid` (which only a real table cursor supports). + if !schema.without_rowid && select_projects_rowid_pseudo_column(select, schema) { + let r = reg.alloc(); + em.emit(Instruction::new(Opcode::Rowid, cursors.table, r, 0)); + } + // Compute every genuine-expression sort key into its own register, // appended after the schema-column block. A key's final register // need not be the highest one its expression allocates (e.g. `CASE` diff --git a/src/codegen/select/order_by.rs b/src/codegen/select/order_by.rs index c906bfd1..4807a734 100644 --- a/src/codegen/select/order_by.rs +++ b/src/codegen/select/order_by.rs @@ -146,6 +146,17 @@ pub(super) fn order_by_target_for_expr( schema: &TableSchema, ) -> Result { match &expr.kind { + // #708: `rowid`/`_rowid_`/`oid` has no schema column index to + // give `OrderByTarget::Column` (its "index" is the sentinel + // `emit_column_read` recognizes against a *live* cursor, which + // this target's consumers don't all have) — routing it through + // `Expr` instead reuses ordinary expression compilation + // (`Scope::resolve` already knows the pseudo-column). + ExprKind::Column { + table: None, name, .. + } if rowid_pseudo_column_index(schema, name).is_some() => { + Ok(OrderByTarget::Expr(expr.clone())) + } ExprKind::Column { table: None, name, .. } => column_index(schema, name) @@ -189,6 +200,9 @@ pub(super) fn resolve_order_by_target( { return order_by_target_for_expr(&entry.expr, schema); } + if rowid_pseudo_column_index(schema, name).is_some() { + return Ok(OrderByTarget::Expr(expr.clone())); + } column_index(schema, name) .map(OrderByTarget::Column) .ok_or_else(|| CodegenError::UnknownColumn { name: name.clone() }) diff --git a/src/codegen/select/projection.rs b/src/codegen/select/projection.rs index 7db24e7a..5ef772e1 100644 --- a/src/codegen/select/projection.rs +++ b/src/codegen/select/projection.rs @@ -119,16 +119,44 @@ pub(super) fn compile_row_values( // through to `compile_value`, matching this crate's // existing register-reuse limitations for compound // result-column expressions. + // + // #708's bare `rowid`/`_rowid_`/`oid` pseudo-column + // (not a declared column at all) needs the identical + // treatment: `compile_sorted_scan`'s pass 1 materializes + // it into the sorted record right after the schema + // columns block, at position `schema.columns.len()` — + // matching `rowid_pseudo_column_index`'s sentinel — so + // pass 2 reads it back with a plain `Column` op instead + // of re-issuing `Rowid` against the pseudo cursor. if let ExprKind::Column { name, table: None, catalog: None, } = &expr.kind { + let declared_idx = column_index(schema, name); let pseudo_rowid_idx = pseudo - .then(|| column_index(schema, name)) + .then_some(declared_idx) .flatten() - .filter(|idx| schema.rowid_alias == Some(*idx)); + .filter(|idx| schema.rowid_alias == Some(*idx)) + .or_else(|| { + // SQLite's shadowing rule: a *declared* + // column named `rowid`/`_rowid_`/`oid` wins + // over the pseudo-column, so the sentinel is + // only reachable when no such column exists. + // Without this guard a declared, non-alias + // `rowid` column read back through the + // post-`ORDER BY` pseudo cursor projected the + // hidden rowid instead of its own value. + if declared_idx.is_some() { + return None; + } + pseudo + .then(|| { + crate::codegen::expr::rowid_pseudo_column_index(schema, name) + }) + .flatten() + }); if let Some(idx) = pseudo_rowid_idx { let r = reg.alloc(); em.emit(Instruction::new( diff --git a/tests/unit/rowid_projection_test.rs b/tests/unit/rowid_projection_test.rs new file mode 100644 index 00000000..b562d03b --- /dev/null +++ b/tests/unit/rowid_projection_test.rs @@ -0,0 +1,537 @@ +// Copyright 2026 Schuberg Philis +// SPDX-License-Identifier: Apache-2.0 +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +//! #708: `rowid`/`_rowid_`/`oid` were resolvable in a `WHERE` clause +//! (`is_rowid_reference`'s seek fast path) but not in a projection — +//! `Scope::resolve` had no pseudo-column awareness. Oracle-diffed +//! against the pinned 3.53.4 `sqlite3`, reusing +//! `tests/unit/codegen_select_test.rs`'s scratch-db-plus-oracle +//! pattern, but through the real `oracle::pinned_oracle()` (never the +//! system `/usr/bin/sqlite3`). + +#[path = "../corpus/oracle.rs"] +#[allow(dead_code)] +mod oracle; + +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::rc::Rc; + +use oracle::pinned_oracle; +use sqlite_rs::codegen::compile_select; +use sqlite_rs::header::DatabaseHeader; +use sqlite_rs::parser::{parse_select, ParseOutcome}; +use sqlite_rs::record::Value; +use sqlite_rs::schema::TableSchema; +use sqlite_rs::vdbe::execute_with_db; +use sqlite_rs::vfs::{UnixVfs, Vfs, VfsPageSource}; + +fn scratch_db(label: &str, oracle: &Path, setup_sql: &str) -> PathBuf { + let path = std::env::temp_dir().join(format!( + "sqlite_rs_rowid_projection_test_{}_{}.db", + std::process::id(), + label + )); + std::fs::remove_file(&path).ok(); + let status = Command::new(oracle) + .arg(&path) + .arg(setup_sql) + .status() + .expect("creating scratch fixture db"); + assert!(status.success()); + path +} + +fn our_rows(path: &Path, schema: &TableSchema, sql: &str) -> Result>, String> { + let select = match parse_select(sql) { + ParseOutcome::Accepted(s) => *s, + other => return Err(format!("parser rejected {sql:?}: {other:?}")), + }; + let program = compile_select(&select, schema).map_err(|e| format!("{e:?}"))?; + let vfs = UnixVfs; + let file = vfs.open_read(path).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, path, header.page_size).unwrap(); + execute_with_db(&program, Rc::new(source), header).map_err(|e| format!("{e:?}")) +} + +fn oracle_rows(oracle: &Path, db: &Path, sql: &str) -> Vec> { + let output = Command::new(oracle) + .arg("-readonly") + .arg("-separator") + .arg("\u{1f}") + .arg(db) + .arg(sql) + .output() + .expect("invoking sqlite3 oracle"); + String::from_utf8_lossy(&output.stdout) + .lines() + .map(|l| l.split('\u{1f}').map(str::to_string).collect()) + .collect() +} + +fn value_to_oracle_text(v: &Value) -> String { + match v { + Value::Null => String::new(), + Value::Integer(i) => i.to_string(), + Value::Real(r) => { + if r.fract() == 0.0 { + format!("{r:.1}") + } else { + r.to_string() + } + } + Value::Text(s) => s.to_string(), + Value::Blob(_) => "".to_string(), + } +} + +fn assert_matches_oracle(oracle: &Path, db: &Path, schema: &TableSchema, sql: &str) { + let ours = our_rows(db, schema, sql).unwrap_or_else(|e| panic!("compiling {sql:?}: {e}")); + let ours_text: Vec> = ours + .iter() + .map(|row| row.iter().map(value_to_oracle_text).collect()) + .collect(); + let expected = oracle_rows(oracle, db, sql); + assert_eq!(ours_text, expected, "mismatch for {sql:?}"); +} + +fn rowid_table_schema() -> TableSchema { + TableSchema { + unresolved_autoindex: false, + name: "t".to_string(), + root_page: 2, + columns: vec!["a".to_string(), "v".to_string()], + column_types: vec!["INTEGER".to_string(), "TEXT".to_string()], + column_collations: vec![], + without_rowid: false, + strict: false, + is_virtual: false, + sql: String::new(), + indexes: vec![], + rowid_alias: None, + } + .with_computed_rowid_alias() +} + +/// The core defect: `rowid` in a projection alongside real columns. +/// Also exercises gaps left by a delete (#708's acceptance criteria). +#[test] +fn rowid_selectable_in_projection_including_after_deletes() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "basic", + &oracle, + "CREATE TABLE t(a INTEGER, v TEXT); \ + INSERT INTO t VALUES (1, 'aa'), (2, 'bb'), (3, 'cc'), (4, 'dd'); \ + DELETE FROM t WHERE a = 2;", + ); + let schema = rowid_table_schema(); + assert_matches_oracle(&oracle, &db, &schema, "SELECT rowid, a, v FROM t"); + assert_matches_oracle(&oracle, &db, &schema, "SELECT _rowid_, a FROM t"); + assert_matches_oracle(&oracle, &db, &schema, "SELECT oid FROM t"); +} + +/// `rowid` in a projection, WHERE and ORDER BY together, plus an +/// alias, in the same statement. +#[test] +fn rowid_works_in_where_order_by_and_alias_together() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "combo", + &oracle, + "CREATE TABLE t(a INTEGER, v TEXT); \ + INSERT INTO t VALUES (1, 'aa'), (2, 'bb'), (3, 'cc');", + ); + let schema = rowid_table_schema(); + assert_matches_oracle( + &oracle, + &db, + &schema, + "SELECT rowid AS rid, a FROM t WHERE rowid > 1 ORDER BY rowid DESC", + ); +} + +/// Shadowing: a declared column named `rowid` wins over the +/// pseudo-column. +#[test] +fn declared_rowid_column_shadows_the_pseudo_column() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "shadow", + &oracle, + "CREATE TABLE t(rowid TEXT, v INTEGER); \ + INSERT INTO t VALUES ('x', 1), ('y', 2);", + ); + let schema = TableSchema { + unresolved_autoindex: false, + name: "t".to_string(), + root_page: 2, + columns: vec!["rowid".to_string(), "v".to_string()], + column_types: vec!["TEXT".to_string(), "INTEGER".to_string()], + column_collations: vec![], + without_rowid: false, + strict: false, + is_virtual: false, + sql: String::new(), + indexes: vec![], + rowid_alias: None, + } + .with_computed_rowid_alias(); + assert_matches_oracle(&oracle, &db, &schema, "SELECT rowid, v FROM t"); +} + +/// `INTEGER PRIMARY KEY` tables: `rowid` and the alias column must +/// agree. +#[test] +fn integer_primary_key_alias_and_rowid_agree() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "ipk", + &oracle, + "CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT); \ + INSERT INTO t VALUES (5, 'aa'), (9, 'bb');", + ); + let schema = TableSchema { + unresolved_autoindex: false, + name: "t".to_string(), + root_page: 2, + columns: vec!["id".to_string(), "v".to_string()], + column_types: vec!["INTEGER".to_string(), "TEXT".to_string()], + column_collations: vec![], + without_rowid: false, + strict: false, + is_virtual: false, + sql: "CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT)".to_string(), + indexes: vec![], + rowid_alias: None, + } + .with_computed_rowid_alias(); + assert_matches_oracle(&oracle, &db, &schema, "SELECT rowid, id, v FROM t"); +} + +/// `WITHOUT ROWID` tables have no rowid at all — must be rejected the +/// same way the oracle rejects it (an unknown-column error), not +/// silently produce a number. +#[test] +fn without_rowid_table_rejects_rowid_reference() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "without_rowid", + &oracle, + "CREATE TABLE t(a INTEGER PRIMARY KEY, v TEXT) WITHOUT ROWID; \ + INSERT INTO t VALUES (1, 'aa');", + ); + let oracle_out = Command::new(&oracle) + .arg("-readonly") + .arg(&db) + .arg("SELECT rowid FROM t") + .output() + .expect("invoking sqlite3 oracle"); + assert!( + !oracle_out.status.success(), + "expected the oracle itself to reject rowid on a WITHOUT ROWID table" + ); + + let schema = TableSchema { + unresolved_autoindex: false, + name: "t".to_string(), + root_page: 2, + columns: vec!["a".to_string(), "v".to_string()], + column_types: vec!["INTEGER".to_string(), "TEXT".to_string()], + column_collations: vec![], + without_rowid: true, + strict: false, + is_virtual: false, + sql: "CREATE TABLE t(a INTEGER PRIMARY KEY, v TEXT) WITHOUT ROWID".to_string(), + indexes: vec![], + rowid_alias: None, + } + .with_computed_rowid_alias(); + let err = our_rows(&db, &schema, "SELECT rowid FROM t") + .expect_err("rowid on a WITHOUT ROWID table must be rejected"); + assert!( + err.contains("UnknownColumn"), + "expected an unknown-column error, got: {err}" + ); +} + +/// #708 follow-up: `GROUP BY rowid` regressed relative to main (a +/// runtime "pseudo cursor" internals error) because `compile_grouped_scan` +/// never got the materialize-then-read-back-with-`Column` treatment +/// `compile_sorted_scan` (#718) established for `ORDER BY rowid`. Covers +/// the base case, gaps left by a delete, and every spelling. +#[test] +fn group_by_rowid_matches_oracle() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "group_by_rowid", + &oracle, + "CREATE TABLE t(a INTEGER, v TEXT); \ + INSERT INTO t VALUES (1, 'aa'), (2, 'bb'), (3, 'cc'), (4, 'dd'); \ + DELETE FROM t WHERE a = 2;", + ); + let schema = rowid_table_schema(); + assert_matches_oracle( + &oracle, + &db, + &schema, + "SELECT rowid, count(*) FROM t GROUP BY rowid", + ); + assert_matches_oracle( + &oracle, + &db, + &schema, + "SELECT _rowid_, count(*) FROM t GROUP BY _rowid_", + ); + assert_matches_oracle( + &oracle, + &db, + &schema, + "SELECT oid, count(*) FROM t GROUP BY oid", + ); +} + +/// #708 follow-up: `GROUP BY rowid` combined with `HAVING` on the same +/// pseudo-column — both the group key comparison and the `HAVING` +/// re-projection go through the same pass-2 pseudo cursor, so both need +/// the fix. +#[test] +fn group_by_rowid_with_having_matches_oracle() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "group_by_rowid_having", + &oracle, + "CREATE TABLE t(a INTEGER, v TEXT); \ + INSERT INTO t VALUES (1, 'aa'), (2, 'bb'), (3, 'cc'), (4, 'dd'); \ + DELETE FROM t WHERE a = 2;", + ); + let schema = rowid_table_schema(); + assert_matches_oracle( + &oracle, + &db, + &schema, + "SELECT rowid, count(*) FROM t GROUP BY rowid HAVING rowid > 1", + ); + // A mix of spellings between the `GROUP BY` key and `HAVING` is + // legal in SQLite (all three are the same pseudo-column). + assert_matches_oracle( + &oracle, + &db, + &schema, + "SELECT rowid, count(*) FROM t GROUP BY rowid HAVING _rowid_ > 1", + ); +} + +/// #708 follow-up: `GROUP BY` combined with `ORDER BY` is rejected +/// outright regardless of what's being grouped by (see +/// `compile_select_scan`'s check, predating this ticket) — `rowid` +/// must hit that same clean compile-time rejection, not the "pseudo +/// cursor" runtime error this ticket fixes elsewhere, and not a panic. +#[test] +fn group_by_rowid_with_order_by_is_cleanly_unsupported() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let schema = rowid_table_schema(); + let db = scratch_db( + "group_by_rowid_order_by", + &oracle, + "CREATE TABLE t(a INTEGER, v TEXT); INSERT INTO t VALUES (1, 'aa'), (2, 'bb');", + ); + let err = our_rows( + &db, + &schema, + "SELECT rowid, count(*) FROM t GROUP BY rowid ORDER BY rowid", + ) + .expect_err("GROUP BY combined with ORDER BY is not yet supported, rowid or not"); + assert!( + err.contains("Unsupported"), + "expected a clean Unsupported rejection, not a runtime error, got: {err}" + ); + assert!( + !err.contains("pseudo cursor") && !err.contains("CursorTypeMismatch"), + "must not surface the internals error this ticket fixes, got: {err}" + ); +} + +/// Control: `GROUP BY` on an ordinary declared column, with no rowid +/// involved at all, must not regress from this ticket's changes to +/// `compile_grouped_scan`'s pass 1/pass 2. +#[test] +fn group_by_real_column_still_matches_oracle() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "group_by_real_column", + &oracle, + "CREATE TABLE t(a INTEGER, v TEXT); \ + INSERT INTO t VALUES (1, 'aa'), (2, 'aa'), (3, 'bb');", + ); + let schema = rowid_table_schema(); + assert_matches_oracle( + &oracle, + &db, + &schema, + "SELECT v, count(*) FROM t GROUP BY v", + ); +} + +/// `WITHOUT ROWID` tables have no rowid to `GROUP BY` at all — must be +/// rejected the same way a bare `SELECT rowid` already is (#708), not +/// silently produce a number and not hit the pseudo-cursor internals +/// error. +#[test] +fn group_by_rowid_on_without_rowid_table_rejects_cleanly() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned 3.53.4 sqlite3 oracle on this machine"); + return; + }; + let db = scratch_db( + "group_by_without_rowid", + &oracle, + "CREATE TABLE t(a INTEGER PRIMARY KEY, v TEXT) WITHOUT ROWID; \ + INSERT INTO t VALUES (1, 'aa'), (2, 'bb');", + ); + let oracle_out = Command::new(&oracle) + .arg("-readonly") + .arg(&db) + .arg("SELECT rowid, count(*) FROM t GROUP BY rowid") + .output() + .expect("invoking sqlite3 oracle"); + assert!( + !oracle_out.status.success(), + "expected the oracle itself to reject rowid on a WITHOUT ROWID table" + ); + + let schema = TableSchema { + unresolved_autoindex: false, + name: "t".to_string(), + root_page: 2, + columns: vec!["a".to_string(), "v".to_string()], + column_types: vec!["INTEGER".to_string(), "TEXT".to_string()], + column_collations: vec![], + without_rowid: true, + strict: false, + is_virtual: false, + sql: "CREATE TABLE t(a INTEGER PRIMARY KEY, v TEXT) WITHOUT ROWID".to_string(), + indexes: vec![], + rowid_alias: None, + } + .with_computed_rowid_alias(); + let err = our_rows(&db, &schema, "SELECT rowid, count(*) FROM t GROUP BY rowid") + .expect_err("GROUP BY rowid on a WITHOUT ROWID table must be rejected"); + assert!( + err.contains("UnknownColumn"), + "expected an unknown-column error, got: {err}" + ); +} + +/// #708 follow-up: the implicit whole-table group (an aggregate with no +/// `GROUP BY`) took `try_compile_direct_agg_scan`'s fast path, whose +/// synthetic per-group record has one field per declared column and no +/// slot for the rowid pseudo-column — so `SELECT rowid, count(*)` read +/// off the end of the record and projected an empty value where the +/// oracle returns the rowid. Silent wrong answer, not an error. +#[test] +fn bare_rowid_alongside_an_aggregate_with_no_group_by() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned sqlite3 oracle found"); + return; + }; + let db = scratch_db( + "implicit_group_rowid", + &oracle, + "CREATE TABLE t(a INTEGER, v TEXT); \ + INSERT INTO t VALUES (10, 'aa'), (20, 'bb'), (30, 'cc');", + ); + let schema = rowid_table_schema(); + for sql in [ + "SELECT rowid, count(*) FROM t", + "SELECT _rowid_, count(*) FROM t", + "SELECT oid, count(*) FROM t", + "SELECT rowid, count(*) FROM t WHERE v > 'aa'", + "SELECT rowid, count(*) FROM t HAVING count(*) > 0", + "SELECT max(rowid) FROM t", + // Control: the fast path must still serve an aggregate that + // never mentions the pseudo-column. + "SELECT count(*) FROM t", + "SELECT count(*), max(a) FROM t", + ] { + assert_matches_oracle(&oracle, &db, &schema, sql); + } +} + +/// #708 follow-up: SQLite's shadowing rule says a *declared* column +/// named `rowid` wins over the pseudo-column. That held on a plain scan +/// but broke once the row was read back through the post-`ORDER BY` +/// pseudo cursor, which projected the hidden rowid instead of the +/// declared column's own value — correct on `origin/main`, so a +/// regression this stack introduced. +#[test] +fn a_declared_rowid_column_still_wins_after_order_by() { + let Some(oracle) = pinned_oracle() else { + eprintln!("skipping: no pinned sqlite3 oracle found"); + return; + }; + let db = scratch_db( + "declared_rowid_order_by", + &oracle, + "CREATE TABLE t(rowid TEXT, v TEXT); \ + INSERT INTO t VALUES ('r1', 'bb'), ('r2', 'aa');", + ); + let schema = TableSchema { + unresolved_autoindex: false, + name: "t".to_string(), + root_page: 2, + columns: vec!["rowid".to_string(), "v".to_string()], + column_types: vec!["TEXT".to_string(), "TEXT".to_string()], + column_collations: vec![], + without_rowid: false, + strict: false, + is_virtual: false, + sql: "CREATE TABLE t(rowid TEXT, v TEXT)".to_string(), + indexes: vec![], + rowid_alias: None, + } + .with_computed_rowid_alias(); + for sql in [ + // The regression: a sort puts the read behind a pseudo cursor. + "SELECT rowid, v FROM t ORDER BY v", + "SELECT rowid, v FROM t ORDER BY v DESC", + "SELECT rowid FROM t ORDER BY rowid", + "SELECT t.rowid FROM t ORDER BY v", + // Controls: these were already correct and must stay so. + "SELECT rowid, v FROM t", + "SELECT rowid, v FROM t WHERE rowid = 'r1'", + ] { + assert_matches_oracle(&oracle, &db, &schema, sql); + } +}