Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
16 changes: 10 additions & 6 deletions src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down Expand Up @@ -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<usize> = 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(),
Expand Down
2 changes: 1 addition & 1 deletion src/codegen/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
22 changes: 22 additions & 0 deletions src/codegen/expr/cond.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,28 @@ pub(crate) fn column_index(schema: &TableSchema, name: &str) -> Option<usize> {
.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<usize> {
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
Expand Down
6 changes: 5 additions & 1 deletion src/codegen/expr/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(());
}
Expand Down
2 changes: 1 addition & 1 deletion src/codegen/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
152 changes: 152 additions & 0 deletions src/codegen/select/aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -431,6 +443,7 @@ where
schema,
catalog,
&snapshot_regs,
None,
&agg_slots,
limit.as_ref(),
end_label,
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -810,13 +894,34 @@ 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 {
// Always resolves: `needed_columns` includes every column
// `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)
Expand Down Expand Up @@ -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()`.
Expand Down Expand Up @@ -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::<Result<_, CodegenError>>()?;
Expand Down Expand Up @@ -995,6 +1129,7 @@ where
schema,
catalog,
&snapshot_regs,
rowid_snapshot_reg,
&agg_slots,
limit.as_ref(),
end_label,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1070,6 +1219,7 @@ where
schema,
catalog,
&snapshot_regs,
rowid_snapshot_reg,
&agg_slots,
limit.as_ref(),
end_label,
Expand Down Expand Up @@ -1331,6 +1481,7 @@ where
schema,
catalog,
&snapshot_regs,
None,
&agg_slots,
limit.as_ref(),
end_label,
Expand Down Expand Up @@ -1374,6 +1525,7 @@ where
schema,
catalog,
&snapshot_regs,
None,
&agg_slots,
limit.as_ref(),
end_label,
Expand Down
Loading
Loading