From 2f0a517a4a1a56e17130f09f92ba235644573edb Mon Sep 17 00:00:00 2001 From: Diederik Siderius Date: Fri, 4 Sep 2026 14:40:56 +0200 Subject: [PATCH 1/2] refactor: lift the SELECT compile pipeline out of the CLI into the library (#695) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Connection::prepare(sql)` cannot be written today. `compile_statement` handles write and DDL statements only — hand it a `SELECT` and it answers `Unrecognized("SELECT")`, which I hit for real while writing #692's tests. A `SELECT` needs its CTEs and views expanded, its FROM tables resolved and `sqlite_stat1` stats loaded first, and the pipeline that does all of that lived in `src/bin/sqlite-rs/query.rs` — inside the executable, where no library consumer can reach it. That split is a CLI implementation detail, not something a consumer should have to know about. Stock SQLite has exactly one `sqlite3_prepare_v2()`: hand it any statement, get a handle back. Spec 013's `Connection::prepare` has to behave the same way, so the missing half moves into `src/codegen/prepare.rs` where both callers can use it. Not a rewrite. `compile_select_program` and `SelectOutcome` are the CLI's own code relocated, and the 127-line local copy is deleted rather than left to drift — `query.rs` and `repl.rs` now call the library's. `derive_headers` comes along as `result_column_names`, so `Row`'s by-name access and the CLI's column headers will agree by construction instead of by coincidence. One thing changed rather than moved: every error was `.map_err(|e| e.to_string())`, flattening a structured `CodegenError` into a message. That is fine when the next step is printing to a terminal and wrong for a library, where a caller needs to match on what went wrong. Errors are now `PrepareError`, which wraps `CodegenError` and adds the one case that is about the request rather than the SQL: asking for `EXPLAIN QUERY PLAN` on a FROM-less `SELECT`, which is not a `CodegenError::NoFromClause` because `SELECT 1` compiles fine, it just has no access path to explain. `Display` reproduces the old strings exactly, so CLI output is unchanged. `from_less_schema()` stays private. The CLI inlined that 12-field literal; a public `TableSchema::none()` would be new API surface this lift does not need. The CLI behaving identically is the claim that matters here, since this is a refactor of the path every `sqlite-rs query` and REPL statement takes. Evidence: 1575 unit, 388 corpus (which includes the CLI e2e suite) and 15 sqllogictest all pass unchanged, `make lint` clean both clippy passes, and four queries plus `EXPLAIN QUERY PLAN` hand-checked byte-identical against the pinned 3.53.4 oracle — including the FROM-less EQP error, whose message survives the `String` -> `PrepareError` change intact. No `Connection` or `Statement` yet; this is the groundwork they need. PRAGMA dispatch is deliberately not lifted: spec 013's non-goals give the PRAGMA catalogue to V7, and SQE's statement list contains none. Refs: 013/Req-3, #695, #678 Co-Authored-By: Claude Opus 5 (1M context) --- src/bin/sqlite-rs/query.rs | 137 +---------------------- src/bin/sqlite-rs/repl.rs | 5 +- src/codegen.rs | 2 + src/codegen/prepare.rs | 217 +++++++++++++++++++++++++++++++++++++ 4 files changed, 224 insertions(+), 137 deletions(-) create mode 100644 src/codegen/prepare.rs diff --git a/src/bin/sqlite-rs/query.rs b/src/bin/sqlite-rs/query.rs index a9a4744f..03804298 100644 --- a/src/bin/sqlite-rs/query.rs +++ b/src/bin/sqlite-rs/query.rs @@ -14,150 +14,17 @@ use std::process::ExitCode; use std::rc::Rc; use sqlite_rs::btree::TableCursor; -use sqlite_rs::codegen::{ - compile_select_compound, compile_select_joined, compile_select_with_catalog, - compile_select_with_catalog_and_stats, expand_with_clause, explain_query_plan, - flatten_from_subqueries, push_down_where_predicates, resolve_from_table_schema, resolve_views, - CodegenError, EqpRow, ExpandViews, -}; +use sqlite_rs::codegen::{compile_select_program, SelectOutcome}; use sqlite_rs::dump; use sqlite_rs::format::{write_csv_value, write_query_value}; -use sqlite_rs::parser::ast::Select; use sqlite_rs::parser::{parse_explain, parse_select, ParseOutcome}; use sqlite_rs::schema::{read_schema, read_views, TableSchema, ViewSchema}; -use sqlite_rs::vdbe::{execute_with_db, explain, Program}; +use sqlite_rs::vdbe::{execute_with_db, explain}; use sqlite_rs::vfs::{PageSource, UnixVfs}; use crate::common::{fatal, CSV_ROW_TERMINATOR}; use crate::pragma_query::{execute_pragma_query, parse_pragma_query}; -/// What [`compile_select_program`] produced: either `EXPLAIN QUERY -/// PLAN`'s rows (nothing further to compile — there's no bytecode to -/// run or `-explain`) or an ordinary compiled `Program`. -pub(crate) enum SelectOutcome { - Eqp(Vec), - Program(Program), -} - -/// Parses (already done by the caller — `select`/`eqp_mode` come from -/// `parse_select`/`parse_explain`), resolves every table `select` -/// touches against `schemas`, and compiles it: FROM-less (#260), -/// single-table, joined (#237), or `UNION ALL` compound (#240), -/// whichever `select`'s shape calls for. Shared by `run_query` (a -/// fresh read-only `Pager` per invocation) and the REPL (#365, one -/// shared read/write `Pager` per session) — both need exactly this -/// parse-resolve-compile pipeline, just against a different -/// `PageSource`. -pub(crate) fn compile_select_program( - select: &Select, - eqp_mode: bool, - schemas: &[TableSchema], - views: &[ViewSchema], - stats_by_table: &std::collections::HashMap, -) -> Result { - // #376: a `WITH` clause is rewritten away before any table - // resolution happens — every CTE reference in `FROM`/`JOIN` becomes - // a `TableRefKind::Subquery` wrapping that CTE's own query, so the - // rest of this pipeline (and #257's subquery-in-FROM codegen) needs - // no CTE-specific handling at all. - let cte_expanded = expand_with_clause(select); - // #380: every catalog-view reference in `FROM`/`JOIN` is rewritten - // away next, the same shape as the CTE rewrite above — into a - // `TableRefKind::Subquery` wrapping the view's own stored query, - // reusing #257's subquery-in-FROM codegen unchanged. Runs *after* - // the CTE rewrite (rather than before) so it also reaches into any - // `TableRefKind::Subquery` the CTE rewrite just produced — a CTE - // whose own body references a view is resolved this way, without - // `expand_views` needing any CTE-specific handling of its own; a - // CTE also shadows a same-named view for the scope of its declaring - // `SELECT`, matching how it already shadows a same-named real table. - let resolved_views = resolve_views(views); - let expanded = cte_expanded - .expand_views(&resolved_views) - .map_err(|e| e.to_string())?; - // `flatten_from_subqueries`/`push_down_where_predicates` below always - // need `&mut Select`, so this is where the deferred clone (if any — - // Cow was `Borrowed` for the common no-CTE/no-view case) finally - // happens, at most once total rather than once per expansion pass. - let mut expanded = expanded.into_owned(); - // #566: flatten a simple FROM-subquery/view/CTE directly into the - // enclosing query first — eliminating it outright makes any base- - // table index it hides visible to the planner, which a mere - // predicate push-down (below) can't do. - flatten_from_subqueries(&mut expanded); - // #532: push safely-movable outer WHERE conjuncts into whatever - // views/derived-tables flattening didn't eliminate, so their own - // materialization scan (below) can filter before scanning. - push_down_where_predicates(&mut expanded); - let select = &expanded; - - let resolve_table = |table_ref: &sqlite_rs::parser::ast::TableRef| { - resolve_from_table_schema(table_ref, schemas) - }; - - let Some(from) = &select.from else { - if eqp_mode { - return Err("EXPLAIN QUERY PLAN requires a FROM clause".to_string()); - } - let no_table = TableSchema { - unresolved_autoindex: false, - name: String::new(), - root_page: 0, - columns: vec![], - column_types: vec![], - column_collations: vec![], - without_rowid: false, - strict: false, - is_virtual: false, - sql: String::new(), - indexes: vec![], - rowid_alias: None, - }; - let program = - compile_select_with_catalog(select, &no_table, &[]).map_err(|e| e.to_string())?; - return Ok(SelectOutcome::Program(program)); - }; - - let schema = resolve_table(&from.first).map_err(|e| e.to_string())?; - - if eqp_mode { - let mut joined_schemas = vec![schema]; - for join in &from.joins { - joined_schemas.push(resolve_table(&join.table).map_err(|e| e.to_string())?); - } - let rows = explain_query_plan(select, &joined_schemas, stats_by_table, schemas) - .map_err(|e| e.to_string())?; - return Ok(SelectOutcome::Eqp(rows)); - } - - let program = if !select.compound.is_empty() { - let mut arm_schemas = Vec::with_capacity(select.compound.len()); - for arm in &select.compound { - let Some(arm_from) = &arm.from else { - return Err(CodegenError::NoFromClause.to_string()); - }; - arm_schemas.push(resolve_table(&arm_from.first).map_err(|e| e.to_string())?); - } - compile_select_compound(select, &schema, &arm_schemas, schemas) - .map_err(|e| e.to_string())? - } else if from.joins.is_empty() { - let stats = stats_by_table - .get(&schema.name) - .cloned() - .unwrap_or_default(); - compile_select_with_catalog_and_stats(select, &schema, schemas, &stats) - .map_err(|e| e.to_string())? - } else { - let mut joined_schemas = vec![schema]; - for join in &from.joins { - joined_schemas.push(resolve_table(&join.table).map_err(|e| e.to_string())?); - } - compile_select_joined(select, &joined_schemas, schemas, stats_by_table) - .map_err(|e| e.to_string())? - }; - Ok(SelectOutcome::Program(program)) -} - pub fn run_query(raw_args: Vec) -> ExitCode { let mut csv = false; let mut explain_flag = false; diff --git a/src/bin/sqlite-rs/repl.rs b/src/bin/sqlite-rs/repl.rs index b6fdcd7d..b2df7090 100644 --- a/src/bin/sqlite-rs/repl.rs +++ b/src/bin/sqlite-rs/repl.rs @@ -47,7 +47,8 @@ use std::rc::Rc; use sqlite_rs::btree::TableCursor; use sqlite_rs::codegen::{ - compile_statement, leading_keywords, output_column_names, resolve_from_table_schema, + compile_select_program, compile_statement, leading_keywords, output_column_names, + resolve_from_table_schema, SelectOutcome, }; use sqlite_rs::dump; use sqlite_rs::parser::{ends_with_semicolon, parse_select, split_statements, ParseOutcome}; @@ -60,7 +61,7 @@ use crate::dot_commands::{ }; use crate::mode::{print_rows, OutputMode}; use crate::pragma_query::{execute_pragma_query, parse_pragma_query}; -use crate::query::{compile_select_program, write_list_row, SelectOutcome}; +use crate::query::write_list_row; use crate::readline::{history_path, ReadlineError}; use crate::tables::{list_table_and_view_names, print_table_names}; diff --git a/src/codegen.rs b/src/codegen.rs index 177185c8..19f6040f 100644 --- a/src/codegen.rs +++ b/src/codegen.rs @@ -12,6 +12,7 @@ pub mod dispatch; pub mod expr; pub(crate) mod index_maintenance; pub mod pragma; +pub mod prepare; pub mod select; pub mod stmt; pub(crate) mod subquery; @@ -24,6 +25,7 @@ pub use ddl::{ }; pub use dispatch::{compile_statement, leading_keywords, DispatchError}; pub use pragma::compile_pragma; +pub use prepare::{compile_select_program, result_column_names, PrepareError, SelectOutcome}; pub use select::{ compile_select, compile_select_compound, compile_select_joined, compile_select_with_catalog, compile_select_with_catalog_and_stats, explain_query_plan, output_column_names, CodegenError, diff --git a/src/codegen/prepare.rs b/src/codegen/prepare.rs new file mode 100644 index 00000000..ad6b3407 --- /dev/null +++ b/src/codegen/prepare.rs @@ -0,0 +1,217 @@ +// Copyright 2026 Schuberg Philis +// SPDX-License-Identifier: Apache-2.0 +//! Compiling an arbitrary SQL string, whatever kind of statement it is. +//! +//! [`compile_statement`](crate::codegen::compile_statement) covers write +//! and DDL statements only — hand it a `SELECT` and it answers +//! `DispatchError::Unrecognized("SELECT")`. A `SELECT` needs its `FROM` +//! tables resolved, its CTEs and views expanded, and `sqlite_stat1` +//! statistics loaded before it can be compiled at all, and the pipeline +//! that does that lived in `src/bin/sqlite-rs/query.rs` — inside the +//! executable, where no library consumer could reach it. +//! +//! That split is a CLI implementation detail, not something a consumer +//! should have to know. Stock SQLite has exactly one +//! `sqlite3_prepare_v2()`: you hand it any statement and get a handle +//! back. Spec 013's `Connection::prepare` has to behave the same way, so +//! the `SELECT` half moves here where both the CLI and the embedding API +//! can call it (013/Req 3, #695). +//! +//! Errors are [`CodegenError`]/[`PrepareError`] rather than the `String` +//! the CLI copy produced. Flattening a structured error into a message is +//! fine when the next step is printing it to a terminal; a library +//! consumer needs to match on what went wrong. + +use std::collections::HashMap; +use std::fmt; + +use crate::codegen::{ + compile_select_compound, compile_select_joined, compile_select_with_catalog, + compile_select_with_catalog_and_stats, expand_with_clause, explain_query_plan, + flatten_from_subqueries, output_column_names, push_down_where_predicates, + resolve_from_table_schema, resolve_views, CodegenError, EqpRow, ExpandViews, +}; +use crate::parser::ast::{Select, TableRef}; +use crate::planner::Stats; +use crate::schema::{TableSchema, ViewSchema}; +use crate::vdbe::Program; + +/// What [`compile_select_program`] produced: either `EXPLAIN QUERY PLAN`'s rows +/// (nothing further to compile — there is no bytecode to run) or an +/// ordinary compiled [`Program`]. +pub enum SelectOutcome { + /// `EXPLAIN QUERY PLAN` output rows. + Eqp(Vec), + /// A compiled program, ready to execute. + Program(Program), +} + +/// Why a `SELECT` could not be compiled. +/// +/// Distinct from [`CodegenError`] only for the cases that are about the +/// *request* rather than the SQL: asking for `EXPLAIN QUERY PLAN` on a +/// statement that has no `FROM` clause to plan. +#[derive(Debug, PartialEq, Eq)] +pub enum PrepareError { + /// `EXPLAIN QUERY PLAN` was requested for a FROM-less `SELECT`. + /// + /// Not a `CodegenError::NoFromClause`: a FROM-less `SELECT 1` is + /// perfectly compilable, it just has no access path to explain. + EqpWithoutFrom, + /// The statement itself could not be compiled. + Codegen(CodegenError), +} + +impl fmt::Display for PrepareError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + PrepareError::EqpWithoutFrom => { + write!(f, "EXPLAIN QUERY PLAN requires a FROM clause") + } + PrepareError::Codegen(e) => write!(f, "{e}"), + } + } +} + +impl std::error::Error for PrepareError {} + +impl From for PrepareError { + fn from(e: CodegenError) -> Self { + PrepareError::Codegen(e) + } +} + +/// Resolves every table a `SELECT` touches and compiles it — FROM-less +/// (#260), single-table, joined (#237), or compound (#240), whichever +/// its shape calls for. +/// +/// `select`/`eqp_mode` come from `parse_select`/`parse_explain`; parsing +/// is the caller's. Lifted verbatim from the CLI's own +/// `compile_select_program` (#695), with `String` errors replaced by +/// [`PrepareError`] — the CLI and the embedding API need exactly this +/// pipeline, just against a different `PageSource`. +pub fn compile_select_program( + select: &Select, + eqp_mode: bool, + schemas: &[TableSchema], + views: &[ViewSchema], + stats_by_table: &HashMap, +) -> Result { + // #376: a `WITH` clause is rewritten away before any table + // resolution happens — every CTE reference in `FROM`/`JOIN` becomes + // a `TableRefKind::Subquery` wrapping that CTE's own query, so the + // rest of this pipeline (and #257's subquery-in-FROM codegen) needs + // no CTE-specific handling at all. + let cte_expanded = expand_with_clause(select); + // #380: every catalog-view reference is rewritten away next, the + // same shape as the CTE rewrite. Runs *after* it so it also reaches + // into any `TableRefKind::Subquery` the CTE rewrite just produced. + let resolved_views = resolve_views(views); + let expanded = cte_expanded.expand_views(&resolved_views)?; + // The passes below need `&mut Select`, so this is where the deferred + // clone (if any — `Cow` was `Borrowed` for the common + // no-CTE/no-view case) finally happens, at most once total. + let mut expanded = expanded.into_owned(); + // #566: flatten a simple FROM-subquery/view/CTE into the enclosing + // query first — eliminating it outright makes any base-table index + // it hides visible to the planner, which a predicate push-down + // cannot do. + flatten_from_subqueries(&mut expanded); + // #532: push safely-movable outer WHERE conjuncts into whatever + // flattening did not eliminate. + push_down_where_predicates(&mut expanded); + let select = &expanded; + + let resolve_table = |table_ref: &TableRef| -> Result { + resolve_from_table_schema(table_ref, schemas) + }; + + let Some(from) = &select.from else { + if eqp_mode { + return Err(PrepareError::EqpWithoutFrom); + } + let program = compile_select_with_catalog(select, &from_less_schema(), &[])?; + return Ok(SelectOutcome::Program(program)); + }; + + let schema = resolve_table(&from.first)?; + + if eqp_mode { + let mut joined_schemas = vec![schema]; + for join in &from.joins { + joined_schemas.push(resolve_table(&join.table)?); + } + let rows = explain_query_plan(select, &joined_schemas, stats_by_table, schemas)?; + return Ok(SelectOutcome::Eqp(rows)); + } + + let program = if !select.compound.is_empty() { + let mut arm_schemas = Vec::with_capacity(select.compound.len()); + for arm in &select.compound { + let Some(arm_from) = &arm.from else { + return Err(CodegenError::NoFromClause.into()); + }; + arm_schemas.push(resolve_table(&arm_from.first)?); + } + compile_select_compound(select, &schema, &arm_schemas, schemas)? + } else if from.joins.is_empty() { + let stats = stats_by_table + .get(&schema.name) + .cloned() + .unwrap_or_default(); + compile_select_with_catalog_and_stats(select, &schema, schemas, &stats)? + } else { + let mut joined_schemas = vec![schema]; + for join in &from.joins { + joined_schemas.push(resolve_table(&join.table)?); + } + compile_select_joined(select, &joined_schemas, schemas, stats_by_table)? + }; + Ok(SelectOutcome::Program(program)) +} + +/// The result column names a `SELECT` produces, for access by name. +/// +/// Falls back to `column1`, `column2`, ... for shapes whose names this +/// cannot derive (joins and compounds), which is what the CLI already +/// printed as headers for them. Lifted from the CLI's `derive_headers` +/// (#695) so `Row::get_by_name` and the CLI agree by construction rather +/// than by coincidence. +pub fn result_column_names(select: &Select, schemas: &[TableSchema]) -> Vec { + let single_table = select.compound.is_empty() + && select + .from + .as_ref() + .is_some_and(|from| from.joins.is_empty()); + if single_table { + if let Some(from) = &select.from { + if let Ok(schema) = resolve_from_table_schema(&from.first, schemas) { + return output_column_names(select, &schema); + } + } + } + let count = select.columns.len().max(1); + (1..=count).map(|i| format!("column{i}")).collect() +} + +/// The placeholder schema a FROM-less `SELECT` compiles against (#260). +/// +/// Private on purpose: the CLI inlined this literal, and a public +/// `TableSchema::none()` would be new API surface this lift does not +/// need. +fn from_less_schema() -> TableSchema { + TableSchema { + name: String::new(), + root_page: 0, + columns: vec![], + column_types: vec![], + column_collations: vec![], + without_rowid: false, + strict: false, + is_virtual: false, + sql: String::new(), + indexes: vec![], + rowid_alias: None, + unresolved_autoindex: false, + } +} From 6b9f3bc6a5a911e999da63a5a03b8ecd6cd7fa38 Mon Sep 17 00:00:00 2001 From: Diederik Siderius Date: Fri, 4 Sep 2026 14:52:05 +0200 Subject: [PATCH 2/2] test: cover the lifted SELECT pipeline, and correct the bench comment it falsified (#695) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lift commit had no tests of its own. Every existing exercise of `compile_select_program` went through the `sqlite-rs` binary, so the newly-public library surface had zero direct coverage and a regression in it would only have surfaced as a CLI failure. Two files fix that. `tests/unit/prepare_test.rs` (10 tests) pins the dispatch and the error type: one case per `compile_select_program` arm (FROM-less, single-table, joined, compound), `eqp_mode` returning rows rather than a program, `PrepareError::EqpWithoutFrom` including its message being byte-identical to the string the CLI printed before the lift, an unknown table arriving as a matchable `PrepareError::Codegen` rather than a message to parse, and `result_column_names`' positional fallback for joins and compounds — asserted rather than left implicit, because `Row::get_by_name` will be built on it and `column1` is a real answer a consumer can receive. `tests/corpus/prepare_oracle_test.rs` pins the answers. Eleven queries compiled through the library and diffed against the pinned 3.53.4 oracle, on a fixture the oracle itself created — the adoption direction that matters. Shapes include SQE's own: `UNION`, `UNION ALL`, `LIMIT 1` existence probes that hit and miss, a join, aggregates. I had checked four queries by hand while doing the lift, which is worth nothing once the terminal closes; a refactor of the path every `sqlite-rs query` takes deserves a check that runs in CI. Mutation-verified that it really talks to the oracle: perturbing integer rendering fails it. `SelectOutcome` gains `derive(Debug)`. It is public and a consumer matches on it, both payloads already derive it, and without it `unwrap`/`expect_err` on a `Result` will not compile — which is a papercut for every caller, not just these tests. Also corrected: `tests/performance/v6.rs`'s comment explaining why that bench duplicates the compile pipeline said the real function "is `pub(crate)` to the binary crate and not callable from an external test/bench crate". The lift made that false. The comment now says so and points at the duplication as removable — deliberately not removed here, since replacing it changes the path this bench measures and that belongs in a change whose bench numbers are the point. One test failure worth recording: `result_column_names_honours_aliases` was written as `SELECT a AS first, b AS second` and failed to parse. Not a bad test — `FIRST` is one of 89 keywords we reserve that SQLite treats as an identifier (`parse.y:272`'s `%fallback ID`). Filed as #696, which also blocks SQE outright: its `iceberg_namespace_properties` has a `key` column, and we cannot create or read that table at all. The test here uses non-keyword aliases and the keyword case is pinned in #696 rather than smuggled in. Verified: 1572 unit (1562 + 10), 388 corpus (387 + 1), 15 sqllogictest, `make lint` clean both passes, `make check-mod-files`, assurance 86/86 and 276/276 with no dead links. Refs: 013/Req-3, #695, #696 Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 4 + src/codegen/prepare.rs | 1 + tests/corpus/main.rs | 1 + tests/corpus/prepare_oracle_test.rs | 157 +++++++++++++++++++ tests/performance/v6.rs | 13 +- tests/unit/prepare_test.rs | 224 ++++++++++++++++++++++++++++ 6 files changed, 395 insertions(+), 5 deletions(-) create mode 100644 tests/corpus/prepare_oracle_test.rs create mode 100644 tests/unit/prepare_test.rs diff --git a/Cargo.toml b/Cargo.toml index 99457437..177e7bad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -132,6 +132,10 @@ path = "tests/unit/vdbe_integrity_check_test.rs" name = "vdbe_streaming_execution" path = "tests/unit/vdbe_streaming_execution_test.rs" +[[test]] +name = "prepare" +path = "tests/unit/prepare_test.rs" + [[test]] name = "codegen_expr" path = "tests/unit/codegen_expr_test.rs" diff --git a/src/codegen/prepare.rs b/src/codegen/prepare.rs index ad6b3407..efa90608 100644 --- a/src/codegen/prepare.rs +++ b/src/codegen/prepare.rs @@ -39,6 +39,7 @@ use crate::vdbe::Program; /// What [`compile_select_program`] produced: either `EXPLAIN QUERY PLAN`'s rows /// (nothing further to compile — there is no bytecode to run) or an /// ordinary compiled [`Program`]. +#[derive(Debug)] pub enum SelectOutcome { /// `EXPLAIN QUERY PLAN` output rows. Eqp(Vec), diff --git a/tests/corpus/main.rs b/tests/corpus/main.rs index e0a60865..b38019f6 100644 --- a/tests/corpus/main.rs +++ b/tests/corpus/main.rs @@ -48,6 +48,7 @@ mod pager_write_test; mod parser_oracle_test; mod partial_sort_test; mod plan_parity_test; +mod prepare_oracle_test; mod regen_test; mod repl_test; mod schema_test; diff --git a/tests/corpus/prepare_oracle_test.rs b/tests/corpus/prepare_oracle_test.rs new file mode 100644 index 00000000..bce600be --- /dev/null +++ b/tests/corpus/prepare_oracle_test.rs @@ -0,0 +1,157 @@ +// Copyright 2026 Schuberg Philis +// SPDX-License-Identifier: Apache-2.0 +//! Oracle diff for the lifted `SELECT` pipeline (#695). +//! +//! `tests/unit/prepare_test.rs` pins the dispatch and the error type. +//! This pins the *answers*: the same queries, compiled through +//! `codegen::compile_select_program` from the library and executed here, +//! must produce byte-identical rows to the pinned `sqlite3` 3.53.4. +//! +//! It exists because the lift's whole claim is "same code, new home". I +//! checked four queries by hand while doing it, which is worth nothing +//! once the terminal closes — a refactor of the path every `sqlite-rs +//! query` takes deserves a check that runs in CI. + +#![allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::indexing_slicing +)] + +use std::collections::HashMap; +use std::process::Command; +use std::rc::Rc; + +use sqlite_rs::codegen::{compile_select_program, result_column_names, SelectOutcome}; +use sqlite_rs::parser::error::ParseOutcome; +use sqlite_rs::parser::parse_select; +use sqlite_rs::record::Value; +use sqlite_rs::schema::{read_schema, read_views}; +use sqlite_rs::vdbe::execute_with_db; +use sqlite_rs::vfs::PageSource; + +use crate::oracle::{pinned_oracle, skip_no_oracle}; + +const SETUP: &[&str] = &[ + "CREATE TABLE t(a INTEGER, b TEXT)", + "CREATE TABLE u(a INTEGER, c TEXT)", + "CREATE INDEX t_a ON t(a)", + "INSERT INTO t VALUES (1,'x'),(2,'y'),(3,'z')", + "INSERT INTO u VALUES (1,'p'),(3,'q')", +]; + +/// One per `compile_select_program` dispatch arm, plus the shapes SQE's +/// own statement list uses (`UNION`, `LIMIT 1` probes). +const QUERIES: &[&str] = &[ + "SELECT 1 + 1", + "SELECT a, b FROM t ORDER BY a", + "SELECT count(*) FROM t", + "SELECT a FROM t WHERE a > 1 ORDER BY a", + "SELECT b FROM t WHERE a = 2", + "SELECT t.a, u.c FROM t JOIN u ON t.a = u.a ORDER BY t.a", + "SELECT a FROM t UNION ALL SELECT a FROM u ORDER BY a", + "SELECT a FROM t UNION SELECT a FROM u ORDER BY a", + "SELECT b FROM t WHERE a = 1 LIMIT 1", + "SELECT a FROM t WHERE b = 'nope' LIMIT 1", + "SELECT max(a), min(a) FROM t", +]; + +/// Renders a row the way the oracle's default `-list` mode does, so the +/// two are comparable as text. +fn render(row: &[Value]) -> String { + row.iter() + .map(|v| match v { + Value::Null => String::new(), + Value::Integer(i) => i.to_string(), + Value::Real(r) => format!("{r}"), + Value::Text(s) => s.to_string(), + Value::Blob(b) => String::from_utf8_lossy(b).to_string(), + }) + .collect::>() + .join("|") +} + +#[test] +fn lifted_pipeline_answers_match_the_oracle() { + let Some(bin) = pinned_oracle() else { + skip_no_oracle("lifted_pipeline_answers_match_the_oracle"); + return; + }; + let dir = std::env::temp_dir().join(format!("sqlite-rs-prepare-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let db = dir.join("prepare.db"); + std::fs::remove_file(&db).ok(); + + // The oracle builds the fixture, so we are reading a file stock + // SQLite wrote — the adoption direction that matters. + let status = Command::new(&bin) + .arg(&db) + .arg(SETUP.join(";\n")) + .status() + .unwrap(); + assert!(status.success(), "oracle setup failed"); + + let (_vfs, pager, header) = open_readonly(&db); + + for sql in QUERIES { + let theirs = Command::new(&bin).arg(&db).arg(sql).output().unwrap(); + assert!( + theirs.status.success(), + "oracle rejected {sql}: {}", + String::from_utf8_lossy(&theirs.stderr) + ); + let expected = String::from_utf8_lossy(&theirs.stdout) + .trim_end() + .to_string(); + + let select = match parse_select(sql) { + ParseOutcome::Accepted(s) => *s, + other => panic!("{sql} did not parse: {other:?}"), + }; + let (schemas, views) = { + let mut c1 = sqlite_rs::btree::TableCursor::new(&*pager, &header, 1); + let schemas = read_schema(&mut c1, header.text_encoding).unwrap(); + let mut c2 = sqlite_rs::btree::TableCursor::new(&*pager, &header, 1); + let views = read_views(&mut c2, header.text_encoding).unwrap(); + (schemas, views) + }; + let program = + match compile_select_program(&select, false, &schemas, &views, &HashMap::new()) + .unwrap_or_else(|e| panic!("{sql} did not compile: {e}")) + { + SelectOutcome::Program(p) => p, + SelectOutcome::Eqp(_) => panic!("{sql} unexpectedly produced EQP rows"), + }; + + // Column names come from the same function the CLI's headers do, + // so a divergence there is a divergence for both. + let names = result_column_names(&select, &schemas); + assert!(!names.is_empty(), "{sql} derived no column names"); + + let source: Rc = Rc::clone(&pager) as Rc; + let rows = execute_with_db(&program, source, header) + .unwrap_or_else(|e| panic!("{sql} failed to execute: {e}")); + let ours = rows + .iter() + .map(|r| render(r)) + .collect::>() + .join("\n"); + + assert_eq!(ours, expected, "rows diverge for {sql}"); + } + + std::fs::remove_dir_all(&dir).ok(); +} + +fn open_readonly( + path: &std::path::Path, +) -> ( + sqlite_rs::vfs::UnixVfs, + Rc, + sqlite_rs::header::DatabaseHeader, +) { + let vfs = sqlite_rs::vfs::UnixVfs; + let (header, pager) = sqlite_rs::dump::open(&vfs, path).expect("open fixture"); + (vfs, Rc::new(pager), header) +} diff --git a/tests/performance/v6.rs b/tests/performance/v6.rs index 46e6bc6d..5297d395 100644 --- a/tests/performance/v6.rs +++ b/tests/performance/v6.rs @@ -481,11 +481,14 @@ fn open_ours(path: &Path) -> OursFixture { } /// `WITH`/view-expansion-then-resolve-then-compile pipeline mirroring -/// `src/bin/sqlite-rs/query.rs::compile_select_program` (the `sqlite-rs -/// query` CLI's real dispatch) closely enough for this bench's needs — -/// that function itself is `pub(crate)` to the binary crate and not -/// callable from an external test/bench crate, so the CTE-expansion + -/// resolve + single-table/joined dispatch is reproduced here rather than +/// `codegen::compile_select_program` (the `sqlite-rs query` CLI's real +/// dispatch) closely enough for this bench's needs. NOTE: that function +/// was `pub(crate)` to the binary crate when this duplicate was written, +/// which is why the duplicate exists; #695 lifted it into the library, so +/// this bench can now call the real thing and the duplication should be +/// removed — deliberately not done in the lift's own PR, since it changes +/// the path this bench measures. The CTE-expansion + resolve + +/// single-table/joined dispatch is reproduced here rather than /// reused. No view expansion: none of this file's SQL references a view. fn compile_ours_select(select: &Select, catalog: &[TableSchema]) -> Program { let expanded = expand_with_clause(select); diff --git a/tests/unit/prepare_test.rs b/tests/unit/prepare_test.rs new file mode 100644 index 00000000..9e5a2ecb --- /dev/null +++ b/tests/unit/prepare_test.rs @@ -0,0 +1,224 @@ +// Copyright 2026 Schuberg Philis +// SPDX-License-Identifier: Apache-2.0 +//! `codegen::prepare` — the `SELECT` compile pipeline, as a library +//! function rather than a CLI internal (#695). +//! +//! These exist because the lift made `compile_select_program` and +//! `result_column_names` public API with no direct coverage: everything +//! that exercised them went through the `sqlite-rs` binary, so a +//! regression in the library surface would only have shown up as a CLI +//! failure. The shapes below are the four `compile_select_program` +//! dispatches on (FROM-less, single-table, joined, compound) plus the +//! one error that is about the request rather than the SQL. +//! +//! Row-level agreement with the oracle is +//! `tests/corpus/prepare_oracle_test.rs`'s job; this file is about the +//! dispatch and the error type. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::collections::HashMap; + +use sqlite_rs::codegen::{ + compile_select_program, result_column_names, PrepareError, SelectOutcome, +}; +use sqlite_rs::parser::error::ParseOutcome; +use sqlite_rs::parser::{parse_explain, parse_select}; +use sqlite_rs::planner::Stats; +use sqlite_rs::record::Collation; +use sqlite_rs::schema::{TableSchema, ViewSchema}; + +/// Two tables, declared the way `ddl_reader` would report them. +fn catalog() -> Vec { + vec![table("t", &["a", "b"]), table("u", &["a", "c"])] +} + +fn table(name: &str, columns: &[&str]) -> TableSchema { + TableSchema { + name: name.to_string(), + root_page: 2, + columns: columns.iter().map(|c| (*c).to_string()).collect(), + column_types: columns.iter().map(|_| "INTEGER".to_string()).collect(), + column_collations: columns.iter().map(|_| Collation::Binary).collect(), + without_rowid: false, + strict: false, + is_virtual: false, + sql: format!("CREATE TABLE {name}({})", columns.join(", ")), + indexes: vec![], + rowid_alias: None, + unresolved_autoindex: false, + } +} + +fn select_of(sql: &str) -> sqlite_rs::parser::ast::Select { + match parse_select(sql) { + ParseOutcome::Accepted(s) => *s, + other => panic!("{sql} did not parse: {other:?}"), + } +} + +fn compile(sql: &str) -> Result { + let select = select_of(sql); + compile_select_program(&select, false, &catalog(), &[], &HashMap::new()) +} + +fn program_of(sql: &str) -> sqlite_rs::vdbe::Program { + match compile(sql).unwrap_or_else(|e| panic!("{sql} did not compile: {e}")) { + SelectOutcome::Program(p) => p, + SelectOutcome::Eqp(_) => panic!("{sql} produced EQP rows, not a program"), + } +} + +#[test] +fn compiles_all_four_select_shapes() { + // The dispatch inside `compile_select_program`, one case each. A + // non-empty program is the assertion: each arm reaches a different + // compiler, and a silent fallthrough would come back empty or error. + for sql in [ + "SELECT 1 + 1", // FROM-less (#260) + "SELECT a, b FROM t WHERE a > 1", // single-table + "SELECT t.a, u.c FROM t JOIN u ON t.a = u.a", // joined (#237) + "SELECT a FROM t UNION ALL SELECT a FROM u", // compound (#240) + ] { + let program = program_of(sql); + assert!( + !program.instructions.is_empty(), + "{sql} compiled to an empty program" + ); + } +} + +#[test] +fn explain_query_plan_returns_rows_not_a_program() { + let sql = "EXPLAIN QUERY PLAN SELECT a FROM t WHERE a > 1"; + let select = match parse_explain(sql) { + ParseOutcome::Accepted(e) => e, + other => panic!("{sql} did not parse: {other:?}"), + }; + let outcome = + compile_select_program(&select.select, true, &catalog(), &[], &HashMap::new()).unwrap(); + match outcome { + SelectOutcome::Eqp(rows) => assert!(!rows.is_empty(), "EQP produced no rows"), + SelectOutcome::Program(_) => panic!("eqp_mode returned a program"), + } +} + +/// The one error that is about the *request* rather than the SQL, and +/// the reason `PrepareError` exists rather than reusing `CodegenError`: +/// `SELECT 1` compiles perfectly well, it just has no access path to +/// explain. +#[test] +fn eqp_without_from_is_its_own_error() { + let select = select_of("SELECT 1"); + let err = compile_select_program(&select, true, &catalog(), &[], &HashMap::new()) + .expect_err("EQP on a FROM-less SELECT should fail"); + + assert_eq!(err, PrepareError::EqpWithoutFrom); + // Byte-identical to the string the CLI printed before the lift. + assert_eq!(err.to_string(), "EXPLAIN QUERY PLAN requires a FROM clause"); + + // And the same statement without `eqp_mode` still compiles, which is + // what makes this distinct from `CodegenError::NoFromClause`. + assert!(matches!( + compile_select_program(&select, false, &catalog(), &[], &HashMap::new()), + Ok(SelectOutcome::Program(_)) + )); +} + +#[test] +fn a_missing_table_surfaces_as_a_codegen_error_not_a_string() { + let select = select_of("SELECT a FROM nonexistent"); + let err = compile_select_program(&select, false, &catalog(), &[], &HashMap::new()) + .expect_err("unknown table should fail"); + + // The point of the lift's error change: a caller can match on the + // variant instead of parsing a message. + assert!( + matches!(err, PrepareError::Codegen(_)), + "expected a wrapped CodegenError, got {err:?}" + ); + assert!( + err.to_string().contains("nonexistent"), + "message should name the table: {err}" + ); +} + +#[test] +fn result_column_names_uses_real_names_for_a_single_table() { + let select = select_of("SELECT a, b FROM t"); + assert_eq!(result_column_names(&select, &catalog()), vec!["a", "b"]); +} + +#[test] +fn result_column_names_honours_aliases() { + // Non-keyword aliases on purpose. `AS first` is what this test used + // first, and it failed — not because aliasing is broken but because + // `FIRST` is one of the 89 keywords we reserve that SQLite treats as + // an identifier (#696). That is a parser bug, not a `prepare` bug, + // so it is pinned there rather than smuggled in here. + let select = select_of("SELECT a AS alpha, b AS beta FROM t"); + assert_eq!( + result_column_names(&select, &catalog()), + vec!["alpha", "beta"] + ); +} + +/// Joins and compounds fall back to positional names. Asserted rather +/// than left implicit because `Row::get_by_name` will be built on this, +/// and "column1" is a real answer a consumer can receive — not a bug. +#[test] +fn result_column_names_falls_back_positionally_for_joins_and_compounds() { + let joined = select_of("SELECT t.a, u.c FROM t JOIN u ON t.a = u.a"); + assert_eq!( + result_column_names(&joined, &catalog()), + vec!["column1", "column2"] + ); + + let compound = select_of("SELECT a FROM t UNION ALL SELECT a FROM u"); + assert_eq!(result_column_names(&compound, &catalog()), vec!["column1"]); +} + +/// An unknown table makes name derivation fall back rather than panic — +/// `result_column_names` returns names, not a `Result`, so it has to +/// have an answer for every input. +#[test] +fn result_column_names_falls_back_when_the_table_is_unknown() { + let select = select_of("SELECT a, b FROM nonexistent"); + assert_eq!( + result_column_names(&select, &catalog()), + vec!["column1", "column2"] + ); +} + +/// `views` is threaded through the pipeline; an empty catalog must not +/// make a plain `SELECT` fail. +#[test] +fn an_empty_view_catalog_is_fine() { + let select = select_of("SELECT a FROM t"); + let views: Vec = vec![]; + assert!(matches!( + compile_select_program(&select, false, &catalog(), &views, &HashMap::new()), + Ok(SelectOutcome::Program(_)) + )); +} + +/// Stats are consulted for the single-table path; a table with no +/// `sqlite_stat1` row must compile the same as one with stats present. +#[test] +fn missing_stats_compile_the_same_as_present_stats() { + let select = select_of("SELECT a FROM t WHERE a > 1"); + let without = program_of("SELECT a FROM t WHERE a > 1"); + + let mut stats = HashMap::new(); + stats.insert("t".to_string(), Stats::default()); + let with = match compile_select_program(&select, false, &catalog(), &[], &stats).unwrap() { + SelectOutcome::Program(p) => p, + SelectOutcome::Eqp(_) => panic!("unexpected EQP"), + }; + + assert_eq!( + without.instructions.len(), + with.instructions.len(), + "default stats should not change the plan" + ); +}