From 5377c2b2b3d126dd37ae5dd9de44a2947cd41953 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 9 Jul 2026 12:56:54 -0400 Subject: [PATCH 1/2] fix(scanner): repair NOT NULL bpm/bit_depth columns left by out-of-order migration On some databases, migration 20260612222838 (which made media_file.bpm and bit_depth nullable) is recorded as applied in goose_db_version but its DDL never took effect, leaving the columns declared NOT NULL. Since v0.63.0 the scanner writes NULL for tracks without a BPM tag (PR #5603 changed the model fields to *int and removed the nil->0 shim), so scans on those databases fail with "NOT NULL constraint failed: media_file.bpm". Because goose considers the earlier migration already applied, it will never re-run it, so a new migration is required. This migration inspects the columns via PRAGMA table_info and only rebuilds the ones still declared NOT NULL, converting existing 0 values to NULL and preserving the media_file_bpm index. On healthy databases it is a no-op, avoiding a full media_file table rewrite. Fixes #5747 --- ...0260709160132_fix_bpm_bitdepth_not_null.go | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 db/migrations/20260709160132_fix_bpm_bitdepth_not_null.go diff --git a/db/migrations/20260709160132_fix_bpm_bitdepth_not_null.go b/db/migrations/20260709160132_fix_bpm_bitdepth_not_null.go new file mode 100644 index 000000000..8b8afca2a --- /dev/null +++ b/db/migrations/20260709160132_fix_bpm_bitdepth_not_null.go @@ -0,0 +1,95 @@ +package migrations + +import ( + "context" + "database/sql" + "fmt" + + "github.com/navidrome/navidrome/log" + "github.com/pressly/goose/v3" +) + +func init() { + goose.AddMigrationContext(upFixBpmBitdepthNotNull, downFixBpmBitdepthNotNull) +} + +// upFixBpmBitdepthNotNull repairs databases where media_file.bpm/bit_depth are +// still declared NOT NULL even though migration 20260612222838 (which made them +// nullable) is recorded as applied. On those instances the scanner writes a NULL +// bpm for tracks without a BPM tag and the write fails with +// "NOT NULL constraint failed: media_file.bpm" (issue #5747). Because goose +// considers the earlier migration already applied, it will never re-run it, so a +// fresh migration is required. It re-inspects the column definition and only +// rebuilds a column when it is still NOT NULL, making it a no-op on healthy DBs. +func upFixBpmBitdepthNotNull(ctx context.Context, tx *sql.Tx) error { + repaired := false + + // bpm carries the media_file_bpm index, which must be dropped before the + // column can be dropped and recreated after. + notNull, err := columnIsNotNull(ctx, tx, "media_file", "bpm") + if err != nil { + return err + } + if notNull { + if _, err = tx.ExecContext(ctx, ` +drop index if exists media_file_bpm; +alter table media_file add column bpm_new integer; +update media_file set bpm_new = nullif(bpm, 0); +alter table media_file drop column bpm; +alter table media_file rename column bpm_new to bpm; +create index if not exists media_file_bpm on media_file (bpm); +`); err != nil { + return err + } + repaired = true + } + + notNull, err = columnIsNotNull(ctx, tx, "media_file", "bit_depth") + if err != nil { + return err + } + if notNull { + if _, err = tx.ExecContext(ctx, ` +alter table media_file add column bit_depth_new integer; +update media_file set bit_depth_new = nullif(bit_depth, 0); +alter table media_file drop column bit_depth; +alter table media_file rename column bit_depth_new to bit_depth; +`); err != nil { + return err + } + repaired = true + } + + if repaired { + log.Warn(ctx, "Repaired media_file.bpm/bit_depth NOT NULL columns left over from an out-of-order migration (issue #5747)") + } + return nil +} + +func downFixBpmBitdepthNotNull(ctx context.Context, tx *sql.Tx) error { + // No-op: re-adding the NOT NULL constraint would reintroduce the bug. + return nil +} + +// columnIsNotNull reports whether the given column of the given table is declared +// with a NOT NULL constraint, using PRAGMA table_info. The table name is a fixed +// internal constant, so it is safe to interpolate (PRAGMA cannot bind parameters). +func columnIsNotNull(ctx context.Context, tx *sql.Tx, table, column string) (bool, error) { + rows, err := tx.QueryContext(ctx, fmt.Sprintf("PRAGMA table_info(%s)", table)) + if err != nil { + return false, err + } + defer rows.Close() + for rows.Next() { + var cid, notnull, pk int + var name, ctype string + var dflt sql.NullString + if err = rows.Scan(&cid, &name, &ctype, ¬null, &dflt, &pk); err != nil { + return false, err + } + if name == column { + return notnull == 1, nil + } + } + return false, rows.Err() +} From 81cb803980b3f2723dbb6ada43a919776615a507 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 9 Jul 2026 13:07:15 -0400 Subject: [PATCH 2/2] refactor(scanner): query column nullability with a single-row pragma_table_info select Replace the manual PRAGMA table_info iteration in columnIsNotNull with a single-row query against the pragma_table_info table-valued function, binding both the table and column names. Note that the "notnull" result column must be quoted, as NOTNULL is an SQLite operator keyword. Suggested by review, with corrections for the keyword quoting and errorlint-compliant ErrNoRows check. --- ...0260709160132_fix_bpm_bitdepth_not_null.go | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/db/migrations/20260709160132_fix_bpm_bitdepth_not_null.go b/db/migrations/20260709160132_fix_bpm_bitdepth_not_null.go index 8b8afca2a..3cd28c5f0 100644 --- a/db/migrations/20260709160132_fix_bpm_bitdepth_not_null.go +++ b/db/migrations/20260709160132_fix_bpm_bitdepth_not_null.go @@ -3,7 +3,7 @@ package migrations import ( "context" "database/sql" - "fmt" + "errors" "github.com/navidrome/navidrome/log" "github.com/pressly/goose/v3" @@ -72,24 +72,16 @@ func downFixBpmBitdepthNotNull(ctx context.Context, tx *sql.Tx) error { } // columnIsNotNull reports whether the given column of the given table is declared -// with a NOT NULL constraint, using PRAGMA table_info. The table name is a fixed -// internal constant, so it is safe to interpolate (PRAGMA cannot bind parameters). +// with a NOT NULL constraint. "notnull" must be quoted: NOTNULL is an SQLite +// operator keyword. func columnIsNotNull(ctx context.Context, tx *sql.Tx, table, column string) (bool, error) { - rows, err := tx.QueryContext(ctx, fmt.Sprintf("PRAGMA table_info(%s)", table)) + var notNull int + err := tx.QueryRowContext(ctx, `SELECT "notnull" FROM pragma_table_info(?) WHERE name = ?`, table, column).Scan(¬Null) + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } if err != nil { return false, err } - defer rows.Close() - for rows.Next() { - var cid, notnull, pk int - var name, ctype string - var dflt sql.NullString - if err = rows.Scan(&cid, &name, &ctype, ¬null, &dflt, &pk); err != nil { - return false, err - } - if name == column { - return notnull == 1, nil - } - } - return false, rows.Err() + return notNull == 1, nil }