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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions .openspec/specs/010-vdbe-write-opcodes/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -381,13 +381,16 @@ Two ways to satisfy this, and either is acceptable:
read-only until it can. Narrower, and it converts silent corruption into an
error a caller can act on.

Creating `sqlite_autoindex_*` for a declared constraint is a separate `CREATE
Creating `sqlite_autoindex_*` for a declared constraint was a separate `CREATE
TABLE`-side gap (`src/codegen/stmt/insert.rs` records it, V3/V7 owns it), and it
has its own file-level consequence: a table this crate creates with a declared
composite `PRIMARY KEY` has no autoindex, and stock `sqlite3` then answers any
had its own file-level consequence: a table this crate created with a declared
composite `PRIMARY KEY` had no autoindex, and stock `sqlite3` then answered any
write or `integrity_check` on it with "database disk image is malformed (11)".
Closing this requirement without closing that one leaves creation broken; closing
that one without this leaves adoption of a foreign file broken.
Closing this requirement without closing that one left creation broken; closing
that one without this left adoption of a foreign file broken. Closed by #687:
`Opcode::CreateTable` now allocates a root page and a `sql IS NULL`
`sqlite_master` row per constraint `schema::autoindex_key_lists` says stock
SQLite would autoindex, numbered by the same rule #685's reader consumes.

**Implementation:** `src/schema/ddl_reader.rs::index_schema` (planned), consumed
by `src/codegen/stmt/insert.rs::emit_unique_check`
Expand Down Expand Up @@ -422,6 +425,26 @@ by `src/codegen/stmt/insert.rs::emit_unique_check`

**Tests:** `tests/corpus/autoindex_maintenance_test.rs::named_index_round_trips` (planned)

#### Scenario: CREATE TABLE with a declared composite key emits its own autoindex

- GIVEN `CREATE TABLE t (a INTEGER, b TEXT, PRIMARY KEY (a, b))` run through this
crate
- WHEN the oracle opens the resulting file
- THEN `PRAGMA integrity_check` reports ok, a `sqlite_autoindex_t_1` row exists
with `sql IS NULL`, and the oracle can insert and then rejects a duplicate key

**Tests:** `tests/corpus/create_table_autoindex_test.rs::composite_primary_key_passes_oracle_integrity_check_and_accepts_oracle_writes`, `tests/corpus/create_table_autoindex_test.rs::autoindex_master_row_shape_matches_oracle_convention`

#### Scenario: Rowid-alias and WITHOUT ROWID primary keys emit no autoindex on creation

- GIVEN `CREATE TABLE t (a INTEGER, b TEXT, PRIMARY KEY (a))` (a rowid alias,
#686) and separately the same DDL with `WITHOUT ROWID` appended
- WHEN this crate creates the table
- THEN no `sqlite_autoindex_*` row is registered for either — the constraint
consumes no autoindex number

**Tests:** `tests/corpus/create_table_autoindex_test.rs::rowid_alias_primary_key_gains_no_autoindex`, `tests/corpus/create_table_autoindex_test.rs::without_rowid_primary_key_gains_no_autoindex`

## Related regimes

- Tier suite: `tests/tiers/tier2.rs::t2_crud_round_trips_on_rowid_tables`
Expand Down
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ All notable changes to sqlite-rs. Format follows [Keep a Changelog](https://keep
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).
- `CREATE TABLE` with a declared composite `PRIMARY KEY`/`UNIQUE`
constraint created no `sqlite_autoindex_*` b-tree or `sqlite_master`
row, so stock `sqlite3` answered any write or `integrity_check`
against a table this crate created with "database disk image is
malformed (11)". `Opcode::CreateTable` now allocates a root page and
a `sql IS NULL` `sqlite_master` row for every constraint
`schema::autoindex_key_lists` says stock SQLite would autoindex,
numbered by the same declaration-order rule #685's reader already
consumes; rowid-alias and `WITHOUT ROWID` primary keys still consume
no number. `btree::MasterEntry::sql` is now `Option<String>` so a
`NULL` `sql` column can be represented at all, rather than an empty
string (#687).

## [0.18.10] - 2026-08-31

Expand Down
17 changes: 11 additions & 6 deletions src/btree/master.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,10 @@ pub struct MasterEntry {
pub tbl_name: String,
/// Root page number of the object's b-tree, or 0 when it has none.
pub rootpage: u32,
/// The `CREATE ...` SQL text that defines the object.
pub sql: String,
/// The `CREATE ...` SQL text that defines the object, or `None` for
/// an implicitly created `sqlite_autoindex_*` — stock SQLite stores
/// `NULL` there, not an empty string (#687).
pub sql: Option<String>,
}

/// Scans the table b-tree rooted at `root_page` and returns the highest
Expand Down Expand Up @@ -145,7 +147,10 @@ pub fn insert_master_row(
Value::Text(entry.name.as_str().into()),
Value::Text(entry.tbl_name.as_str().into()),
Value::Integer(entry.rootpage as i64),
Value::Text(entry.sql.as_str().into()),
match &entry.sql {
Some(sql) => Value::Text(sql.as_str().into()),
None => Value::Null,
},
];
let payload = encode_record(&values, header.text_encoding);
super::insert_row(pager, header, SQLITE_MASTER_ROOT_PAGE, next_rowid, &payload)
Expand Down Expand Up @@ -203,7 +208,7 @@ pub fn ensure_sqlite_sequence_table(
name: "sqlite_sequence".to_string(),
tbl_name: "sqlite_sequence".to_string(),
rootpage: root_page,
sql: SQLITE_SEQUENCE_SQL.to_string(),
sql: Some(SQLITE_SEQUENCE_SQL.to_string()),
},
)?;
bump_schema_cookie(pager)?;
Expand Down Expand Up @@ -323,7 +328,7 @@ pub fn ensure_sqlite_stat1_table(
name: "sqlite_stat1".to_string(),
tbl_name: "sqlite_stat1".to_string(),
rootpage: root_page,
sql: SQLITE_STAT1_SQL.to_string(),
sql: Some(SQLITE_STAT1_SQL.to_string()),
},
)?;
bump_schema_cookie(pager)?;
Expand Down Expand Up @@ -445,7 +450,7 @@ mod tests {
name: "t".to_string(),
tbl_name: "t".to_string(),
rootpage: 2,
sql: "CREATE TABLE t(a INTEGER, b TEXT)".to_string(),
sql: Some("CREATE TABLE t(a INTEGER, b TEXT)".to_string()),
},
)
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion src/btree/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ mod tests {
name: "t".to_string(),
tbl_name: "t".to_string(),
rootpage: table_root,
sql: "CREATE TABLE t(name)".to_string(),
sql: Some("CREATE TABLE t(name)".to_string()),
};

let index_root = create_empty_index_root(&mut pager).unwrap();
Expand Down
11 changes: 6 additions & 5 deletions src/codegen/stmt/insert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,12 @@
//! seek+branch primitive (`src/vdbe/cursor.rs`, built on
//! `IndexCursor::seek`) and dispatches `ON CONFLICT` the same way
//! `emit_pk_conflict` does for the rowid-PK case. A composite
//! `PRIMARY KEY(...)`/`UNIQUE(...)` *table* constraint with no backing
//! `CREATE INDEX`/on-disk index (this codebase doesn't auto-create
//! `sqlite_autoindex_*` entries yet) has no real index to seek against,
//! so it still isn't enforced — that's a `CREATE TABLE`-side gap, not
//! an INSERT-codegen one.
//! `PRIMARY KEY(...)`/`UNIQUE(...)` *table* constraint goes through this
//! same path: `CREATE TABLE` now backs it with a `sqlite_autoindex_*`
//! b-tree and `sqlite_master` row (#687), so by the time INSERT compiles
//! there is a real on-disk index in `schema.indexes` for
//! `emit_unique_check` to seek against — no `CREATE TABLE`-side gap is
//! left for this codegen to work around.
//!
//! Known simplifications (deferred to follow-up tickets, not chased
//! here):
Expand Down
6 changes: 3 additions & 3 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
mod ddl_reader;

pub use ddl_reader::{
column_defs, column_type, read_schema, read_schema_and_views, read_table_and_view_names,
read_views, rowid_alias_from_sql, DdlError, IndexSchema, IndexedColumn, TableSchema,
ViewSchema,
autoindex_key_lists, column_defs, column_names_and_without_rowid, column_type, read_schema,
read_schema_and_views, read_table_and_view_names, read_views, rowid_alias_from_sql, DdlError,
IndexSchema, IndexedColumn, TableSchema, ViewSchema,
};
16 changes: 15 additions & 1 deletion src/schema/ddl_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ fn same_key_list(a: &[IndexedColumn], b: &[IndexedColumn]) -> bool {
///
/// `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<Vec<Vec<IndexedColumn>>> {
pub fn autoindex_key_lists(sql: &str, without_rowid: bool) -> Option<Vec<Vec<IndexedColumn>>> {
let (start, end) = column_list_span(sql)?;
let inner = sql.get(start..end)?;
let alias = rowid_alias_column_name(sql, without_rowid);
Expand Down Expand Up @@ -756,6 +756,20 @@ struct ParsedCreateTable {
strict: bool,
}

/// Returns a fresh `CREATE TABLE`'s column names (declared order) and
/// whether it is `WITHOUT ROWID` — the two pieces `Opcode::CreateTable`
/// needs to resolve [`autoindex_key_lists`]' `IndexedColumn` names into
/// column positions for [`crate::btree::populate_index_from_table`]
/// (#687). Empty columns and `without_rowid: false` for anything this
/// naive reader can't parse, same degradation as the rest of this
/// module.
pub fn column_names_and_without_rowid(sql: &str) -> (Vec<String>, bool) {
match parse_create_table(sql) {
Some(parsed) => (parsed.columns, parsed.without_rowid),
None => (Vec::new(), false),
}
}

/// Parses `CREATE TABLE ... (col-defs) [table-options]`. Returns `None`
/// for anything this naive reader can't find a column list in — the
/// caller treats that identically to a virtual table (empty columns,
Expand Down
73 changes: 69 additions & 4 deletions src/vdbe/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2178,22 +2178,87 @@ pub fn create_table(vm: &mut Vm, instr: &Instruction) -> Result<Step, ExecError>
&btree::MasterEntry {
kind: "table".to_string(),
name: name.clone(),
tbl_name: name,
tbl_name: name.clone(),
rootpage: root_page,
sql,
sql: Some(sql.clone()),
},
)
.map_err(|e| ExecError::MalformedInstruction {
opcode: "CreateTable",
reason: e.to_string(),
})?;
create_declared_autoindexes(&mut pager, &header, &name, &sql, root_page)?;
btree::bump_schema_cookie(&mut pager).map_err(|e| ExecError::MalformedInstruction {
opcode: "CreateTable",
reason: e.to_string(),
})?;
Ok(Step::Next)
}

