db: warn on schema drift at startup

Migrations can be recorded without being executed. That is not hypothetical:
migration 0009 exists to clean up after 0004, which production recorded but
never applied, leaving eleven columns behind that fresh installs never had.
Nothing reported it at the time; it surfaced later as confusing behavior.

InitDB now compares an existing database against schema.sql after migrations run
and logs one warning per difference. Fresh databases skip the check, since they
were just built from schema.sql and agree by construction.

The comparison works by applying schema.sql to a throwaway in-memory database
and introspecting that, rather than parsing the DDL. SQLite's own resolution of
types, defaults and implicit indexes is exactly what we want to compare against,
and a hand-rolled parser would drift from the engine. The introspection is
shared with TestSchemaMatchesMigrations, so the test exercises the same code
that runs at boot.

Warn-only, never fatal. A database merely ahead of or behind schema.sql is
almost always still able to serve traffic, so refusing to boot would turn a diff
that wants a corrective migration into an outage, during a deploy, which is the
worst possible moment to have one.

The README claimed new tables go in schema.sql only. That is wrong in the
direction that hurts: InitDB skips schema.sql entirely once schema_migrations
has rows, so such a table appears on fresh installs, passes every test, and is
silently absent in production. Documented the real rule along with two others
the test cannot enforce: migrations must not return rows (go-libsql rejects them
with "Execute returned rows"), and rebuild migrations must name columns
explicitly, since column order legitimately differs between fresh and upgraded
databases.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Evan Jarrett
2026-08-11 21:11:46 -05:00
co-authored by Claude Opus 5
parent f5ddc229a1
commit 2abcae95f7
4 changed files with 443 additions and 134 deletions
+246
View File
@@ -0,0 +1,246 @@
package db
import (
"database/sql"
"fmt"
"regexp"
"slices"
"sort"
"strings"
)
// schemaSnapshot is a structural description of a database: the column set of
// every user table, plus the explicitly-created indexes.
//
// Columns are held as sorted descriptor strings rather than as ordered lists,
// because ordinal position legitimately differs between a fresh install and an
// upgraded one. Migrations append with ALTER TABLE ADD COLUMN; schema.sql places
// the same column mid-table. Comparing as a set is what makes the two
// comparable at all.
type schemaSnapshot struct {
Tables map[string][]string
Indexes []string
}
var driftWhitespaceRun = regexp.MustCompile(`\s+`)
// introspectSchema reads the structure of db.
//
// SQLite internal tables (sqlite_sequence and friends, created implicitly by
// AUTOINCREMENT) are skipped: they are an artifact of the engine rather than
// something either schema.sql or a migration declares.
func introspectSchema(db DBTX) (schemaSnapshot, error) {
snap := schemaSnapshot{Tables: make(map[string][]string)}
tables, err := listUserTables(db)
if err != nil {
return snap, err
}
for _, table := range tables {
cols, err := describeColumns(db, table)
if err != nil {
return snap, err
}
snap.Tables[table] = cols
}
snap.Indexes, err = describeIndexes(db)
if err != nil {
return snap, err
}
return snap, nil
}
func listUserTables(db DBTX) ([]string, error) {
rows, err := db.Query(`
SELECT name FROM sqlite_master
WHERE type = 'table' AND name NOT LIKE 'sqlite_%'
ORDER BY name
`)
if err != nil {
return nil, fmt.Errorf("list tables: %w", err)
}
defer rows.Close()
var names []string
for rows.Next() {
var n string
if err := rows.Scan(&n); err != nil {
return nil, fmt.Errorf("scan table name: %w", err)
}
names = append(names, n)
}
return names, rows.Err()
}
// describeColumns returns one sorted descriptor per column: name, declared
// type, not-null, default and primary-key position. Ordinal position is
// deliberately excluded; see schemaSnapshot.
func describeColumns(db DBTX, table string) ([]string, error) {
rows, err := db.Query(fmt.Sprintf("PRAGMA table_info(%q)", table))
if err != nil {
return nil, fmt.Errorf("table_info(%s): %w", table, err)
}
defer rows.Close()
var cols []string
for rows.Next() {
var (
cid int
name string
colType string
notNull int
dfltValue sql.NullString
pk int
)
if err := rows.Scan(&cid, &name, &colType, &notNull, &dfltValue, &pk); err != nil {
return nil, fmt.Errorf("scan table_info(%s): %w", table, err)
}
dflt := "<none>"
if dfltValue.Valid {
dflt = dfltValue.String
}
cols = append(cols, fmt.Sprintf("%s type=%s notnull=%d default=%s pk=%d",
name, strings.ToUpper(colType), notNull, dflt, pk))
}
if err := rows.Err(); err != nil {
return nil, err
}
sort.Strings(cols)
return cols, nil
}
// describeIndexes returns normalized DDL for the explicitly-created indexes.
// Indexes SQLite creates implicitly for UNIQUE and PRIMARY KEY constraints have
// a NULL sql column and are skipped, since the column set already implies them.
func describeIndexes(db DBTX) ([]string, error) {
rows, err := db.Query(`
SELECT sql FROM sqlite_master
WHERE type = 'index' AND sql IS NOT NULL
`)
if err != nil {
return nil, fmt.Errorf("list indexes: %w", err)
}
defer rows.Close()
var out []string
for rows.Next() {
var ddl string
if err := rows.Scan(&ddl); err != nil {
return nil, fmt.Errorf("scan index sql: %w", err)
}
norm := driftWhitespaceRun.ReplaceAllString(strings.TrimSpace(ddl), " ")
norm = strings.ReplaceAll(norm, "IF NOT EXISTS ", "")
out = append(out, norm)
}
if err := rows.Err(); err != nil {
return nil, err
}
sort.Strings(out)
return out, nil
}
// referenceSnapshot builds the structure schema.sql describes, by applying it to
// a throwaway in-memory database and introspecting that.
//
// Introspecting a real database beats parsing the DDL: SQLite's own resolution
// of types, defaults and implicit indexes is the thing we want to compare
// against, and reimplementing it in a parser would drift from the engine.
func referenceSnapshot() (schemaSnapshot, error) {
connector, err := openLibsqlLocalConnector(":memory:")
if err != nil {
return schemaSnapshot{}, fmt.Errorf("open reference connector: %w", err)
}
ref := sql.OpenDB(connector)
// One connection, so every statement lands in the same in-memory database.
ref.SetMaxOpenConns(1)
defer ref.Close()
for i, stmt := range splitSQLStatements(schemaSQL) {
if _, err := ref.Exec(stmt); err != nil {
return schemaSnapshot{}, fmt.Errorf("apply schema.sql statement %d: %w", i+1, err)
}
}
return introspectSchema(ref)
}
// SchemaDrift compares a live database against what schema.sql describes and
// returns one human-readable line per difference. An empty result means they
// agree.
//
// This exists because migrations can be recorded without being executed. That is
// not hypothetical: migration 0009 had to clean up after 0004, which production
// recorded but never applied, leaving eleven columns behind that fresh installs
// did not have. Nothing reported that at the time; it surfaced later as
// confusing behavior. A check at boot turns that class of problem into a log
// line at deploy time.
//
// Callers should warn on the result and continue. Drift is a signal to write a
// corrective migration, not a reason to refuse to serve traffic: a database that
// is merely ahead of or behind schema.sql is usually still perfectly able to run
// the application, and failing the boot would turn a cosmetic diff into an
// outage.
func SchemaDrift(live DBTX) ([]string, error) {
want, err := referenceSnapshot()
if err != nil {
return nil, err
}
got, err := introspectSchema(live)
if err != nil {
return nil, fmt.Errorf("introspect live database: %w", err)
}
var findings []string
wantTables := sortedKeys(want.Tables)
gotTables := sortedKeys(got.Tables)
for _, table := range wantTables {
if !slices.Contains(gotTables, table) {
findings = append(findings, fmt.Sprintf(
"table %q is in schema.sql but missing from the database (a migration to create it is probably missing)", table))
continue
}
for _, col := range want.Tables[table] {
if !slices.Contains(got.Tables[table], col) {
findings = append(findings, fmt.Sprintf(
"table %q: schema.sql expects column [%s] but the database does not have it", table, col))
}
}
for _, col := range got.Tables[table] {
if !slices.Contains(want.Tables[table], col) {
findings = append(findings, fmt.Sprintf(
"table %q: database has column [%s] which schema.sql does not declare", table, col))
}
}
}
for _, table := range gotTables {
if !slices.Contains(wantTables, table) {
findings = append(findings, fmt.Sprintf(
"table %q exists in the database but is not declared in schema.sql", table))
}
}
for _, idx := range want.Indexes {
if !slices.Contains(got.Indexes, idx) {
findings = append(findings, fmt.Sprintf("index missing from the database: %s", idx))
}
}
for _, idx := range got.Indexes {
if !slices.Contains(want.Indexes, idx) {
findings = append(findings, fmt.Sprintf("index in the database but not in schema.sql: %s", idx))
}
}
return findings, nil
}
func sortedKeys(m map[string][]string) []string {
out := make([]string, 0, len(m))
for k := range m {
out = append(out, k)
}
sort.Strings(out)
return out
}
+37 -5
View File
@@ -6,13 +6,43 @@ This directory contains database migrations for the ATCR AppView database.
**`schema.sql`** (in parent directory) contains the **complete base schema** for fresh database installations. It includes all tables, indexes, and constraints.
**Migrations** (this directory) handle **changes to existing databases**. They are only for:
**Migrations** (this directory) handle **changes to existing databases**:
- `CREATE TABLE` statements (see below — new tables need a migration too)
- `ALTER TABLE` statements (add/modify/drop columns)
- `UPDATE` statements (data transformations)
- `DELETE` statements (data cleanup)
- Creating/modifying indexes on existing tables
**NEW TABLES go in `schema.sql`, NOT in migrations.**
### New tables go in BOTH places
`InitDB` skips `schema.sql` entirely once `schema_migrations` has any rows (see
`hasAppliedMigrations` in `schema.go`). **A table added only to `schema.sql` will
never be created on an existing database** — it appears on fresh installs, works
in every test, and is silently absent in production.
So a new table needs two changes:
1. `schema.sql`, for fresh installs.
2. A migration with `CREATE TABLE IF NOT EXISTS`, for existing databases.
`TestSchemaMatchesMigrations` enforces this. It builds the schema both ways and
fails if they disagree, so forgetting either half is caught before it ships.
### Two more rules the test cannot enforce for you
**Migrations must not return rows.** go-libsql rejects a row-returning statement
passed to `Exec` with `Execute returned rows`. A migration that opens with a bare
`SELECT` fails on every database that has not already recorded it. (Migration
0001 does exactly this; it survives only because every real database recorded it
years ago.)
**Rebuild migrations must name their columns.** Column order in `schema.sql` is
illustrative, not authoritative: migrations append with `ADD COLUMN` while
`schema.sql` places the same column mid-table, so a fresh database and an
upgraded one legitimately differ in column order on `manifests`, `users`,
`devices` and `repo_pages`. `INSERT INTO new_table SELECT * FROM old_table` will
therefore silently write the wrong values into the wrong columns. Always write
`INSERT INTO new_table (a, b, c) SELECT a, b, c FROM old_table`, as migrations
0009 and 0011 do.
## Migration Format
@@ -101,7 +131,9 @@ query: |
- **Never modify existing migrations** - Once applied, they're immutable
- **Test migrations** before committing - Ensure they work on existing databases
- **Version numbers must be unique** - The migration system will fail if duplicates exist
- **Version numbers must be unique** - The migration system silently skips a duplicate, so the second file never runs (see `TestMigrationVersionsAreUnique`)
- **Migrations run automatically** on `InitDB()` - Schema first, then migrations
- **CRITICAL: Update `schema.sql` for structural changes** - When you ALTER a table or add columns, update both the migration AND `schema.sql` so fresh installations have the same structure
- **New tables go in `schema.sql` only** - Don't create migration files for new tables
- **CRITICAL: Update `schema.sql` for every structural change** - Columns, tables and indexes all need both the migration AND the `schema.sql` entry, or fresh and existing databases diverge. `TestSchemaMatchesMigrations` fails the build if they do
- **Migrations must not return rows** - a bare `SELECT` fails under go-libsql with `Execute returned rows`
- **Rebuild migrations must name columns explicitly** - never `INSERT INTO new SELECT * FROM old`; column order differs between fresh and upgraded databases
- **Drift is reported at boot** - `InitDB` logs a warning for any difference between an existing database and `schema.sql`. It never fails the boot; treat the warning as a request for a corrective migration
+31
View File
@@ -125,9 +125,40 @@ func InitDB(path string, cfg LibsqlConfig) (*sql.DB, error) {
return nil, err
}
// Report any structural drift from schema.sql. Only meaningful for existing
// databases: a fresh one was just built from schema.sql, so it agrees by
// construction. Warn and continue — see SchemaDrift for why this never
// fails the boot.
if isExisting {
reportSchemaDrift(db)
}
return db, nil
}
// reportSchemaDrift logs any difference between the live database and
// schema.sql. Migrations can be recorded without being executed (0004 was, on
// production, which is the whole reason migration 0009 exists), and until now
// nothing surfaced that. Failure to run the check is itself only a warning: an
// introspection problem must not stop the server from starting.
func reportSchemaDrift(db *sql.DB) {
findings, err := SchemaDrift(db)
if err != nil {
slog.Warn("Could not check database schema for drift", "error", err)
return
}
if len(findings) == 0 {
slog.Debug("Database schema matches schema.sql")
return
}
slog.Warn("Database schema differs from schema.sql",
"differences", len(findings),
"hint", "a migration may have been recorded without being executed; a corrective migration is probably needed")
for _, f := range findings {
slog.Warn("Schema drift", "detail", f)
}
}
// hasAppliedMigrations checks if this is an existing database with migrations applied
func hasAppliedMigrations(db *sql.DB) (bool, error) {
// Check if schema_migrations table exists
+129 -129
View File
@@ -3,10 +3,10 @@ package db
import (
"database/sql"
"fmt"
"log/slog"
"os"
"regexp"
"path/filepath"
"slices"
"sort"
"strings"
"testing"
)
@@ -40,8 +40,17 @@ func TestSchemaMatchesMigrations(t *testing.T) {
migrated := migratedSchemaDB(t)
defer migrated.Close()
freshTables := tableNames(t, fresh)
migratedTables := tableNames(t, migrated)
freshSnap, err := introspectSchema(fresh)
if err != nil {
t.Fatalf("introspect fresh database: %v", err)
}
migratedSnap, err := introspectSchema(migrated)
if err != nil {
t.Fatalf("introspect migrated database: %v", err)
}
freshTables := sortedKeys(freshSnap.Tables)
migratedTables := sortedKeys(migratedSnap.Tables)
if diff := diffStringSets(freshTables, migratedTables); diff != "" {
t.Errorf("table sets differ between schema.sql and the migrations:\n%s\n\n"+
@@ -51,23 +60,118 @@ func TestSchemaMatchesMigrations(t *testing.T) {
}
for _, table := range freshTables {
if !contains(migratedTables, table) {
if !slices.Contains(migratedTables, table) {
continue // already reported above
}
freshCols := columnSet(t, fresh, table)
migratedCols := columnSet(t, migrated, table)
if diff := diffStringSets(freshCols, migratedCols); diff != "" {
if diff := diffStringSets(freshSnap.Tables[table], migratedSnap.Tables[table]); diff != "" {
t.Errorf("table %q differs between schema.sql and the migrations:\n%s", table, diff)
}
}
freshIdx := indexSet(t, fresh)
migratedIdx := indexSet(t, migrated)
if diff := diffStringSets(freshIdx, migratedIdx); diff != "" {
if diff := diffStringSets(freshSnap.Indexes, migratedSnap.Indexes); diff != "" {
t.Errorf("index sets differ between schema.sql and the migrations:\n%s", diff)
}
}
// TestSchemaDriftCleanOnFreshDatabase pins the other end of the same invariant:
// SchemaDrift, which runs at boot against real databases, must report nothing
// for a database that was just built from schema.sql. If this ever fails, the
// startup check has become noisy and operators will learn to ignore it, which
// costs more than not having it at all.
func TestSchemaDriftCleanOnFreshDatabase(t *testing.T) {
db := freshSchemaDB(t)
defer db.Close()
findings, err := SchemaDrift(db)
if err != nil {
t.Fatalf("SchemaDrift: %v", err)
}
if len(findings) != 0 {
t.Errorf("expected no drift against a database built from schema.sql, got %d:\n %s",
len(findings), strings.Join(findings, "\n "))
}
}
// TestSchemaDriftDetectsMissingColumn proves SchemaDrift actually compares
// rather than always returning clean. A database missing a column schema.sql
// declares is exactly the shape of the 0004 incident.
func TestSchemaDriftDetectsMissingColumn(t *testing.T) {
db := freshSchemaDB(t)
defer db.Close()
// Rebuild users without registry_domain, imitating a migration that was
// recorded but never executed.
stmts := []string{
`CREATE TABLE users_drifted (
did TEXT PRIMARY KEY,
handle TEXT NOT NULL,
pds_endpoint TEXT NOT NULL,
avatar TEXT,
default_hold_did TEXT,
oci_client TEXT DEFAULT '',
last_seen TIMESTAMP NOT NULL,
UNIQUE(handle)
)`,
`DROP TABLE users`,
`ALTER TABLE users_drifted RENAME TO users`,
}
for _, s := range stmts {
if _, err := db.Exec(s); err != nil {
t.Fatalf("set up drifted schema: %v", err)
}
}
findings, err := SchemaDrift(db)
if err != nil {
t.Fatalf("SchemaDrift: %v", err)
}
if !anyContains(findings, "registry_domain") {
t.Errorf("expected a finding naming the missing registry_domain column, got: %v", findings)
}
}
// TestInitDBWarnsOnDriftAndStillBoots covers the integration rather than the
// comparison: an existing database whose schema has drifted must still open.
//
// The check is warn-only on purpose. A database that is merely ahead of or
// behind schema.sql is almost always still able to serve traffic, so refusing to
// boot would convert a diff that wants a corrective migration into an outage,
// and it would do so during a deploy, which is the worst possible moment.
func TestInitDBWarnsOnDriftAndStillBoots(t *testing.T) {
path := filepath.Join(t.TempDir(), "app.db")
first, err := InitDB(path, LibsqlConfig{})
if err != nil {
t.Fatalf("first InitDB: %v", err)
}
// Imitate a migration that was recorded but never executed.
if _, err := first.Exec(`ALTER TABLE users DROP COLUMN registry_domain`); err != nil {
t.Fatalf("drop column: %v", err)
}
first.Close()
var logged strings.Builder
restore := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(&logged, &slog.HandlerOptions{Level: slog.LevelWarn})))
defer slog.SetDefault(restore)
// The second open takes the existing-database path, which is the only one
// that runs the drift check.
second, err := InitDB(path, LibsqlConfig{})
if err != nil {
t.Fatalf("second InitDB must succeed despite drift, got: %v", err)
}
defer second.Close()
out := logged.String()
if !strings.Contains(out, "Schema drift") {
t.Errorf("expected a schema drift warning at startup, got log output:\n%s", out)
}
if !strings.Contains(out, "registry_domain") {
t.Errorf("expected the warning to name the missing column, got log output:\n%s", out)
}
}
// freshSchemaDB returns a database built the way a fresh install builds one:
// schema.sql applied directly, migrations recorded but not executed.
func freshSchemaDB(t *testing.T) *sql.DB {
@@ -110,8 +214,8 @@ func migratedSchemaDB(t *testing.T) *sql.DB {
}
// freshDB=false so the DDL actually executes rather than merely being
// recorded. base_schema.sql creates schema_migrations empty, so every
// migration is pending.
// recorded. base_schema.sql records versions 1-8 as already applied, so
// everything from 0009 onward is pending.
if err := runMigrations(db, false); err != nil {
db.Close()
t.Fatalf("runMigrations against the base snapshot: %v", err)
@@ -119,132 +223,28 @@ func migratedSchemaDB(t *testing.T) *sql.DB {
return db
}
// tableNames returns the user tables in db, sorted. SQLite internal tables
// (sqlite_sequence and friends, created implicitly by AUTOINCREMENT) are
// excluded since they are an artifact of the engine, not of our schema.
func tableNames(t *testing.T, db *sql.DB) []string {
t.Helper()
rows, err := db.Query(`
SELECT name FROM sqlite_master
WHERE type = 'table' AND name NOT LIKE 'sqlite_%'
ORDER BY name
`)
if err != nil {
t.Fatalf("list tables: %v", err)
}
defer rows.Close()
var names []string
for rows.Next() {
var n string
if err := rows.Scan(&n); err != nil {
t.Fatalf("scan table name: %v", err)
}
names = append(names, n)
}
if err := rows.Err(); err != nil {
t.Fatalf("iterate tables: %v", err)
}
return names
}
// columnSet returns one descriptor per column: name, type, not-null, default and
// primary-key position. Ordinal position (cid) is deliberately excluded.
func columnSet(t *testing.T, db *sql.DB, table string) []string {
t.Helper()
rows, err := db.Query(fmt.Sprintf("PRAGMA table_info(%q)", table))
if err != nil {
t.Fatalf("table_info(%s): %v", table, err)
}
defer rows.Close()
var cols []string
for rows.Next() {
var (
cid int
name string
colType string
notNull int
dfltValue sql.NullString
pk int
)
if err := rows.Scan(&cid, &name, &colType, &notNull, &dfltValue, &pk); err != nil {
t.Fatalf("scan table_info(%s): %v", table, err)
}
dflt := "<none>"
if dfltValue.Valid {
dflt = dfltValue.String
}
cols = append(cols, fmt.Sprintf("%s type=%s notnull=%d default=%s pk=%d",
name, strings.ToUpper(colType), notNull, dflt, pk))
}
if err := rows.Err(); err != nil {
t.Fatalf("iterate table_info(%s): %v", table, err)
}
sort.Strings(cols)
return cols
}
var whitespaceRun = regexp.MustCompile(`\s+`)
// indexSet returns the explicitly-created indexes as normalized DDL. Indexes
// SQLite creates implicitly for UNIQUE and PRIMARY KEY constraints have a NULL
// sql column and are skipped: they are already implied by the column set.
func indexSet(t *testing.T, db *sql.DB) []string {
t.Helper()
rows, err := db.Query(`
SELECT sql FROM sqlite_master
WHERE type = 'index' AND sql IS NOT NULL
`)
if err != nil {
t.Fatalf("list indexes: %v", err)
}
defer rows.Close()
var out []string
for rows.Next() {
var ddl string
if err := rows.Scan(&ddl); err != nil {
t.Fatalf("scan index sql: %v", err)
}
norm := whitespaceRun.ReplaceAllString(strings.TrimSpace(ddl), " ")
norm = strings.ReplaceAll(norm, "IF NOT EXISTS ", "")
out = append(out, norm)
}
if err := rows.Err(); err != nil {
t.Fatalf("iterate indexes: %v", err)
}
sort.Strings(out)
return out
}
// diffStringSets returns a human-readable diff, or "" when the sets match.
// "schema.sql" is the a side, "migrations" the b side.
func diffStringSets(a, b []string) string {
var onlyA, onlyB []string
var sb strings.Builder
for _, s := range a {
if !contains(b, s) {
onlyA = append(onlyA, s)
if !slices.Contains(b, s) {
fmt.Fprintf(&sb, " only in schema.sql: %s\n", s)
}
}
for _, s := range b {
if !contains(a, s) {
onlyB = append(onlyB, s)
if !slices.Contains(a, s) {
fmt.Fprintf(&sb, " only in migrations: %s\n", s)
}
}
if len(onlyA) == 0 && len(onlyB) == 0 {
return ""
}
var sb strings.Builder
for _, s := range onlyA {
fmt.Fprintf(&sb, " only in schema.sql: %s\n", s)
}
for _, s := range onlyB {
fmt.Fprintf(&sb, " only in migrations: %s\n", s)
}
return strings.TrimRight(sb.String(), "\n")
}
func contains(haystack []string, needle string) bool {
return slices.Contains(haystack, needle)
func anyContains(haystack []string, needle string) bool {
for _, s := range haystack {
if strings.Contains(s, needle) {
return true
}
}
return false
}