From e4c1bc5723772fef73f8faa652919a63f2833cf5 Mon Sep 17 00:00:00 2001 From: Diederik Siderius Date: Fri, 4 Sep 2026 10:43:01 +0200 Subject: [PATCH 1/3] fix: recover sqlite_autoindex_* keys so writes stop corrupting the file (#685) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing to a table carrying a `sqlite_autoindex_*` returned success and left the index stale. `ddl_reader::index_schema` recovers an index's key columns by parsing its `sqlite_master.sql`, and every autoindex has `sql = NULL`, so it was dropped from `TableSchema::indexes` — the same list that drives both `emit_unique_check` (insert.rs:698) and index maintenance (insert.rs:796). Duplicates were accepted and the index was never updated; the oracle then reported `wrong # of entries in index` and `count(*)` undercounted from the stale index. Autoindex rows are now deferred during the `sqlite_master` walk and their keys recovered from the owning table's own DDL. The numbering rule is oracle-derived (3.51.0), not inferred, and several parts of it are counter-intuitive: - declaration order decides, not primary-key-first — `UNIQUE (c), PRIMARY KEY (a, b)` numbers the UNIQUE `_1`; - column-level `PRIMARY KEY`/`UNIQUE` count as much as table-level; - a rowid-alias primary key gets no index AND consumes no number, so `(a INTEGER PRIMARY KEY, b TEXT UNIQUE)` puts UNIQUE(b) at `_1`; - a `WITHOUT ROWID` primary key gets none — it *is* the table; - redundant constraints collapse: `PRIMARY KEY (a), UNIQUE (a)` is one index, as is `a TEXT PRIMARY KEY UNIQUE`. Safety valve, per spec 010/Req 8 and spec 007/Req 1's hot-journal precedent: an autoindex whose key cannot be recovered sets `TableSchema::unresolved_autoindex`, and INSERT/UPDATE/DELETE codegen refuse rather than write. Failing the statement beats silently producing a corrupt database. Two things found while doing this, both handled deliberately: - The rowid-alias rule needed here is not the one `rowid_alias_from_sql` implements — that function additionally requires the primary key to name the table's *only* column, which is not SQLite's rule and is a live read-correctness bug (#686). This ticket implements the correct rule in a local helper rather than smuggling a crate-wide rowid change into a corruption fix; the helper collapses into a call to the shared one when #686 lands. - Unique-violation messages named the index, not the columns. Stock SQLite says `UNIQUE constraint failed: t.a, t.b, t.c`; we said `t.sqlite_autoindex_t_1`. That divergence pre-existed for *every* unique index, but this fix made it newly reachable for autoindexes with a generated name no caller could act on, so the format now matches the oracle byte-for-byte. No existing test asserted the old format, which is a coverage gap this ticket closes. Verified: 1562 unit tests and 387 corpus tests pass (both unchanged from baseline), clippy/fmt/mod-files clean, assurance still 86/86 and 276/276 with no dead links. New `tests/corpus/autoindex_maintenance_test.rs` covers all three spec 010/Req 8 scenarios plus the numbering rule, the rowid-alias and WITHOUT ROWID cases, and message parity. Not included: emitting `sqlite_autoindex_*` on CREATE TABLE, so a table this crate *creates* with a declared composite key still lacks its index. That is the other half of the corruption story and is filed separately — this half fixes adopting a stock-created file, which is the SQE case. Refs: 010/Req-8, #685, #686, #678 Co-Authored-By: Claude Opus 5 (1M context) --- src/bin/sqlite-rs/pragma_query.rs | 1 + src/bin/sqlite-rs/query.rs | 1 + src/bin/sqlite-rs/readline/completion.rs | 1 + src/codegen/analyze.rs | 1 + src/codegen/ddl/create_index.rs | 1 + src/codegen/ddl/drop_table.rs | 1 + src/codegen/select/aggregate.rs | 1 + src/codegen/select/aggregate/accum.rs | 1 + src/codegen/select/entry.rs | 2 + src/codegen/select/join_access.rs | 1 + src/codegen/select/join_order.rs | 1 + src/codegen/stmt/delete.rs | 14 + src/codegen/stmt/insert.rs | 31 +- src/codegen/stmt/update.rs | 15 + src/codegen/subquery/from_clause.rs | 2 + src/codegen/subquery/scalar.rs | 1 + src/dump.rs | 1 + src/integrity.rs | 1 + src/planner.rs | 1 + src/schema/ddl_reader.rs | 280 ++++++++++++++++- tests/corpus/autoindex_maintenance_test.rs | 348 +++++++++++++++++++++ tests/corpus/main.rs | 1 + tests/performance/compile_path.rs | 1 + tests/performance/point_lookup.rs | 1 + tests/sqllogictest/runner.rs | 1 + 25 files changed, 708 insertions(+), 2 deletions(-) create mode 100644 tests/corpus/autoindex_maintenance_test.rs diff --git a/src/bin/sqlite-rs/pragma_query.rs b/src/bin/sqlite-rs/pragma_query.rs index 3e515b94..ee459907 100644 --- a/src/bin/sqlite-rs/pragma_query.rs +++ b/src/bin/sqlite-rs/pragma_query.rs @@ -453,6 +453,7 @@ mod tests { #[test] fn primary_key_columns_inline_and_table_level() { let mut schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["a".to_string(), "b".to_string()], diff --git a/src/bin/sqlite-rs/query.rs b/src/bin/sqlite-rs/query.rs index 74a5ba9c..a9a4744f 100644 --- a/src/bin/sqlite-rs/query.rs +++ b/src/bin/sqlite-rs/query.rs @@ -100,6 +100,7 @@ pub(crate) fn compile_select_program( 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![], diff --git a/src/bin/sqlite-rs/readline/completion.rs b/src/bin/sqlite-rs/readline/completion.rs index 195e682f..ca4202d9 100644 --- a/src/bin/sqlite-rs/readline/completion.rs +++ b/src/bin/sqlite-rs/readline/completion.rs @@ -170,6 +170,7 @@ mod tests { fn schema(name: &str, columns: &[&str]) -> TableSchema { let columns: Vec = columns.iter().map(|s| s.to_string()).collect(); TableSchema { + unresolved_autoindex: false, name: name.to_string(), root_page: 2, column_types: vec![String::new(); columns.len()], diff --git a/src/codegen/analyze.rs b/src/codegen/analyze.rs index b62500e0..6494f01f 100644 --- a/src/codegen/analyze.rs +++ b/src/codegen/analyze.rs @@ -67,6 +67,7 @@ mod tests { fn table(name: &str, root_page: u32, indexes: Vec) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: name.to_string(), root_page, columns: vec!["a".to_string()], diff --git a/src/codegen/ddl/create_index.rs b/src/codegen/ddl/create_index.rs index 8c8df106..1206c119 100644 --- a/src/codegen/ddl/create_index.rs +++ b/src/codegen/ddl/create_index.rs @@ -97,6 +97,7 @@ mod tests { fn schema() -> TableSchema { TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["a".to_string(), "b".to_string()], diff --git a/src/codegen/ddl/drop_table.rs b/src/codegen/ddl/drop_table.rs index 621dca72..2e85a2df 100644 --- a/src/codegen/ddl/drop_table.rs +++ b/src/codegen/ddl/drop_table.rs @@ -53,6 +53,7 @@ mod tests { fn schema_with_index() -> TableSchema { TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["a".to_string()], diff --git a/src/codegen/select/aggregate.rs b/src/codegen/select/aggregate.rs index 9c6408d8..605b3690 100644 --- a/src/codegen/select/aggregate.rs +++ b/src/codegen/select/aggregate.rs @@ -541,6 +541,7 @@ fn compact_index_map(needed_order: &[usize], schema_len: usize) -> Vec TableSchema { TableSchema { + unresolved_autoindex: false, name: schema.name.clone(), root_page: 0, columns: needed_order diff --git a/src/codegen/select/aggregate/accum.rs b/src/codegen/select/aggregate/accum.rs index 47e573d3..9fbea571 100644 --- a/src/codegen/select/aggregate/accum.rs +++ b/src/codegen/select/aggregate/accum.rs @@ -367,6 +367,7 @@ where let mut synthetic_types = schema.column_types.clone(); synthetic_types.extend(synthetic_names.iter().map(|_| String::new())); let synthetic_schema = TableSchema { + unresolved_autoindex: false, name: schema.name.clone(), root_page: 0, columns: synthetic_columns, diff --git a/src/codegen/select/entry.rs b/src/codegen/select/entry.rs index 046a07d5..99b23064 100644 --- a/src/codegen/select/entry.rs +++ b/src/codegen/select/entry.rs @@ -163,6 +163,7 @@ pub(super) fn compile_select_no_from( em.patch_p2(init_addr, body_start); let no_table = TableSchema { + unresolved_autoindex: false, name: String::new(), root_page: 0, columns: vec![], @@ -460,6 +461,7 @@ pub fn compile_select_compound( // trailing ORDER BY/LIMIT and its terms bind to the compound's // result columns, never to any arm's table columns. let output_schema = TableSchema { + unresolved_autoindex: false, name: String::new(), root_page: 0, columns: output_column_names(first, first_schema), diff --git a/src/codegen/select/join_access.rs b/src/codegen/select/join_access.rs index 82a6eab8..d1fe8011 100644 --- a/src/codegen/select/join_access.rs +++ b/src/codegen/select/join_access.rs @@ -795,6 +795,7 @@ mod tests { fn schema(name: &str, columns: &[&str], indexes: Vec) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: name.to_string(), root_page: 0, columns: columns.iter().map(|c| (*c).to_string()).collect(), diff --git a/src/codegen/select/join_order.rs b/src/codegen/select/join_order.rs index e2c13448..a560e8ac 100644 --- a/src/codegen/select/join_order.rs +++ b/src/codegen/select/join_order.rs @@ -282,6 +282,7 @@ mod tests { fn schema(name: &str, columns: &[&str], indexes: Vec) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: name.to_string(), root_page: 0, columns: columns.iter().map(|c| (*c).to_string()).collect(), diff --git a/src/codegen/stmt/delete.rs b/src/codegen/stmt/delete.rs index bb53f42a..15dfa325 100644 --- a/src/codegen/stmt/delete.rs +++ b/src/codegen/stmt/delete.rs @@ -54,6 +54,20 @@ pub fn compile_delete_with_catalog( reason: "WITHOUT ROWID tables are not supported by DELETE codegen yet".to_string(), }); } + // #685: an on-disk `sqlite_autoindex_*` whose key columns could not + // be recovered from the table's DDL is absent from `schema.indexes`, + // so this codegen would neither enforce its uniqueness nor maintain + // it — the write would report success and leave the index stale. + // Refuse instead, per spec 010/Req 8 and spec 007/Req 1's precedent. + if schema.unresolved_autoindex { + return Err(CodegenError::Unsupported { + reason: format!( + "table {} carries an automatic index this reader could not \ + interpret, so DELETE would corrupt it; the table is read-only", + schema.name + ), + }); + } let mut em = Emitter::new(); let mut reg = RegAlloc::new(); diff --git a/src/codegen/stmt/insert.rs b/src/codegen/stmt/insert.rs index a9f239a9..6c8425a3 100644 --- a/src/codegen/stmt/insert.rs +++ b/src/codegen/stmt/insert.rs @@ -246,6 +246,20 @@ pub fn compile_insert( reason: "WITHOUT ROWID tables are not supported by INSERT codegen yet".to_string(), }); } + // #685: an on-disk `sqlite_autoindex_*` whose key columns could not + // be recovered from the table's DDL is absent from `schema.indexes`, + // so this codegen would neither enforce its uniqueness nor maintain + // it — the write would report success and leave the index stale. + // Refuse instead, per spec 010/Req 8 and spec 007/Req 1's precedent. + if schema.unresolved_autoindex { + return Err(CodegenError::Unsupported { + reason: format!( + "table {} carries an automatic index this reader could not \ + interpret, so INSERT would corrupt it; the table is read-only", + schema.name + ), + }); + } let create = cached_create_table(schema)?; @@ -367,6 +381,7 @@ pub fn compile_insert( // support. Every `CHECK` column reference reads via ordinary // `Opcode::Column` instead. let check_schema = TableSchema { + unresolved_autoindex: false, sql: String::new(), rowid_alias: None, ..schema.clone() @@ -964,7 +979,21 @@ fn emit_unique_check( em.place(seek_ok); } ConflictAction::Abort | ConflictAction::Fail | ConflictAction::Rollback => { - let message = format!("UNIQUE constraint failed: {}.{}", schema.name, index.name); + // Stock SQLite names the *columns*, not the index: + // `UNIQUE constraint failed: t.a, t.b, t.c`. Naming the + // index instead diverged for every unique index (measured + // against 3.51.0), and #685 made it newly reachable for + // autoindexes, whose generated name would be meaningless in + // an application's error log. + let message = format!( + "UNIQUE constraint failed: {}", + index + .columns + .iter() + .map(|col| format!("{}.{}", schema.name, col.name)) + .collect::>() + .join(", ") + ); em.emit(Instruction::with_p4( Opcode::Halt, SQLITE_CONSTRAINT_UNIQUE, diff --git a/src/codegen/stmt/update.rs b/src/codegen/stmt/update.rs index 6785f020..cdd1feca 100644 --- a/src/codegen/stmt/update.rs +++ b/src/codegen/stmt/update.rs @@ -83,6 +83,20 @@ pub fn compile_update_with_catalog( reason: "WITHOUT ROWID tables are not supported by UPDATE codegen yet".to_string(), }); } + // #685: an on-disk `sqlite_autoindex_*` whose key columns could not + // be recovered from the table's DDL is absent from `schema.indexes`, + // so this codegen would neither enforce its uniqueness nor maintain + // it — the write would report success and leave the index stale. + // Refuse instead, per spec 010/Req 8 and spec 007/Req 1's precedent. + if schema.unresolved_autoindex { + return Err(CodegenError::Unsupported { + reason: format!( + "table {} carries an automatic index this reader could not \ + interpret, so UPDATE would corrupt it; the table is read-only", + schema.name + ), + }); + } let create = cached_create_table(schema)?; @@ -105,6 +119,7 @@ pub fn compile_update_with_catalog( // for the rowid-alias column — cleared here alongside `sql`, and which the pseudo-cursor can't // answer). let check_schema = TableSchema { + unresolved_autoindex: false, sql: String::new(), rowid_alias: None, ..schema.clone() diff --git a/src/codegen/subquery/from_clause.rs b/src/codegen/subquery/from_clause.rs index 9b1eb7d0..44e58888 100644 --- a/src/codegen/subquery/from_clause.rs +++ b/src/codegen/subquery/from_clause.rs @@ -134,6 +134,7 @@ fn subquery_result_schema( ) -> TableSchema { let columns = subquery_output_columns(subquery, table_refs, schemas); TableSchema { + unresolved_autoindex: false, name: String::new(), root_page: 0, columns: columns.clone(), @@ -386,6 +387,7 @@ mod tests { fn table(name: &str, root_page: u32) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: name.to_string(), root_page, columns: vec!["a".to_string(), "b".to_string()], diff --git a/src/codegen/subquery/scalar.rs b/src/codegen/subquery/scalar.rs index f29d6ff9..8336e471 100644 --- a/src/codegen/subquery/scalar.rs +++ b/src/codegen/subquery/scalar.rs @@ -696,6 +696,7 @@ mod tests { fn table(name: &str, root_page: u32, columns: &[&str], sql: &str) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: name.to_string(), root_page, columns: columns.iter().map(|c| c.to_string()).collect(), diff --git a/src/dump.rs b/src/dump.rs index 0dfdf632..cd6547fd 100644 --- a/src/dump.rs +++ b/src/dump.rs @@ -324,6 +324,7 @@ mod tests { fn schema(sql: &str) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec![], diff --git a/src/integrity.rs b/src/integrity.rs index 7cbbf54d..a67fa830 100644 --- a/src/integrity.rs +++ b/src/integrity.rs @@ -376,6 +376,7 @@ mod tests { fn table_schema(name: &str, root_page: u32, indexes: Vec) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: name.to_string(), root_page, columns: vec![], diff --git a/src/planner.rs b/src/planner.rs index b5fe8615..de3e05fa 100644 --- a/src/planner.rs +++ b/src/planner.rs @@ -268,6 +268,7 @@ mod tests { .unwrap(); let schemas = vec![TableSchema { + unresolved_autoindex: false, name: "sqlite_stat1".to_string(), root_page: stat1_root, columns: vec!["tbl".to_string(), "idx".to_string(), "stat".to_string()], diff --git a/src/schema/ddl_reader.rs b/src/schema/ddl_reader.rs index c1db7630..b1a86733 100644 --- a/src/schema/ddl_reader.rs +++ b/src/schema/ddl_reader.rs @@ -108,6 +108,16 @@ pub struct TableSchema { /// graceful-degradation rule as unparseable DDL elsewhere in this /// reader (#211). pub indexes: Vec, + /// Set when this table carries a `sqlite_autoindex_*` whose key + /// columns could not be recovered from its own DDL (#685). + /// + /// Writes MUST be refused while this is set. The index exists on + /// disk but is absent from `indexes`, so codegen would neither + /// enforce its uniqueness nor maintain it — the write would succeed + /// and leave the file corrupt. Same argument as spec 007/Req 1's + /// hot-journal refusal: failing the operation beats silently + /// producing a wrong database. + pub unresolved_autoindex: bool, /// The rowid-alias column index (0-based into `columns`), resolved /// once at schema-decode time by [`rowid_alias_from_sql`] — SQLite's /// single-`INTEGER PRIMARY KEY` special case (see @@ -189,6 +199,9 @@ pub fn read_schema_and_views( // a hash lookup instead of an O(indexes × tables) linear scan (#589). let mut table_pos: std::collections::HashMap = std::collections::HashMap::new(); let mut pending_indexes: Vec<(String, IndexSchema)> = Vec::new(); + // (table name, index name, root page) for `sqlite_autoindex_*` rows, + // resolved after the walk because resolution needs the table's DDL. + let mut pending_autoindexes: Vec<(String, String, u32)> = Vec::new(); let mut row = cursor.first_row()?; while let Some(r) = row { let values = decode_record(&r.payload, encoding)?; @@ -201,7 +214,26 @@ pub fn read_schema_and_views( table_pos.insert(schema.name.clone(), schemas.len()); schemas.push(schema); } - "index" => pending_indexes.extend(index_schema(&values)), + "index" => { + if let Some(entry) = index_schema(&values) { + pending_indexes.push(entry); + } else { + // `sql` is NULL (or unparseable). Every autoindex + // SQLite builds for a `PRIMARY KEY`/`UNIQUE` + // constraint has that shape, and dropping it here is + // what corrupts writes (#685) — so defer it and + // recover its key columns from the owning table's + // own DDL below. + pending_autoindexes.push(( + text(values.get(2)).to_string(), + text(values.get(1)).to_string(), + match values.get(3) { + Some(Value::Integer(i)) => u32::try_from(*i).unwrap_or(0), + _ => 0, + }, + )); + } + } "view" => views.push(ViewSchema { name: text(values.get(1)).to_string(), sql: text(values.get(4)).to_string(), @@ -218,6 +250,39 @@ pub fn read_schema_and_views( schema.indexes.push(index); } } + for (table_name, index_name, root_page) in pending_autoindexes { + let Some(&pos) = table_pos.get(&table_name) else { + continue; + }; + // Read the DDL out before taking the mutable borrow. + let Some((sql, without_rowid)) = schemas + .get(pos) + .map(|schema| (schema.sql.clone(), schema.without_rowid)) + else { + continue; + }; + let resolved = autoindex_ordinal(&index_name, &table_name).and_then(|ordinal| { + autoindex_key_lists(&sql, without_rowid) + .and_then(|lists| lists.into_iter().nth(ordinal.saturating_sub(1))) + }); + let Some(schema) = schemas.get_mut(pos) else { + continue; + }; + match resolved { + Some(columns) => schema.indexes.push(IndexSchema { + name: index_name, + // Every `sqlite_autoindex_*` backs a `PRIMARY KEY` or + // `UNIQUE` constraint, so it is always unique. + unique: true, + columns, + root_page, + }), + // Could not recover the key. The index is real and on disk, + // so the table must become read-only rather than writable + // and silently corrupting. + None => schema.unresolved_autoindex = true, + } + } Ok((schemas, views)) } @@ -297,6 +362,7 @@ fn table_schema(values: &[Value]) -> TableSchema { if is_virtual_table(sql) { return TableSchema { + unresolved_autoindex: false, name, root_page: 0, columns: Vec::new(), @@ -314,6 +380,7 @@ fn table_schema(values: &[Value]) -> TableSchema { let parsed = parse_create_table(sql).unwrap_or_default(); let rowid_alias = rowid_alias_from_sql(sql, parsed.without_rowid); TableSchema { + unresolved_autoindex: false, name, root_page, columns: parsed.columns, @@ -328,6 +395,217 @@ fn table_schema(values: &[Value]) -> TableSchema { } } +/// The ordinal `N` in `sqlite_autoindex__` (#685). +fn autoindex_ordinal(index_name: &str, table_name: &str) -> Option { + let prefix = format!("sqlite_autoindex_{table_name}_"); + index_name + .strip_prefix(prefix.as_str())? + .parse::() + .ok() + .filter(|n| *n >= 1) +} + +/// Whether `def` contains `keyword` at paren depth 0, outside quotes. +/// +/// `UNIQUE` inside `CHECK (x <> 'UNIQUE')` or inside a nested key list +/// must not count, which is why this scans the masked copy rather than +/// the raw text. +fn def_has_top_level_keyword(def: &str, keyword: &str) -> bool { + let masked = mask_quotes_and_comments(def); + let kw = keyword.as_bytes(); + let mut depth = 0i32; + let mut i = 0usize; + while i < masked.len() { + match masked.get(i) { + Some(b'(') => depth = depth.saturating_add(1), + Some(b')') => depth = depth.saturating_sub(1), + _ => { + if depth == 0 { + if let Some(window) = masked.get(i..i.saturating_add(kw.len())) { + if window.eq_ignore_ascii_case(kw) { + return true; + } + } + } + } + } + i = i.saturating_add(1); + } + false +} + +/// A table constraint's parenthesised key list as [`IndexedColumn`]s. +fn constraint_key_columns(constraint: &str) -> Option> { + let open = constraint.find('(')?; + let close = constraint.rfind(')')?; + if close <= open { + return None; + } + let inner = constraint.get(open.saturating_add(1)..close)?; + Some( + split_top_level_commas(inner) + .into_iter() + .map(indexed_column) + .collect(), + ) +} + +/// `PRIMARY KEY` / `UNIQUE` / neither, for a table constraint, seeing +/// through an optional `CONSTRAINT ` prefix. `None` means "this +/// reader cannot tell", which the caller must treat as unresolvable +/// rather than as "no constraint". +fn table_constraint_kind(constraint: &str) -> Option> { + let mut rest = constraint.trim(); + if starts_with_ignore_case(rest, "CONSTRAINT") { + // Skip `CONSTRAINT` and the identifier that follows it. + let after = rest.get("CONSTRAINT".len()..)?.trim_start(); + let ident_end = after + .find(|c: char| c.is_whitespace() || c == '(') + .unwrap_or(after.len()); + rest = after.get(ident_end..)?.trim_start(); + } + if starts_with_ignore_case(rest, "PRIMARY KEY") { + return Some(Some(true)); + } + if starts_with_ignore_case(rest, "UNIQUE") { + return Some(Some(false)); + } + if starts_with_ignore_case(rest, "FOREIGN KEY") || starts_with_ignore_case(rest, "CHECK") { + return Some(None); + } + None +} + +/// The rowid-alias column's name, using SQLite's actual rule. +/// +/// Deliberately local rather than reusing [`rowid_alias_from_sql`]: +/// that function's table-level branch additionally requires the primary +/// key to name the table's *only* column, which is not SQLite's rule +/// (measured — `(a INTEGER, b TEXT, PRIMARY KEY (a))` is an alias). That +/// is a real read-correctness bug, but fixing it changes rowid handling +/// crate-wide, so #686 owns it; #685 must not smuggle that change in +/// alongside a corruption fix. When #686 lands, this collapses into a +/// call to the shared helper. +fn rowid_alias_column_name(sql: &str, without_rowid: bool) -> Option { + if without_rowid { + return None; + } + let (start, end) = column_list_span(sql)?; + let inner = sql.get(start..end)?; + let mut columns: Vec<&str> = Vec::new(); + let mut constraints: Vec<&str> = Vec::new(); + for def in split_top_level_commas(inner) { + if is_table_constraint(def) { + constraints.push(def); + } else { + columns.push(def); + } + } + for def in &columns { + if is_integer_primary_key_inline(def) { + return Some(column_name(def)); + } + } + // Table-level `PRIMARY KEY (col)`: an alias whenever it names one + // INTEGER column, however many other columns the table has. + for constraint in &constraints { + if let Some(pk_col) = primary_key_single_column(constraint) { + if columns + .iter() + .any(|def| is_integer_column(def) && column_name(def).eq_ignore_ascii_case(&pk_col)) + { + return Some(pk_col); + } + } + } + None +} + +/// Case-insensitive key-list equality, so a redundant constraint does +/// not claim a second autoindex number. +fn same_key_list(a: &[IndexedColumn], b: &[IndexedColumn]) -> bool { + a.len() == b.len() + && a.iter() + .zip(b.iter()) + .all(|(x, y)| x.name.eq_ignore_ascii_case(&y.name) && x.desc == y.desc) +} + +/// Every key list SQLite creates a `sqlite_autoindex_*` for, in the +/// order it numbers them (#685). +/// +/// The rule is oracle-derived (`sqlite3` 3.51.0), not inferred: +/// +/// * declaration order wins — `UNIQUE (c), PRIMARY KEY (a, b)` numbers +/// the `UNIQUE` first; +/// * column-level `PRIMARY KEY`/`UNIQUE` count as much as table-level; +/// * a rowid-alias primary key gets no index *and consumes no number*; +/// * a `WITHOUT ROWID` table's primary key gets none — it *is* the table; +/// * two constraints over the same key list collapse to one index. +/// +/// `None` means some constraint could not be understood. The caller must +/// treat that as unresolvable and refuse writes, never as "no index". +fn autoindex_key_lists(sql: &str, without_rowid: bool) -> Option>> { + let (start, end) = column_list_span(sql)?; + let inner = sql.get(start..end)?; + let alias = rowid_alias_column_name(sql, without_rowid); + let mut out: Vec> = Vec::new(); + let push = |cols: Vec, out: &mut Vec>| { + if !out.iter().any(|existing| same_key_list(existing, &cols)) { + out.push(cols); + } + }; + + for def in split_top_level_commas(inner) { + let trimmed = def.trim(); + if is_table_constraint(trimmed) { + let kind = table_constraint_kind(trimmed)?; + let Some(is_pk) = kind else { + continue; // FOREIGN KEY / CHECK — no index + }; + if is_pk && without_rowid { + continue; + } + let cols = constraint_key_columns(trimmed)?; + if cols.is_empty() { + return None; + } + let is_alias = is_pk + && cols.len() == 1 + && alias + .as_deref() + .is_some_and(|a| cols.first().is_some_and(|c| c.name.eq_ignore_ascii_case(a))); + if is_alias { + continue; + } + push(cols, &mut out); + } else { + let name = column_name(trimmed); + if name.is_empty() { + continue; + } + // A column-level constraint's index inherits the column's + // declared `COLLATE`, which uniqueness checking depends on: + // two byte-distinct values can be collation-equal, and the + // autoindex SQLite built used the column's collation. + let col = IndexedColumn { + name: name.clone(), + desc: false, + collation: column_collation(trimmed), + }; + let is_alias = alias + .as_deref() + .is_some_and(|a| a.eq_ignore_ascii_case(&name)); + if def_has_top_level_keyword(trimmed, "PRIMARY KEY") && !is_alias && !without_rowid { + push(vec![col.clone()], &mut out); + } + if def_has_top_level_keyword(trimmed, "UNIQUE") { + push(vec![col], &mut out); + } + } + } + Some(out) +} + /// Parses a `sqlite_master` row with `type = 'index'` into the owning /// table's name and a naive [`IndexSchema`]. Returns `None` for anything /// this naive reader can't find a column list in — most notably diff --git a/tests/corpus/autoindex_maintenance_test.rs b/tests/corpus/autoindex_maintenance_test.rs new file mode 100644 index 00000000..fb334471 --- /dev/null +++ b/tests/corpus/autoindex_maintenance_test.rs @@ -0,0 +1,348 @@ +// Copyright 2026 Schuberg Philis +// SPDX-License-Identifier: Apache-2.0 +//! #685 / spec 010 Req 8 acceptance: a write to a table carrying a +//! `sqlite_autoindex_*` must maintain that index or refuse the write — +//! never succeed and leave it stale. +//! +//! Every autoindex has `sql = NULL` in `sqlite_master`, so +//! `ddl_reader::index_schema` used to drop it, and the same +//! `TableSchema::indexes` list drives both uniqueness checking and index +//! maintenance in `codegen/stmt/insert.rs`. The result was a successful +//! write that left `PRAGMA integrity_check` reporting missing rows and +//! `count(*)` answering from the stale index. +//! +//! The numbering rule these tests pin down is oracle-derived, not +//! inferred — see #685 for the measurement table. Declaration order +//! wins, column-level constraints count, rowid-alias and `WITHOUT ROWID` +//! primary keys get nothing, and redundant constraints collapse. + +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::atomic::{AtomicU64, Ordering}; + +use sqlite_rs::btree::TableCursor; +use sqlite_rs::codegen::compile_insert; +use sqlite_rs::header::DatabaseHeader; +use sqlite_rs::pager::Pager; +use sqlite_rs::parser::{parse_insert, ParseOutcome}; +use sqlite_rs::schema::{read_schema, TableSchema}; +use sqlite_rs::vdbe::execute_with_writable_db; +use sqlite_rs::vfs::{PageSource, UnixVfs, Vfs, VfsPageSource}; + +use crate::oracle::{assert_integrity_check_ok, pinned_oracle, skip_no_oracle}; + +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-autoindex-{label}-{}-{n}", + std::process::id() + )); + std::fs::create_dir_all(&dir).unwrap(); + dir.join("test.db") +} + +fn seed(oracle: &PathBuf, db: &PathBuf, sql: &str) { + let status = Command::new(oracle).arg(db).arg(sql).status().unwrap(); + assert!(status.success()); +} + +fn page_size_of(db: &Path) -> u32 { + let vfs = UnixVfs; + let file = vfs.open_read(db).unwrap(); + let mut header_buf = [0u8; 100]; + file.read_at(&mut header_buf, 0).unwrap(); + let page_size = u16::from_be_bytes([header_buf[16], header_buf[17]]) as u32; + if page_size == 1 { + 65536 + } else { + page_size + } +} + +fn read_header(db: &Path, page_size: u32) -> DatabaseHeader { + let vfs = UnixVfs; + let pager = Pager::open(&vfs, db, page_size).unwrap(); + let raw = pager.read_page(1).unwrap(); + let mut buf = [0u8; 100]; + buf.copy_from_slice(&raw[..100]); + DatabaseHeader::parse(&buf).unwrap() +} + +fn table_schema(db: &Path, header: &DatabaseHeader, table: &str) -> TableSchema { + let vfs = UnixVfs; + let source = VfsPageSource::open(&vfs, db, header.page_size).unwrap(); + let mut cursor = TableCursor::new(source, header, 1); + let schemas = read_schema(&mut cursor, header.text_encoding).unwrap(); + schemas + .into_iter() + .find(|s| s.name == table) + .unwrap_or_else(|| panic!("no schema for table {table}")) +} + +fn oracle_select(oracle: &PathBuf, db: &PathBuf, sql: &str) -> String { + let out = Command::new(oracle) + .arg("-readonly") + .arg("-list") + .arg(db) + .arg(sql) + .output() + .unwrap(); + assert!(out.status.success()); + String::from_utf8_lossy(&out.stdout).trim().to_string() +} + +/// Runs one `INSERT` through our own parse -> codegen -> VDBE path. +fn our_insert(db: &PathBuf, table: &str, sql: &str) -> Result<(), String> { + let page_size = page_size_of(db); + let header = read_header(db, page_size); + let schema = table_schema(db, &header, table); + let insert = match parse_insert(sql) { + ParseOutcome::Accepted(i) => *i, + other => panic!("failed to parse {sql}: {other:?}"), + }; + let program = compile_insert(&insert, &schema, None).map_err(|e| format!("{e:?}"))?; + let vfs = UnixVfs; + let pager = Pager::open(&vfs, db, page_size).unwrap(); + execute_with_writable_db(&program, pager, header) + .map(|_| ()) + .map_err(|e| format!("{e:?}")) +} + +/// The name -> key-columns mapping our reader recovered, for asserting +/// the numbering rule. +fn autoindex_map(db: &PathBuf, table: &str) -> Vec<(String, Vec)> { + let page_size = page_size_of(db); + let header = read_header(db, page_size); + let schema = table_schema(db, &header, table); + let mut out: Vec<(String, Vec)> = schema + .indexes + .iter() + .map(|i| { + ( + i.name.clone(), + i.columns.iter().map(|c| c.name.clone()).collect(), + ) + }) + .collect(); + out.sort(); + out +} + +/// Spec 010 Req 8: "A write to a stock-created composite-PK table keeps +/// the index consistent." +#[test] +fn stock_composite_pk_stays_consistent() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("autoindex_maintenance"); + return; + }; + let db = scratch_db("consistent"); + seed( + &oracle, + &db, + "CREATE TABLE t (a TEXT NOT NULL, b TEXT NOT NULL, c TEXT NOT NULL, v TEXT, \ + PRIMARY KEY (a, b, c)); \ + INSERT INTO t VALUES ('c','ns','t1','v1');", + ); + + // The reader must now see the autoindex at all — this is the + // regression the whole ticket is about. + let map = autoindex_map(&db, "t"); + assert_eq!( + map, + vec![( + "sqlite_autoindex_t_1".to_string(), + vec!["a".to_string(), "b".to_string(), "c".to_string()] + )], + "the composite PRIMARY KEY's autoindex was not recovered" + ); + + our_insert(&db, "t", "INSERT INTO t VALUES ('c','ns','t2','v2')").expect("insert should land"); + + assert_integrity_check_ok(&oracle, &db); + assert_eq!(oracle_select(&oracle, &db, "SELECT count(*) FROM t"), "2"); +} + +/// Spec 010 Req 8: "A duplicate against an autoindex-backed constraint +/// is refused." +#[test] +fn autoindex_duplicate_is_refused() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("autoindex_maintenance"); + return; + }; + let db = scratch_db("duplicate"); + seed( + &oracle, + &db, + "CREATE TABLE t (a TEXT NOT NULL, b TEXT NOT NULL, c TEXT NOT NULL, v TEXT, \ + PRIMARY KEY (a, b, c)); \ + INSERT INTO t VALUES ('c','ns','t1','v1');", + ); + + let err = our_insert(&db, "t", "INSERT INTO t VALUES ('c','ns','t1','dup')") + .expect_err("a duplicate composite key must be refused"); + assert!( + err.contains("UNIQUE"), + "expected a UNIQUE constraint error, got {err}" + ); + + assert_integrity_check_ok(&oracle, &db); + assert_eq!(oracle_select(&oracle, &db, "SELECT count(*) FROM t"), "1"); +} + +/// Spec 010 Req 8: "A named index is unaffected." The control that +/// proves the fix did not achieve consistency by disabling writes. +#[test] +fn named_index_round_trips() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("autoindex_maintenance"); + return; + }; + let db = scratch_db("named"); + seed( + &oracle, + &db, + "CREATE TABLE t (x TEXT, y TEXT); \ + CREATE UNIQUE INDEX u_xy ON t(x, y); \ + INSERT INTO t VALUES ('a','b');", + ); + + our_insert(&db, "t", "INSERT INTO t VALUES ('a','c')").expect("new key should land"); + let err = our_insert(&db, "t", "INSERT INTO t VALUES ('a','b')") + .expect_err("duplicate should be refused"); + assert!(err.contains("UNIQUE"), "got {err}"); + + assert_integrity_check_ok(&oracle, &db); + assert_eq!(oracle_select(&oracle, &db, "SELECT count(*) FROM t"), "2"); +} + +/// A rowid alias gets no autoindex, so the reader must not invent one — +/// a phantom entry would make codegen open a cursor on a root page that +/// holds table data. +#[test] +fn rowid_alias_gains_no_phantom_index() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("autoindex_maintenance"); + return; + }; + let db = scratch_db("rowid-alias"); + seed( + &oracle, + &db, + "CREATE TABLE inline (a INTEGER PRIMARY KEY, b TEXT); \ + CREATE TABLE tabled (a INTEGER, b TEXT, PRIMARY KEY (a)); \ + CREATE TABLE composite (a INTEGER, b TEXT, PRIMARY KEY (a, b));", + ); + + assert!( + autoindex_map(&db, "inline").is_empty(), + "INTEGER PRIMARY KEY is the rowid; no index exists" + ); + assert!( + autoindex_map(&db, "tabled").is_empty(), + "table-level PRIMARY KEY(a) on an INTEGER column is also the rowid" + ); + // A composite key over an INTEGER column is *not* a rowid alias. + assert_eq!( + autoindex_map(&db, "composite"), + vec![( + "sqlite_autoindex_composite_1".to_string(), + vec!["a".to_string(), "b".to_string()] + )] + ); +} + +/// `WITHOUT ROWID` stores rows in the primary-key b-tree itself, so the +/// primary key gets no separate index — but a `UNIQUE` still does. +#[test] +fn without_rowid_primary_key_gains_no_index() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("autoindex_maintenance"); + return; + }; + let db = scratch_db("without-rowid"); + seed( + &oracle, + &db, + "CREATE TABLE w (a TEXT, b TEXT, c TEXT, UNIQUE (c), PRIMARY KEY (a, b)) WITHOUT ROWID;", + ); + assert_eq!( + autoindex_map(&db, "w"), + vec![("sqlite_autoindex_w_1".to_string(), vec!["c".to_string()])], + "only the UNIQUE gets an index under WITHOUT ROWID" + ); +} + +/// The numbering rule, which is the part most easily got wrong: +/// declaration order decides, not primary-key-first. +#[test] +fn autoindex_numbering_follows_declaration_order() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("autoindex_maintenance"); + return; + }; + let db = scratch_db("numbering"); + seed( + &oracle, + &db, + // UNIQUE declared *before* the PRIMARY KEY. + "CREATE TABLE ord (a TEXT, b TEXT, c TEXT, UNIQUE (c), PRIMARY KEY (a, b)); \ + CREATE TABLE lvl (a TEXT PRIMARY KEY, b TEXT UNIQUE); \ + CREATE TABLE skip (a INTEGER PRIMARY KEY, b TEXT UNIQUE); \ + CREATE TABLE dup (a TEXT, b TEXT, PRIMARY KEY (a), UNIQUE (a));", + ); + + assert_eq!( + autoindex_map(&db, "ord"), + vec![ + ("sqlite_autoindex_ord_1".to_string(), vec!["c".to_string()]), + ( + "sqlite_autoindex_ord_2".to_string(), + vec!["a".to_string(), "b".to_string()] + ), + ], + "declaration order must win over primary-key-first" + ); + assert_eq!( + autoindex_map(&db, "lvl"), + vec![ + ("sqlite_autoindex_lvl_1".to_string(), vec!["a".to_string()]), + ("sqlite_autoindex_lvl_2".to_string(), vec!["b".to_string()]), + ], + "column-level constraints get autoindexes too" + ); + assert_eq!( + autoindex_map(&db, "skip"), + vec![("sqlite_autoindex_skip_1".to_string(), vec!["b".to_string()])], + "a rowid-alias PK consumes no number, so UNIQUE(b) is _1" + ); + assert_eq!( + autoindex_map(&db, "dup"), + vec![("sqlite_autoindex_dup_1".to_string(), vec!["a".to_string()])], + "PRIMARY KEY(a) and UNIQUE(a) collapse to one index" + ); +} + +/// The error message must name the columns, as stock SQLite does, not +/// the generated index name — which would be meaningless to a caller. +#[test] +fn unique_violation_message_names_columns_like_the_oracle() { + let Some(oracle) = pinned_oracle() else { + skip_no_oracle("autoindex_maintenance"); + return; + }; + let db = scratch_db("message"); + seed( + &oracle, + &db, + "CREATE TABLE t (a TEXT, b TEXT, c TEXT, PRIMARY KEY (a, b, c)); \ + INSERT INTO t VALUES ('x','y','z');", + ); + let err = our_insert(&db, "t", "INSERT INTO t VALUES ('x','y','z')").expect_err("duplicate"); + assert!( + err.contains("UNIQUE constraint failed: t.a, t.b, t.c"), + "message should match the oracle's column list, got {err}" + ); +} diff --git a/tests/corpus/main.rs b/tests/corpus/main.rs index 96ac7874..e0a60865 100644 --- a/tests/corpus/main.rs +++ b/tests/corpus/main.rs @@ -17,6 +17,7 @@ mod harness; mod oracle; mod analyze_test; +mod autoindex_maintenance_test; mod begin_immediate_lock_interop_test; mod btree_delete_test; mod btree_index_insert_delete_test; diff --git a/tests/performance/compile_path.rs b/tests/performance/compile_path.rs index 7cc5ea8f..f396313a 100644 --- a/tests/performance/compile_path.rs +++ b/tests/performance/compile_path.rs @@ -77,6 +77,7 @@ const WITH_CTE: &str = /// this bench stays fixture-free. fn bench_schema() -> TableSchema { TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec![ diff --git a/tests/performance/point_lookup.rs b/tests/performance/point_lookup.rs index e06cf6da..98552f52 100644 --- a/tests/performance/point_lookup.rs +++ b/tests/performance/point_lookup.rs @@ -59,6 +59,7 @@ fn fixture(row_count: u32, label: &str) -> (PathBuf, TableSchema) { assert!(status.success(), "fixture creation failed"); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["id".to_string(), "payload".to_string()], diff --git a/tests/sqllogictest/runner.rs b/tests/sqllogictest/runner.rs index f623be4e..26cd00ab 100644 --- a/tests/sqllogictest/runner.rs +++ b/tests/sqllogictest/runner.rs @@ -232,6 +232,7 @@ fn run_query(db_path: &Path, record: &QueryRecord) -> Outcome { // internally for this case, which never reads `schema` at all // — this dummy only satisfies the function's signature. let no_from_schema = TableSchema { + unresolved_autoindex: false, name: String::new(), root_page: 0, columns: vec![], From a6b450d7061e49cb9d21cf08e408b0ad088b6aa0 Mon Sep 17 00:00:00 2001 From: Diederik Siderius Date: Fri, 4 Sep 2026 10:46:44 +0200 Subject: [PATCH 2/3] fix: commit the test-file TableSchema updates #685 needs to build (#685) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit added `TableSchema::unresolved_autoindex` but left seven test files' struct literals uncommitted in my working tree, so the branch as pushed did not compile — `cargo build --tests` failed with 8 `missing field` errors in `tests/tiers/tier1.rs` and `tests/unit/codegen*.rs`. The 1562-passing run reported on the previous commit was real, but it measured the working tree rather than the committed branch. Recording that here rather than amending: the distinction is the actual lesson, and force-pushing over a branch already sent for review would hide it. Verified from the committed state this time: `cargo build --tests` clean, 1562 unit tests and 387 corpus tests pass. Refs: #685 Co-Authored-By: Claude Opus 5 (1M context) --- tests/tiers/tier1.rs | 1 + tests/unit/codegen.rs | 1 + tests/unit/codegen_delete_test.rs | 1 + tests/unit/codegen_expr_test.rs | 2 ++ tests/unit/codegen_insert_test.rs | 8 ++++++++ tests/unit/codegen_select_test.rs | 12 ++++++++++++ tests/unit/codegen_update_test.rs | 2 ++ 7 files changed, 27 insertions(+) diff --git a/tests/tiers/tier1.rs b/tests/tiers/tier1.rs index 07b65850..4e052131 100644 --- a/tests/tiers/tier1.rs +++ b/tests/tiers/tier1.rs @@ -157,6 +157,7 @@ fn t1_single_table_where_matches_oracle() { } let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["a".to_string(), "b".to_string()], diff --git a/tests/unit/codegen.rs b/tests/unit/codegen.rs index 7afc0029..72497369 100644 --- a/tests/unit/codegen.rs +++ b/tests/unit/codegen.rs @@ -43,6 +43,7 @@ fn schema(sql: &str, columns: &[&str]) -> TableSchema { }) .collect(); TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: columns.iter().map(|c| (*c).to_string()).collect(), diff --git a/tests/unit/codegen_delete_test.rs b/tests/unit/codegen_delete_test.rs index 992e767f..ede26f61 100644 --- a/tests/unit/codegen_delete_test.rs +++ b/tests/unit/codegen_delete_test.rs @@ -123,6 +123,7 @@ fn rows( fn schema(sql: &str) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["a".to_string(), "b".to_string()], diff --git a/tests/unit/codegen_expr_test.rs b/tests/unit/codegen_expr_test.rs index 966efde3..74865eb0 100644 --- a/tests/unit/codegen_expr_test.rs +++ b/tests/unit/codegen_expr_test.rs @@ -46,6 +46,7 @@ fn one_row_fixture() -> (std::path::PathBuf, TableSchema) { .expect("creating scratch fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["a".to_string(), "b".to_string(), "name".to_string()], @@ -220,6 +221,7 @@ fn affinity_fixture() -> (std::path::PathBuf, TableSchema) { .expect("creating scratch fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["i".to_string(), "r".to_string()], diff --git a/tests/unit/codegen_insert_test.rs b/tests/unit/codegen_insert_test.rs index 8af2bea6..56528623 100644 --- a/tests/unit/codegen_insert_test.rs +++ b/tests/unit/codegen_insert_test.rs @@ -113,6 +113,7 @@ fn not_null_violation_halts_and_inserts_nothing() { let header = seed_minimal_db(&vfs, &path, page_size); let sql = "CREATE TABLE t(a INTEGER NOT NULL, b TEXT)"; let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["a".to_string(), "b".to_string()], @@ -150,6 +151,7 @@ fn valid_row_round_trips() { let header = seed_minimal_db(&vfs, &path, page_size); let sql = "CREATE TABLE t(a INTEGER NOT NULL, b TEXT)"; let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["a".to_string(), "b".to_string()], @@ -191,6 +193,7 @@ fn default_value_applied_when_column_omitted() { let header = seed_minimal_db(&vfs, &path, page_size); let sql = "CREATE TABLE t(a INTEGER, b TEXT DEFAULT 'fallback')"; let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["a".to_string(), "b".to_string()], @@ -235,6 +238,7 @@ fn check_violation_halts() { let header = seed_minimal_db(&vfs, &path, page_size); let sql = "CREATE TABLE t(a INTEGER CHECK (a > 0))"; let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["a".to_string()], @@ -284,6 +288,7 @@ fn primary_key_conflict_aborts_by_default() { let header = seed_minimal_db(&vfs, &path, page_size); let sql = "CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT)"; let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["id".to_string(), "v".to_string()], @@ -332,6 +337,7 @@ fn primary_key_conflict_or_ignore_skips_the_row() { let header = seed_minimal_db(&vfs, &path, page_size); let sql = "CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT)"; let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["id".to_string(), "v".to_string()], @@ -377,6 +383,7 @@ fn primary_key_conflict_or_replace_overwrites_the_row() { let header = seed_minimal_db(&vfs, &path, page_size); let sql = "CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT)"; let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["id".to_string(), "v".to_string()], @@ -422,6 +429,7 @@ fn omitted_rowid_alias_is_auto_assigned() { let header = seed_minimal_db(&vfs, &path, page_size); let sql = "CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT)"; let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["id".to_string(), "v".to_string()], diff --git a/tests/unit/codegen_select_test.rs b/tests/unit/codegen_select_test.rs index b6050e0b..5e503b43 100644 --- a/tests/unit/codegen_select_test.rs +++ b/tests/unit/codegen_select_test.rs @@ -60,6 +60,7 @@ fn explain_query_plan_rejects_schema_count_mismatch() { fn bare_table(name: &str) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: name.to_string(), root_page: 0, columns: vec!["a".to_string()], @@ -158,6 +159,7 @@ fn scratch_fixture_labeled(label: &str) -> (PathBuf, TableSchema) { .expect("creating scratch fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["a".to_string(), "b".to_string(), "name".to_string()], @@ -195,6 +197,7 @@ fn empty_fixture_labeled(label: &str) -> (PathBuf, TableSchema) { .expect("creating empty scratch fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["a".to_string(), "b".to_string(), "name".to_string()], @@ -394,6 +397,7 @@ fn nulls_fixture(label: &str) -> (PathBuf, TableSchema) { .expect("creating nulls fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["i".to_string()], @@ -558,6 +562,7 @@ fn order_by_collate_nocase_is_case_insensitive() { .expect("creating collate fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["name".to_string()], @@ -739,6 +744,7 @@ fn order_by_limit_compiles_a_bounded_sorter_and_matches_full_sort() { .expect("creating scratch fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["a".to_string(), "b".to_string(), "name".to_string()], @@ -975,6 +981,7 @@ fn group_by_fixture(label: &str) -> (PathBuf, TableSchema) { .expect("creating GROUP BY fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["cat".to_string(), "sub".to_string(), "val".to_string()], @@ -1034,6 +1041,7 @@ fn min_max_aggregate_honours_collate_nocase() { .expect("creating agg collate fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["name".to_string()], @@ -1081,6 +1089,7 @@ fn group_by_boundary_honours_collate_nocase() { .expect("creating group by collate fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["name".to_string()], @@ -1211,6 +1220,7 @@ fn group_by_excludes_unreferenced_columns_from_the_sort_record() { .expect("creating GROUP BY fixture db"); assert!(status.success()); let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec![ @@ -1430,6 +1440,7 @@ fn group_by_expression() { #[test] fn plain_group_by_compiles_the_sorter_strategy() { let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["bucket".to_string(), "x".to_string()], @@ -1484,6 +1495,7 @@ fn plain_group_by_compiles_the_sorter_strategy() { #[test] fn distinct_aggregate_group_by_still_compiles_the_sorter_strategy() { let schema = TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 2, columns: vec!["bucket".to_string(), "x".to_string()], diff --git a/tests/unit/codegen_update_test.rs b/tests/unit/codegen_update_test.rs index 816f7a44..3ec5463e 100644 --- a/tests/unit/codegen_update_test.rs +++ b/tests/unit/codegen_update_test.rs @@ -124,6 +124,7 @@ fn rows( fn schema(sql: &str) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: vec!["a".to_string(), "b".to_string()], @@ -141,6 +142,7 @@ fn schema(sql: &str) -> TableSchema { fn schema_with_columns(sql: &str, columns: &[&str], column_types: &[&str]) -> TableSchema { TableSchema { + unresolved_autoindex: false, name: "t".to_string(), root_page: 1, columns: columns.iter().map(|s| (*s).to_string()).collect(), From afad17e15e42d46172d04900ee644870fe4572dd Mon Sep 17 00:00:00 2001 From: Diederik Siderius Date: Fri, 4 Sep 2026 11:18:23 +0200 Subject: [PATCH 3/3] fix: take &Path not &PathBuf in the autoindex test helpers (#685) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `make lint`'s second clippy pass — the one that lints the `test = false` corpus/parity/sqllogictest targets `--tests` skips — failed on `clippy::ptr_arg` for the two new helpers that forward `db` to `page_size_of(&Path)`. Only these two of the file's four `&PathBuf` params were flagged, and that asymmetry is the lint working as designed: `seed` and `oracle_select` pass `db` straight into `Command::arg`, a generic `AsRef` bound clippy won't assume the deref target satisfies, so it stays quiet there. `our_insert` and `autoindex_map` hand `db` to functions already typed `&Path`, proving the slice suffices. Left the other two alone rather than churn lines the gate is happy with. Signature-only; call sites deref-coerce unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- tests/corpus/autoindex_maintenance_test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/corpus/autoindex_maintenance_test.rs b/tests/corpus/autoindex_maintenance_test.rs index fb334471..9c68496c 100644 --- a/tests/corpus/autoindex_maintenance_test.rs +++ b/tests/corpus/autoindex_maintenance_test.rs @@ -93,7 +93,7 @@ fn oracle_select(oracle: &PathBuf, db: &PathBuf, sql: &str) -> String { } /// Runs one `INSERT` through our own parse -> codegen -> VDBE path. -fn our_insert(db: &PathBuf, table: &str, sql: &str) -> Result<(), String> { +fn our_insert(db: &Path, table: &str, sql: &str) -> Result<(), String> { let page_size = page_size_of(db); let header = read_header(db, page_size); let schema = table_schema(db, &header, table); @@ -111,7 +111,7 @@ fn our_insert(db: &PathBuf, table: &str, sql: &str) -> Result<(), String> { /// The name -> key-columns mapping our reader recovered, for asserting /// the numbering rule. -fn autoindex_map(db: &PathBuf, table: &str) -> Vec<(String, Vec)> { +fn autoindex_map(db: &Path, table: &str) -> Vec<(String, Vec)> { let page_size = page_size_of(db); let header = read_header(db, page_size); let schema = table_schema(db, &header, table);