diff --git a/.openspec/specs/001-architecture/spec.md b/.openspec/specs/001-architecture/spec.md index 5691fa5..9866f30 100644 --- a/.openspec/specs/001-architecture/spec.md +++ b/.openspec/specs/001-architecture/spec.md @@ -415,6 +415,14 @@ sqlite-rs MUST be able to extract every stored row from any well-formed SQLite d **Tests:** `src/header.rs::tests::encoding_utf16le`, `src/header.rs::tests::encoding_utf16be`, `src/record/decode.rs::tests::text_utf16le_and_utf16be` +#### Scenario: Table-level PRIMARY KEY(col) rowid alias round-trips regardless of other columns + +- GIVEN a table declaring a single-column, INTEGER-typed table-level `PRIMARY KEY (col)` constraint, whether or not the table has other columns +- WHEN sqlite-rs reads that column back +- THEN it MUST return the stored value (substituted from the rowid), not `NULL` — only a composite table-level key, or a non-INTEGER type, rules out the rowid-alias optimization + +**Tests:** `tests/corpus/rowid_alias_test.rs::table_level_pk_with_other_columns_is_still_rowid_alias`, `tests/corpus/rowid_alias_test.rs::table_level_pk_single_column_is_rowid_alias`, `tests/corpus/rowid_alias_test.rs::table_level_pk_non_integer_is_not_rowid_alias`, `tests/corpus/rowid_alias_test.rs::composite_table_level_pk_is_not_rowid_alias` + #### Scenario: Unknown schema entry degrades gracefully - GIVEN a database containing a virtual table (e.g. FTS5) whose module is unimplemented diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a35d73..c431d7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ 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 + +- A table-level `PRIMARY KEY (col)` rowid alias read back `NULL` once + the table had more than one column: `rowid_alias_from_sql` only + recognized the form when the primary-key column was the table's sole + column. It now matches the constraint's named column against the + column list directly, so a second (or later) column no longer defeats + the rowid-alias optimization — only a composite key or a non-INTEGER + type does (#686). + ## [0.18.10] - 2026-08-31 ### Fixed diff --git a/src/schema/ddl_reader.rs b/src/schema/ddl_reader.rs index b1a8673..f2598d5 100644 --- a/src/schema/ddl_reader.rs +++ b/src/schema/ddl_reader.rs @@ -1094,19 +1094,22 @@ pub fn rowid_alias_from_sql(sql: &str, without_rowid: bool) -> Option { return Some(idx); } } - // The table-level `PRIMARY KEY(col)` form: SQLite only treats this - // as a rowid alias when it names the table's one and only column, - // and that column is INTEGER-typed (a composite key, or a second - // column, rules it out). - if let [only] = columns.as_slice() { - if is_integer_column(only) { - let col_name = column_name(only); - let is_alias = constraints - .iter() - .filter_map(|c| primary_key_single_column(c)) - .any(|pk_col| pk_col.eq_ignore_ascii_case(&col_name)); - if is_alias { - return Some(0); + // The table-level `PRIMARY KEY(col)` form: SQLite treats this as a + // rowid alias when the constraint names exactly one column and that + // column is INTEGER-typed — regardless of how many other columns + // the table has. Only a composite key (naming more than one + // column) rules it out. + if let Some(pk_col) = constraints + .iter() + .find_map(|c| primary_key_single_column(c)) + { + if let Some((idx, def)) = columns + .iter() + .enumerate() + .find(|(_, def)| column_name(def).eq_ignore_ascii_case(&pk_col)) + { + if is_integer_column(def) { + return Some(idx); } } } diff --git a/tests/corpus/main.rs b/tests/corpus/main.rs index e0a6086..383fd1c 100644 --- a/tests/corpus/main.rs +++ b/tests/corpus/main.rs @@ -50,6 +50,7 @@ mod partial_sort_test; mod plan_parity_test; mod regen_test; mod repl_test; +mod rowid_alias_test; mod schema_test; mod skip_scan_test; mod sql_corpus_test; diff --git a/tests/corpus/rowid_alias_test.rs b/tests/corpus/rowid_alias_test.rs new file mode 100644 index 0000000..a11d354 --- /dev/null +++ b/tests/corpus/rowid_alias_test.rs @@ -0,0 +1,150 @@ +// Copyright 2026 Schuberg Philis +// SPDX-License-Identifier: Apache-2.0 +//! #686 acceptance: a table-level `PRIMARY KEY(col)` on a single +//! `INTEGER` column is a rowid alias regardless of how many other +//! columns the table has — only a composite key (naming more than one +//! column) rules it out. Every row of the issue's rule table +//! round-trips against the oracle: the value read back is the actual +//! value, not `NULL`. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::atomic::{AtomicU64, Ordering}; + +use crate::oracle::{pinned_oracle, skip_no_oracle}; + +const CLI: &str = env!("CARGO_BIN_EXE_sqlite-rs"); + +fn scratch_db(label: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "sqlite-rs-rowid-alias-{label}-{}-{n}", + std::process::id() + )); + std::fs::remove_dir_all(&dir).ok(); + std::fs::create_dir_all(&dir).unwrap(); + dir.join("scratch.db") +} + +fn run_query(db: &Path, sql: &str) -> String { + let output = Command::new(CLI) + .arg("query") + .arg(db) + .arg(sql) + .output() + .unwrap_or_else(|e| panic!("running {CLI} query {} {sql:?}: {e}", db.display())); + assert!( + output.status.success(), + "query {sql:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).into_owned() +} + +fn oracle_select(oracle: &Path, db: &Path, sql: &str) -> String { + let output = Command::new(oracle) + .arg(db) + .arg(sql) + .output() + .unwrap_or_else(|e| panic!("running oracle on {}: {e}", db.display())); + assert!( + output.status.success(), + "oracle query {sql:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).into_owned() +} + +fn oracle_exec(oracle: &Path, db: &Path, sql: &str) -> Output { + Command::new(oracle) + .arg(db) + .arg(sql) + .output() + .unwrap_or_else(|e| panic!("running oracle exec on {}: {e}", db.display())) +} + +/// Runs `ddl` + `insert` against the oracle, then asserts our `SELECT` +/// matches the oracle's `SELECT`. +fn assert_round_trips(label: &str, ddl: &str, insert: &str, select: &str) { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("rowid_alias"); + return; + }; + let db = scratch_db(label); + for stmt in [ddl, insert] { + let output = oracle_exec(&oracle, &db, stmt); + assert!( + output.status.success(), + "oracle setup {stmt:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + let ours = run_query(&db, select); + let theirs = oracle_select(&oracle, &db, select); + assert_eq!(ours, theirs, "mismatch for {select:?} on {ddl:?}"); +} + +/// Table-level `PRIMARY KEY (a)` over a single INTEGER column is the +/// rowid, whether or not the table has other columns. +#[test] +fn table_level_pk_single_column_is_rowid_alias() { + assert_round_trips( + "single-column", + "CREATE TABLE m0 (a INTEGER, PRIMARY KEY (a))", + "INSERT INTO m0 VALUES (1)", + "SELECT a FROM m0", + ); +} + +/// The issue's headline repro: a second column must not defeat the +/// rowid-alias optimization for a single-column table-level PK. +#[test] +fn table_level_pk_with_other_columns_is_still_rowid_alias() { + assert_round_trips( + "with-other-columns", + "CREATE TABLE m1 (a INTEGER, b TEXT, PRIMARY KEY (a))", + "INSERT INTO m1 VALUES (1, 'x')", + "SELECT a, b FROM m1", + ); +} + +/// A non-INTEGER typed table-level PK is never a rowid alias, so its +/// value must round-trip as an ordinary stored column too. +#[test] +fn table_level_pk_non_integer_is_not_rowid_alias() { + assert_round_trips( + "non-integer", + "CREATE TABLE m2 (a TEXT, b TEXT, PRIMARY KEY (a))", + "INSERT INTO m2 VALUES ('k', 'x')", + "SELECT a, b FROM m2", + ); +} + +/// A composite table-level key is never a rowid alias. +#[test] +fn composite_table_level_pk_is_not_rowid_alias() { + assert_round_trips( + "composite", + "CREATE TABLE m3 (a INTEGER, b TEXT, PRIMARY KEY (a, b))", + "INSERT INTO m3 VALUES (1, 'x')", + "SELECT a, b FROM m3", + ); +} + +/// Control: the column-level form was already correct. +#[test] +fn column_level_pk_is_rowid_alias() { + assert_round_trips( + "column-level", + "CREATE TABLE m4 (a INTEGER PRIMARY KEY, b TEXT)", + "INSERT INTO m4 VALUES (1, 'x')", + "SELECT a, b FROM m4", + ); +} + +// `WITHOUT ROWID` unaffected by this ticket's fix is covered at the +// `rowid_alias_from_sql` unit level (src/dump.rs's +// `rowid_alias_none_for_without_rowid`) rather than here: `WITHOUT +// ROWID` tables store rows in an index b-tree, which this crate's +// reader does not yet support independent of this ticket.