/// Emits a `sqlite_autoindex_<table>_<n>` entry for each constraint
/// SQLite would autoindex, per the numbering rule #685 derived from the
/// oracle and implements in `schema::autoindex_key_lists` — declaration
/// order, column-level constraints included, rowid-alias/`WITHOUT
/// ROWID` primary keys skipped and consuming no number, redundant
/// constraints collapsed (#687). Runs inside `create_table`'s own write,
/// so a failed `CreateTable` never leaves a partial autoindex behind.
fn create_declared_autoindexes(
pager: &mut crate::pager::Pager,
header: &crate::header::DatabaseHeader,
table_name: &str,
sql: &str,
table_root_page: u32,
) -> Result<(), ExecError> {
let (columns, without_rowid) = crate::schema::column_names_and_without_rowid(sql);
let Some(key_lists) = crate::schema::autoindex_key_lists(sql, without_rowid) else {
return Ok(());
};
for (i, key_cols) in key_lists.into_iter().enumerate() {
let ordinal = i.saturating_add(1);
let index_name = format!("sqlite_autoindex_{table_name}_{ordinal}");
let column_indices: Vec<usize> = key_cols
.iter()
.filter_map(|c| {
columns
.iter()
.position(|col| col.eq_ignore_ascii_case(&c.name))
})
.collect();
let index_root =
btree::create_empty_index_root(pager).map_err(|e| ExecError::MalformedInstruction {
opcode: "CreateTable",
reason: e.to_string(),
})?;
btree::populate_index_from_table(
pager,
header,
table_root_page,
index_root,
&column_indices,
)
.map_err(|e| ExecError::MalformedInstruction {
opcode: "CreateTable",
reason: e.to_string(),
})?;
btree::insert_master_row(
pager,
header,
&btree::MasterEntry {
kind: "index".to_string(),
name: index_name,
tbl_name: table_name.to_string(),
rootpage: index_root,
sql: None,
},
)
.map_err(|e| ExecError::MalformedInstruction {
opcode: "CreateTable",
reason: e.to_string(),
})?;
}
Ok(())
}

/// `CreateView` (#380): registers a `sqlite_master` row with
/// `type = 'view'` and `rootpage = 0` — a view has no b-tree of its own,
/// so unlike [`create_table`] this never allocates a root page.
Expand All @@ -2219,7 +2284,7 @@ pub fn create_view(vm: &mut Vm, instr: &Instruction) -> Result<Step, ExecError>
name: name.clone(),
tbl_name: name,
rootpage: 0,
sql,
sql: Some(sql),
},
)
.map_err(|e| ExecError::MalformedInstruction {
Expand Down Expand Up @@ -2343,7 +2408,7 @@ pub fn create_index(vm: &mut Vm, instr: &Instruction) -> Result<Step, ExecError>
name: name.clone(),
tbl_name: table_name,
rootpage: index_root,
sql,
sql: Some(sql),
},
)
.map_err(|e| ExecError::MalformedInstruction {
Expand Down
Loading