fix: recover sqlite_autoindex_* keys so writes stop corrupting the file (#685)

[dpsiderius]

Summary

Writing to a table that carries a sqlite_autoindex_* — any composite
PRIMARY KEY or UNIQUE constraint — returned success and left the index
stale. Silent data loss, exit code 0 throughout.

ddl_reader::index_schema recovers an index's key columns by parsing its
sqlite_master.sql, and every autoindex SQLite creates has sql = NULL.
The reader returned None, so the index was dropped from
TableSchema::indexes — documented at ddl_reader.rs:106 and locked in by
auto_index_with_null_sql_is_omitted.

Dropping it is defensible on a read: you lose an access path, answers stay
correct. On a write it is corruption, because the same list drives two
separate jobs in src/codegen/stmt/insert.rs:

• :698 — schema.indexes.iter().filter(|idx| idx.unique) feeds
  emit_unique_check, so uniqueness was not enforced and a duplicate was
  accepted.
• :796 — emit_index_key_ops_from_regs maintains the indexes codegen
  emitted for, so the index b-tree was never updated.

Measured against the pinned oracle (sqlite3 3.53.4 —
tests/corpus/oracle.rs:22), on an oracle-created composite-PK table:

after two inserts + a duplicate	main	this branch	oracle
duplicate insert	accepted	rejected	rejected
error message	—	UNIQUE constraint failed: t.a, t.b	identical
SELECT count(*)	1	2	2
PRAGMA integrity_check	wrong # of entries in index	ok	ok

This blocks the SQE integration concretely: its iceberg_tables catalog keys
on (catalog_name, table_namespace, table_name) — exactly this shape — and
today works around it by dropping the declared primary key for a named unique
index and refusing writes to any catalog carrying a sqlite_autoindex_*.

How the keys are recovered

Autoindex rows are deferred during the sqlite_master walk and their keys
recovered afterwards from the owning table's own DDL. The numbering rule is
oracle-derived, not inferred — re-derived against the pinned 3.53.4 build
across eleven DDL shapes, checking pragma_index_info key lists rather than
just index counts. Several parts of it would have been guessed wrong:

• 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

Recovery cannot be total, so it is not the only line of defence. An autoindex
whose constraint this reader fails to parse sets
TableSchema::unresolved_autoindex, and INSERT/UPDATE/DELETE codegen refuse
the statement rather than write. Spec 010/Req 8 accepts either recovering
the index or refusing the write; spec 007/Req 1's hot-journal handling is the
same argument on the read side. Failing a statement beats silently producing a
corrupt database.

Two things found on the way, both handled deliberately

1. The rowid-alias rule this fix needs is not the one the crate implements.
rowid_alias_from_sql additionally requires the primary key to name the
table's only column, which is not SQLite's rule — it is a live
read-correctness bug, filed as #686 (a table-level PRIMARY KEY(col)
rowid alias reads back NULL when the table has other columns). 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 function when #686 lands.

2. 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, so fixing it changes named-index messages too — flagged here because
it is wider than the ticket. It is in scope regardless: this fix made the path
newly reachable for autoindexes, whose generated name tells a caller nothing
they can act on. The format now matches the oracle byte-for-byte. No existing
test asserted the old format, which is the coverage gap the new corpus file
closes.

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 as #687. This
half fixes adopting a stock-created file, which is the SQE case.

Test plan

• tests/corpus/autoindex_maintenance_test.rs — 7 new oracle-diff tests:
  all three spec 010/Req 8 scenarios, the declaration-order numbering
  rule, the rowid-alias and WITHOUT ROWID no-index cases, and
  unique-message parity
• cargo test --locked — 1562 passed / 0 failed, unchanged from the
  origin/main baseline measured the same way
• cargo test --locked --test corpus — 387 (380 baseline + 7)
• make lint, cargo fmt --check, make check-mod-files clean
• make assurance — 86/86 and 276/276, no dead links (spec 013/010-Req-8
  are (planned)-excluded and unmerged, so the dashboard cannot move yet —
  see the traceability note below)
• Numbering rule, key lists and the message format re-verified directly
  against /opt/homebrew/opt/sqlite/bin/sqlite3 3.53.4, the pinned build
• make lint — both clippy passes, including the second one for the
  test = false corpus/parity/sqllogictest targets that --tests skips.
  Worth calling out: my first check used cargo clippy --all-targets,
  which does not cover them, and the third commit exists because the
  second pass caught a real clippy::ptr_arg in the new corpus helpers
  that --all-targets reported clean
• Verified at afad17e from a detached clean worktree, not the
  working tree — see the second commit, which exists because the first
  push did not compile and I would rather record that than force-push
  over it

Notes for a reviewer

• 30 of the 32 changed files are a one-line struct-literal addition for the
  new TableSchema::unresolved_autoindex field. The review surface is
  src/schema/ddl_reader.rs (+280), the three write-path guards, and the
  corpus file.
• No CHANGELOG.md entry or version bump: this repo folds those into a
  separate chore/*-fold-into-0.18.x PR (precedent chore: fold #663 into 0.18.9, revert premature 0.18.10 bump #671).
• Traceability is incomplete until Feat/embedding api spec #678 lands. This fix discharges spec
  010/Req 8, which exists only in Feat/embedding api spec #678 — not on main. So the Refs: line
  below points at a requirement the tree does not yet carry, and the seven new
  corpus tests cannot get their per-scenario **Tests:** links (repo
  convention) until it does. Sequencing this after Feat/embedding api spec #678 is fine; merging it
  before is also fine, as long as the spec PR adds those links.

Spend: over the medium estimate. The fix itself matched it; the overrun
is the two follow-up bugs it surfaced (#686, #687), the message-parity
divergence that had to be fixed in-branch to avoid shipping a
newly-reachable wrong message, and the recovery commit.

Refs: 010/Req-8, #686, #687, #678

Closes #685

🤖 Generated with Claude Code