diff --git a/.openspec/adr/0046-sqlite-master-is-a-resolvable-table.md b/.openspec/adr/0046-sqlite-master-is-a-resolvable-table.md new file mode 100644 index 0000000..8dad3a7 --- /dev/null +++ b/.openspec/adr/0046-sqlite-master-is-a-resolvable-table.md @@ -0,0 +1,88 @@ +# 0046: `sqlite_master`/`sqlite_schema` is a resolvable table, not a synthetic CLI result set + +Date: 2026-09-11 + +## Context + +#707: `SELECT ... FROM sqlite_master` didn't compile at all — +`cannot compile statement: unsupported: no such table: sqlite_master`. +`read_schema` decodes the catalog every statement compiles against, +but nothing made that catalog reachable as a queryable *table* from the +`SELECT` path, so a consumer had no way to discover what indexes exist +on a table (`Connection::table_names()`, added as a stopgap, only +covers table names). + +Two routes were on the table: + +1. Make `sqlite_master` a resolvable table — teach + `resolve_from_table_schema` (`src/codegen/subquery/from_clause.rs`) + about it, so `SELECT`/`WHERE`/`ORDER BY` all work on it exactly like + any other table. +2. Add an index-listing method to the embedding API + (`Connection::indexes(table)`), reading `TableSchema::indexes` + directly — no compiler change at all. + +ADR-0029 (introspection pragmas outside the VDBE) explicitly named +this fork in its "Consequences" section: it chose synthetic, +CLI-layer-only result sets for 9 fixed-shape pragmas specifically +*because* none of them needed real `WHERE`/`JOIN` composability at the +time, and said the virtual-table-style alternative should be revisited +"if a real need for querying pragma output as a table... arrives" — +with an explicit instruction to supersede, not edit, when that +happens. + +## Decision + +Route 1. `sqlite_master` (and its modern alias `sqlite_schema`) is a +real b-tree at a well-known root page (1), with a fixed five-column +shape (`type`, `name`, `tbl_name`, `rootpage`, `sql`) — not a +CLI-synthesized result set like the 9 pragmas ADR-0029 covers. +`resolve_from_table_schema` now recognizes the name and hands back a +hardcoded `TableSchema` rooted at page 1 instead of consulting the +decoded catalog (which never contains an entry for itself — it +describes the *objects* on page 1, not the page itself). Every other +part of codegen treats the result exactly like an ordinary table scan: +no new opcode, no synthesized rows, `WHERE type = 'index'` and +`ORDER BY name` fall out for free. + +This supersedes ADR-0029's problem statement only for `sqlite_master` +specifically — the 9 read-only pragmas ADR-0029 covers +(`table_info`, `index_list`, etc.) are unaffected and still live at the +CLI layer; they were never in this ticket's scope (spec 013's +non-goals explicitly defer the rest of the introspection pragma +catalogue to plan.md V7). + +Route 2 (an `indexes()` API method) was not added: route 1 subsumes it +for any consumer willing to write SQL, and per the ticket's own +framing, growing the compiler to handle a real table beats growing the +embedding API's surface for one more read-only accessor. + +## Alternatives rejected + +- **Route 2 alone** (an API-level `indexes()` method): smaller, but + narrower — it only answers the one question the consumer asked + about (indexes), while route 1 also makes `sqlite_master.sql`, + `.rootpage`, and arbitrary `WHERE`/`JOIN` composition against the + catalog available, matching what every other SQLite consumer already + expects to be able to do. +- **Synthesizing `sqlite_master`'s rows in memory** (CLI/API-layer, + same shape as ADR-0029's 9 pragmas) rather than resolving it as a + literal table scan over its real root page. Rejected: `sqlite_master` + is unlike the 9 pragmas in one load-bearing way — it already *is* an + ordinary rowid table on disk, so treating it as one is less code, not + more, and it makes the existing table-scan/`WHERE`/`ORDER BY` + machinery apply automatically instead of needing its own filtering + logic re-implemented. + +## Consequences + +- `resolve_from_table_schema` is the single choke point every `FROM` + reference goes through (top-level `SELECT`, subqueries, `EXPLAIN + QUERY PLAN`, the CLI's own ad hoc lookups) — recognizing the name + once there covers all of them without hunting down each call site. +- `sqlite_stat1` remains out of scope (non-goal, already read + internally for planning) and is not made resolvable by this change. +- A future ticket that wants `pragma_table_info('t')`-style queryable + pragmas is still the virtual-table alternative ADR-0029 deferred — + this ADR doesn't reopen that question, it only answers it for the + one table that was already a real b-tree. diff --git a/.openspec/adr/index.md b/.openspec/adr/index.md index ab53a47..898bf2b 100644 --- a/.openspec/adr/index.md +++ b/.openspec/adr/index.md @@ -45,3 +45,4 @@ Specs record what the system must do; ADRs record **why it is shaped this way** | [0039](0039-value-payloads-are-arc-not-rc.md) | `Value`'s text and blob payloads are `Arc`, not `Rc` | 2026-09-04 | | [0040](0040-streaming-execution-with-batch-as-wrapper.md) | One streaming execution primitive, with the batch path as its wrapper | 2026-09-01 | | [0041](0041-embedding-api-owns-the-connection-driver-out-of-tree.md) | The embedding API owns the connection; the `sqlx` driver stays out of tree | 2026-08-28 | +| [0046](0046-sqlite-master-is-a-resolvable-table.md) | `sqlite_master`/`sqlite_schema` is a resolvable table, not a synthetic CLI result set | 2026-09-11 | diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c10fe2..bfc46db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,15 @@ All notable changes to sqlite-rs. Format follows [Keep a Changelog](https://keep in one statement, where the post-sort pseudo cursor previously tried to re-issue `Rowid` against itself (#708). +- `SELECT ... FROM sqlite_master` (and its modern alias + `sqlite_schema`) didn't compile at all — the decoded catalog was + never reachable as a queryable table. `sqlite_master` is a real + b-tree at page 1 with a fixed five-column shape, so it's now a + resolvable table like any other: `WHERE type = 'index'`, + `ORDER BY`, and an empty database all work exactly as the oracle + gives them, with no new opcode or synthesized rows (ADR-0046, + supersedes ADR-0029's problem statement for this one table) (#707). + ## [0.18.10] - 2026-08-31 ### Fixed diff --git a/src/codegen/subquery/from_clause.rs b/src/codegen/subquery/from_clause.rs index 44e5888..7fa3b19 100644 --- a/src/codegen/subquery/from_clause.rs +++ b/src/codegen/subquery/from_clause.rs @@ -149,6 +149,49 @@ fn subquery_result_schema( } } +/// #707: `sqlite_master`/`sqlite_schema` (the modern alias, same +/// table) is a real b-tree rooted at page 1 — `read_schema` decodes +/// it, but never places an entry for *itself* into the catalog it +/// builds, since it isn't one of the objects it describes. So an +/// ordinary catalog lookup never finds it; this hands back a +/// hardcoded schema for it instead, matching the on-disk row shape +/// (`type`, `name`, `tbl_name`, `rootpage`, `sql`) exactly, so the rest +/// of codegen treats it as a completely ordinary table scan — no +/// synthesized rows, no special-cased read path. Root page 1 carries +/// every object's row, autoindexes included, whether or not +/// `read_schema` itself was able to fully parse that row's DDL. +fn sqlite_master_schema(name: &str) -> Option { + if !name.eq_ignore_ascii_case("sqlite_master") && !name.eq_ignore_ascii_case("sqlite_schema") { + return None; + } + Some(TableSchema { + unresolved_autoindex: false, + name: "sqlite_master".to_string(), + root_page: 1, + columns: vec![ + "type".to_string(), + "name".to_string(), + "tbl_name".to_string(), + "rootpage".to_string(), + "sql".to_string(), + ], + column_types: vec![ + "TEXT".to_string(), + "TEXT".to_string(), + "TEXT".to_string(), + "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, + }) +} + /// Resolves `table_ref` to the [`TableSchema`] the rest of codegen /// should treat it as: a real catalog lookup by name, or (#257) the /// synthetic schema describing a `FROM`-subquery's own projected @@ -163,13 +206,18 @@ pub fn resolve_from_table_schema( catalog: &[TableSchema], ) -> Result { match &table_ref.kind { - crate::parser::ast::TableRefKind::Name(name) => catalog - .iter() - .find(|s| s.name.eq_ignore_ascii_case(name)) - .cloned() - .ok_or_else(|| CodegenError::Unsupported { - reason: format!("no such table: {name}"), - }), + crate::parser::ast::TableRefKind::Name(name) => { + if let Some(schema) = sqlite_master_schema(name) { + return Ok(schema); + } + catalog + .iter() + .find(|s| s.name.eq_ignore_ascii_case(name)) + .cloned() + .ok_or_else(|| CodegenError::Unsupported { + reason: format!("no such table: {name}"), + }) + } crate::parser::ast::TableRefKind::Subquery(subquery) => { let table_refs = subquery_own_table_refs(subquery)?; let schemas = resolve_subquery_schemas(&table_refs, catalog)?; diff --git a/tests/corpus/main.rs b/tests/corpus/main.rs index e0a6086..9cb663e 100644 --- a/tests/corpus/main.rs +++ b/tests/corpus/main.rs @@ -53,6 +53,7 @@ mod repl_test; mod schema_test; mod skip_scan_test; mod sql_corpus_test; +mod sqlite_master_test; mod subquery_test; mod transaction_oracle_test; mod union_test; diff --git a/tests/corpus/sqlite_master_test.rs b/tests/corpus/sqlite_master_test.rs new file mode 100644 index 0000000..5a6933f --- /dev/null +++ b/tests/corpus/sqlite_master_test.rs @@ -0,0 +1,129 @@ +// Copyright 2026 Schuberg Philis +// SPDX-License-Identifier: Apache-2.0 +//! #707: `SELECT ... FROM sqlite_master` didn't compile at all — the +//! catalog `read_schema` decodes was never reachable as a *table* from +//! the `SELECT` path. `sqlite_master` is a real b-tree at page 1 with a +//! known five-column shape, so `resolve_from_table_schema` now hands +//! back a hardcoded schema for it and the rest of codegen treats it as +//! an ordinary table scan — no synthesized rows. Oracle-diffed through +//! the `sqlite-rs` CLI's `query` subcommand against the pinned 3.53.4 +//! `sqlite3`. + +use crate::oracle::{pinned_oracle, run_oracle, skip_no_oracle}; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::atomic::{AtomicU64, Ordering}; + +const CLI: &str = env!("CARGO_BIN_EXE_sqlite-rs"); + +fn scratch_db(label: &str, oracle: &Path, setup_sql: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let path = std::env::temp_dir().join(format!( + "sqlite_rs_sqlite_master_test_{}_{n}_{label}.db", + std::process::id() + )); + 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_query(db: &Path, sql: &str) -> String { + let output = Command::new(CLI) + .arg("query") + .arg(db) + .arg(sql) + .output() + .unwrap_or_else(|e| panic!("running sqlite-rs query: {e}")); + assert!( + output.status.success(), + "sqlite-rs query failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).into_owned() +} + +/// The core defect: a plain `SELECT ... FROM sqlite_master`, over a +/// database with tables, an explicit index and an implicit +/// (`UNIQUE`-constraint) autoindex — every row shape `sqlite_master` +/// can hold. +#[test] +fn select_from_sqlite_master_matches_oracle_including_indexes_and_autoindexes() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle( + "select_from_sqlite_master_matches_oracle_including_indexes_and_autoindexes", + ); + return; + }; + let db = scratch_db( + "basic", + &oracle, + "CREATE TABLE t(a INTEGER PRIMARY KEY, b TEXT); \ + CREATE INDEX idx_b ON t(b); \ + CREATE TABLE u(x TEXT UNIQUE);", + ); + let sql = "SELECT type, name, tbl_name, rootpage, sql FROM sqlite_master ORDER BY name"; + let ours = our_query(&db, sql); + let expected = run_oracle(&oracle, &db, &[], sql); + assert_eq!(ours, expected); +} + +/// The consumer's actual index-discovery case: filtering by `type`. +#[test] +fn select_name_where_type_is_index_matches_oracle() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("select_name_where_type_is_index_matches_oracle"); + return; + }; + let db = scratch_db( + "index_filter", + &oracle, + "CREATE TABLE t(a INTEGER PRIMARY KEY, b TEXT); \ + CREATE INDEX idx_b ON t(b); \ + CREATE TABLE u(x TEXT UNIQUE);", + ); + let sql = "SELECT name FROM sqlite_master WHERE type = 'index' ORDER BY name"; + let ours = our_query(&db, sql); + let expected = run_oracle(&oracle, &db, &[], sql); + assert_eq!(ours, expected); +} + +/// `sqlite_schema` is the modern alias for the same table. +#[test] +fn sqlite_schema_alias_resolves_to_the_same_table() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("sqlite_schema_alias_resolves_to_the_same_table"); + return; + }; + let db = scratch_db("alias", &oracle, "CREATE TABLE t(a INTEGER);"); + let sql = "SELECT type, name FROM sqlite_schema ORDER BY name"; + let ours = our_query(&db, sql); + let expected = run_oracle(&oracle, &db, &[], sql); + assert_eq!(ours, expected); +} + +/// An empty database (no user objects at all) returns zero rows, not +/// an error. +#[test] +fn empty_database_returns_zero_rows() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("empty_database_returns_zero_rows"); + return; + }; + // Force a real header to exist even with no surviving objects. + let db = scratch_db( + "empty", + &oracle, + "CREATE TABLE tmp(x); DROP TABLE tmp; VACUUM;", + ); + let sql = "SELECT * FROM sqlite_master"; + let ours = our_query(&db, sql); + let expected = run_oracle(&oracle, &db, &[], sql); + assert_eq!(ours, expected); + assert_eq!(ours, ""); +}