Skip to content

fix: writes silently corrupt tables carrying a sqlite_autoindex_* (composite PRIMARY KEY / UNIQUE) #685

Description

@dpsiderius

Description

Writing to a table that carries a sqlite_autoindex_* silently corrupts
the database. The write returns success; the index is left stale.

src/schema/ddl_reader.rs::index_schema recovers an index's column list
by parsing its sqlite_master.sql. Every autoindex SQLite creates for a
PRIMARY KEY/UNIQUE constraint has sql = NULL, so the reader returns
None and the index is dropped from TableSchema::indexes
(documented at ddl_reader.rs:106, locked in by
auto_index_with_null_sql_is_omitted).

Graceful degradation is right for a read — you lose the index as an
access path, answers stay correct. For a write it is data loss, because
the same list drives two jobs in src/codegen/stmt/insert.rs:

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

Measured against stock sqlite3 3.51.0: after inserting into a
stock-created composite-PK table, PRAGMA integrity_check reports
wrong # of entries in index, and count(*) answers from the stale
index and undercounts. Exit code is 0 throughout.

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

Oracle-derived rule

Measured on sqlite3 3.51.0 rather than assumed. Autoindexes are
numbered sqlite_autoindex_<table>_<n>, n starting at 1, in
declaration order — not primary-key-first:

DDL autoindexes
PRIMARY KEY (a, b) _1 -> (a, b)
UNIQUE (a, b) _1 -> (a, b)
PRIMARY KEY (a), UNIQUE (b), UNIQUE (c) _1 -> (a), _2 -> (b), _3 -> (c)
a TEXT PRIMARY KEY, b TEXT UNIQUE _1 -> (a), _2 -> (b) — column-level counts
UNIQUE (c), PRIMARY KEY (a, b) _1 -> (c), _2 -> (a, b) — declaration order wins
a INTEGER PRIMARY KEY none — rowid alias
PRIMARY KEY (a, b)) WITHOUT ROWID none — the PK is the table

So: walk the CREATE TABLE text in order; every PRIMARY KEY or
UNIQUE constraint, column-level or table-level, claims the next
number, except a single INTEGER PRIMARY KEY column and the primary key
of a WITHOUT ROWID table.

Scope

src/schema/ddl_reader.rs:

  • Keep an index row whose sql is NULL and whose name matches
    sqlite_autoindex_% as a deferred entry (name, table, root page)
    rather than discarding it. read_schema_and_views is already
    two-phase (pending_indexes), so this is the existing hook.
  • In the attach phase, recover the deferred entry's columns from the
    owning table's own sql using the rule above, and push a real
    IndexSchema with unique: true.
  • An autoindex that still cannot be resolved must not be silently
    dropped — see the safety valve below.

Safety valve

Spec 010/Req 8 accepts either recovering the index or refusing the
write. Recovery is the fix, but it cannot be total: an autoindex whose
constraint this reader fails to parse must make the table read-only
rather than writable-and-corrupting. Precedent is spec 007/Req 1 —
a hot journal is detected and refused, because serving wrong pages
beats failing to open. Same argument, write side.

Non-goals

Creating sqlite_autoindex_* on CREATE TABLE — filed separately.
Both halves are needed for a complete fix (this one fixes adopting a
stock file; that one fixes producing one), but they are independent
changes with different blast radii.

Complexity

Estimate: medium
Reasoning: The oracle rule is already derived and tabulated above,
and the two-phase hook exists. The work is a constraint-list parser over
CREATE TABLE text (the reader already has column_list_span and
split_top_level_commas), the deferred-resolution wiring, the
read-only safety valve, and corpus tests against oracle-created files.
Bounded, but it touches the schema reader that every write path depends
on, so it needs real test coverage rather than a spot fix.

Acceptance Criteria

  • Writing to a stock-created composite-PK table leaves
    PRAGMA integrity_check clean and count(*) correct
  • A duplicate against an autoindex-backed constraint is refused
  • Named CREATE UNIQUE INDEX behaviour unchanged
  • Rowid-alias and WITHOUT ROWID tables gain no phantom index
  • Declaration-order numbering covered, including UNIQUE-before-PK
  • An unresolvable autoindex makes the table read-only, not corrupting
  • make lint, cargo fmt --check, make check-mod-files clean
  • Full suite shows no regression against baseline

Refs: 010/Req-8, #678

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions