diff --git a/CLAUDE.md b/CLAUDE.md index 8933725..bae21c5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -233,6 +233,19 @@ rather than renaming them. the drift it caused before — `sql/schemas/` and the migrations disagreed, and sqlc generated against the stale one. + **What that costs an existing database is that it does not open**, and + "delete and rescan" is the answer (plan 013, open question 1) — free + for everyone except one machine. The index job's `/cache` volume is a + real `YJ_HOME` that survives between runs, and half of it is the + catalog: deleting it means re-downloading ~205 GB. So `cmd/indexbuild` + repairs it instead (`staleschema.go`), dropping every table `datamap` + does not classify as `Cache` **before** the schema is applied. Nothing + scans, plays or authors in that database, so its non-catalog half is + empty by construction and a shape left over from an older schema is + pure liability. 013's reshaped `audio_files` failed every run of that + job on `CREATE INDEX ... album_id` against the old table until this; + `TestRetireLibraryTables` reproduces exactly that, symptom first. + **The local library is shaped like files, not like MusicBrainz.** `audio_files` carries its own tags — title, artist credit, track and disc numbers, year, composer, the recording MBID — and points at two diff --git a/cmd/indexbuild/main.go b/cmd/indexbuild/main.go index bc8ea9c..dd5f286 100644 --- a/cmd/indexbuild/main.go +++ b/cmd/indexbuild/main.go @@ -32,6 +32,7 @@ package main import ( + "context" "errors" "flag" "fmt" @@ -127,6 +128,13 @@ func run(o opts) error { return fmt.Errorf("resolve data dir: %w", err) } + // Before the schema is applied, not after: applying it over a table + // whose shape has since changed is what fails, and this database's + // non-catalog half is disposable. See retireLibraryTables. + if err := retireLibraryTables(context.Background(), logger); err != nil { + return fmt.Errorf("retire stale tables: %w", err) + } + db, err := database.NewDB(logger) if err != nil { return fmt.Errorf("open database: %w", err) diff --git a/cmd/indexbuild/staleschema.go b/cmd/indexbuild/staleschema.go new file mode 100644 index 0000000..c1562cc --- /dev/null +++ b/cmd/indexbuild/staleschema.go @@ -0,0 +1,150 @@ +//go:build indexbuild + +package main + +import ( + "context" + "database/sql" + "fmt" + "log/slog" + "path" + "strings" + + _ "modernc.org/sqlite" + + "yellowjacket/backend/datamap" + "yellowjacket/backend/system" +) + +// retireLibraryTables drops every table in the index database that is +// not part of the catalog, before the schema is applied over it. +// +// This database is not an install. Nothing scans a library into it, +// nothing plays a track, nothing authors a playlist: every table the +// datamap does not classify as Cache is empty by construction, and so +// is anything left over from a shape the schema no longer describes. +// The catalog is the opposite — it is the ~205 GB of dumps this job +// exists to avoid re-downloading, so it is never touched here. +// +// The alternative was to give this database a migration chain that the +// app deliberately does not have. `sql/schemas/` is CREATE ... IF NOT +// EXISTS, which reaches an *existing* table only if its shape already +// matches; plan 013 reshaped audio_files and every launch since has +// failed on "no such column: album_id" while applying an index to the +// old table. A user's answer to that is "delete and rescan" (plan 013, +// open question 1). This is that answer, for the one database where +// deleting the library half costs nothing and deleting the other half +// costs a day. +func retireLibraryTables(ctx context.Context, logger *slog.Logger) error { + dataDir, err := system.GetUserDataDirPath() + if err != nil { + return fmt.Errorf("resolve data dir: %w", err) + } + + db, err := sql.Open("sqlite", path.Join(dataDir, "yj.db")) + if err != nil { + return fmt.Errorf("open database: %w", err) + } + + defer func() { _ = db.Close() }() + + // Virtual tables first: dropping one takes its four shadow tables + // with it, so a second pass sees a schema with nothing dangling. + for _, virtual := range []bool{true, false} { + dropped, err := dropDisposable(ctx, db, virtual) + if err != nil { + return err + } + + if len(dropped) > 0 { + logger.Info( + "retired tables the catalog does not need", + "virtual", virtual, + "tables", dropped, + ) + } + } + + return nil +} + +// dropDisposable drops one pass of non-catalog objects and returns what +// it dropped. When virtual is true it considers only FTS5 virtual +// tables; otherwise it takes the ordinary tables and views left after +// that pass. +func dropDisposable( + ctx context.Context, + db *sql.DB, + virtual bool, +) ([]string, error) { + rows, err := db.QueryContext(ctx, ` + SELECT name, type, COALESCE(sql, '') + FROM sqlite_master + WHERE type IN ('table', 'view') + `) + if err != nil { + return nil, fmt.Errorf("read schema: %w", err) + } + + type object struct{ name, kind string } + + var doomed []object + + for rows.Next() { + var obj object + + var ddl string + + if err := rows.Scan(&obj.name, &obj.kind, &ddl); err != nil { + _ = rows.Close() + + return nil, fmt.Errorf("read schema: %w", err) + } + + isVirtual := strings.HasPrefix(ddl, "CREATE VIRTUAL") + if isVirtual != virtual || keepTable(obj.name) { + continue + } + + doomed = append(doomed, obj) + } + + _ = rows.Close() + + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("read schema: %w", err) + } + + names := make([]string, 0, len(doomed)) + + for _, obj := range doomed { + stmt := `DROP TABLE IF EXISTS "` + obj.name + `"` + if obj.kind == "view" { + stmt = `DROP VIEW IF EXISTS "` + obj.name + `"` + } + + if _, err := db.ExecContext(ctx, stmt); err != nil { + return nil, fmt.Errorf("drop %s: %w", obj.name, err) + } + + names = append(names, obj.name) + } + + return names, nil +} + +// keepTable reports whether an object survives. SQLite's own +// bookkeeping is not ours to drop, and Cache is the catalog and its +// neighbours — expensive to rebuild, which is the whole point of the +// persistent volume this runs against. Everything else goes, including +// tables the datamap has never heard of: an uncatalogued table in this +// database is one the schema stopped describing. +func keepTable(name string) bool { + if datamap.IsInternal(name) { + return true + } + + entry, known := datamap.Lookup(name) + + return known && entry.Kind == datamap.Cache +} diff --git a/cmd/indexbuild/staleschema_test.go b/cmd/indexbuild/staleschema_test.go new file mode 100644 index 0000000..6837638 --- /dev/null +++ b/cmd/indexbuild/staleschema_test.go @@ -0,0 +1,118 @@ +//go:build indexbuild + +package main + +import ( + "context" + "database/sql" + "log/slog" + "path/filepath" + "testing" + + _ "modernc.org/sqlite" + + "yellowjacket/backend/database" + "yellowjacket/backend/system" +) + +// TestRetireLibraryTables is the CI failure written down. +// +// The index job's persistent volume held a database from before plan +// 013 reshaped audio_files, so every run died applying an index to a +// column the old table does not have. The catalog in the same file is +// a day of downloading to rebuild and has to survive the repair. +func TestRetireLibraryTables(t *testing.T) { + logger := slog.New(slog.DiscardHandler) + + t.Setenv("YJ_HOME", t.TempDir()) + + dataDir, err := system.GetUserDataDirPath() + if err != nil { + t.Fatalf("resolve data dir: %v", err) + } + + dbPath := filepath.Join(dataDir, "yj.db") + + // Build the real thing, then put it back into the shape the volume + // was actually in: a pre-013 audio_files, and a table the schema + // stopped describing at all. + if _, err := database.NewDB(logger); err != nil { + t.Fatalf("first open: %v", err) + } + + exec(t, dbPath, ` + INSERT INTO explore_index (entity_type, mbid, title, artist_name, artist_mbid) + VALUES (1, randomblob(16), 'A Catalog Row', 'An Artist', randomblob(16)); + + DROP TABLE audio_files; + CREATE TABLE audio_files ( + id INTEGER PRIMARY KEY, + file_path TEXT NOT NULL UNIQUE, + recording_id INTEGER + ); + CREATE TABLE recordings (id INTEGER PRIMARY KEY, title TEXT); + `) + + // The symptom, before the repair: the schema cannot be applied over + // a table whose shape has moved on. + if _, err := database.NewDB(logger); err == nil { + t.Fatal("expected the stale shape to fail to open; it did not") + } + + if err := retireLibraryTables(context.Background(), logger); err != nil { + t.Fatalf("retireLibraryTables: %v", err) + } + + for _, table := range []string{"audio_files", "recordings"} { + if count(t, dbPath, sqliteMasterQuery(table)) != 0 { + t.Errorf("%s survived; the schema cannot be applied over it", table) + } + } + + if _, err := database.NewDB(logger); err != nil { + t.Fatalf("open after retiring stale tables: %v", err) + } + + // The catalog is why the volume exists. + if got := count(t, dbPath, "SELECT COUNT(*) FROM explore_index"); got != 1 { + t.Errorf("explore_index rows = %d, want 1 (the catalog must survive)", got) + } +} + +func sqliteMasterQuery(name string) string { + return `SELECT COUNT(*) FROM sqlite_master WHERE name = '` + name + `'` +} + +func exec(t *testing.T, dbPath, statements string) { + t.Helper() + + db, err := sql.Open("sqlite", dbPath) + if err != nil { + t.Fatalf("open: %v", err) + } + + defer func() { _ = db.Close() }() + + if _, err := db.Exec(statements); err != nil { + t.Fatalf("exec: %v", err) + } +} + +func count(t *testing.T, dbPath, query string) int { + t.Helper() + + db, err := sql.Open("sqlite", dbPath) + if err != nil { + t.Fatalf("open: %v", err) + } + + defer func() { _ = db.Close() }() + + var n int + + if err := db.QueryRow(query).Scan(&n); err != nil { + t.Fatalf("%s: %v", query, err) + } + + return n +}