From d4387c550279cf9fcb8f3f5614f694aaa9b3b165 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 3 Jul 2026 08:58:55 -0400 Subject: [PATCH] perf(db): index media_file album/artist sort orders (#5706) * perf(db): add composite indexes for song list album/artist sorts The media_file sort mappings for album, artist and albumArtist expand to multi-column ORDER BY clauses that no existing index could satisfy, so SQLite fell back to a full table scan plus a temp B-tree sort of every row (including the large lyrics/tags/full_text columns) even for a single 15-item page. On a 96K-track library this made /api/song?_sort=album take 3.6s on a cold cache. Add composite indexes matching the three sort mappings, allowing the query to walk the index and stop at the page size, in both directions. Drop the now redundant single-column order_album_name/order_artist_name indexes (strict prefixes of the new composites) and three indexes with no query path: birth_time is only read in Go code, and artist/album_artist text column lookups go through the media_file_artists table instead. * fix(ui): make composer and track number columns non-sortable in song list Clicking the Composer header was a silent no-op: composer is not a media_file column, so the native API's sanitizeSort drops the sort and returns rows in table order. Track number sorting across the whole library is not meaningful and cannot use an index (the existing index leads with disc_number). Mark both columns sortable={false}, like quality and mood. * test(persistence): add sort index coverage test for large tables Guard against sort options silently losing index support: every sort mapping on media_file, album and artist is now verified with EXPLAIN QUERY PLAN to be satisfiable by an index (both directions), so adding a mapping or dropping an index that reintroduces a full-table temp B-tree sort fails the test. Sorts that genuinely cannot use an index (random, annotation-join columns, JSON expressions) must be declared in an exceptions list with the reason, keeping the trade-off visible in review. To make the sort mappings the complete declared sort surface, add identity mappings for the media_file columns the UI sorts by without a mapping (year, genre, duration, channels, bpm, path, comment, play_count, play_date, rating). These are behaviorally no-ops: the same ORDER BY was previously produced by the field whitelist fallback. * perf(db): drop PreferSortTags expression indexes from media_file The media_file sort_title/sort_artist_name/sort_album_name expression indexes are only usable when PreferSortTags is enabled - a config reported by ~0.1% of installations (insights, week of 2026-06-22) - yet every install pays their storage (~8.6MB on a 96K-track library) and scanner write overhead. Drop them: PreferSortTags installs fall back to a full sort for title/artist/album orders, everyone else gets smaller DBs and cheaper writes. The order_album_name and order_artist_name collation checks remain valid, now satisfied by the composite sort indexes. Signed-off-by: Deluan --------- Signed-off-by: Deluan --- ...13908_optimize_media_file_sort_indexes.sql | 56 ++++++ persistence/collation_test.go | 3 - persistence/mediafile_repository.go | 10 ++ persistence/sort_index_coverage_test.go | 168 ++++++++++++++++++ ui/src/song/SongList.jsx | 6 +- 5 files changed, 238 insertions(+), 5 deletions(-) create mode 100644 db/migrations/20260703013908_optimize_media_file_sort_indexes.sql create mode 100644 persistence/sort_index_coverage_test.go diff --git a/db/migrations/20260703013908_optimize_media_file_sort_indexes.sql b/db/migrations/20260703013908_optimize_media_file_sort_indexes.sql new file mode 100644 index 000000000..dd36bf4c2 --- /dev/null +++ b/db/migrations/20260703013908_optimize_media_file_sort_indexes.sql @@ -0,0 +1,56 @@ +-- +goose Up +-- +goose StatementBegin + +-- Composite indexes matching the media_file sort mappings for album, artist and +-- albumArtist. Without them, SQLite cannot satisfy the multi-column ORDER BY and +-- falls back to a full scan + temp B-tree sort of the whole table (including all +-- its large columns) even for a small LIMIT. +create index if not exists media_file_album_sort + on media_file(order_album_name, album_id, disc_number, track_number, order_artist_name, title); +create index if not exists media_file_artist_sort + on media_file(order_artist_name, order_album_name, release_date, disc_number, track_number); +create index if not exists media_file_album_artist_sort + on media_file(order_album_artist_name, order_album_name, release_date, disc_number, track_number); + +-- These two are strict prefixes of the composites above, so they are redundant now. +drop index if exists media_file_order_album_name; +drop index if exists media_file_order_artist_name; + +-- No query filters or sorts on these columns: birth_time is only read in Go code; +-- artist/album_artist conditions go through the media_file_artists table. +drop index if exists media_file_birth_time; +drop index if exists media_file_artist; +drop index if exists media_file_album_artist; + +-- These expression indexes are only usable when PreferSortTags is enabled, a +-- config used by ~0.1% of installations (per insights), yet they are maintained +-- on every write of every install. Dropping them means those installs fall back +-- to a full sort; everyone else saves the space and the scanner write overhead. +drop index if exists media_file_sort_title; +drop index if exists media_file_sort_artist_name; +drop index if exists media_file_sort_album_name; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +drop index if exists media_file_album_sort; +drop index if exists media_file_artist_sort; +drop index if exists media_file_album_artist_sort; + +create index if not exists media_file_order_album_name + on media_file(order_album_name); +create index if not exists media_file_order_artist_name + on media_file(order_artist_name); +create index if not exists media_file_birth_time + on media_file(birth_time); +create index if not exists media_file_artist + on media_file(artist); +create index if not exists media_file_album_artist + on media_file(album_artist); +create index if not exists media_file_sort_title + on media_file (coalesce(nullif(sort_title,''),order_title) collate NOCASE); +create index if not exists media_file_sort_artist_name + on media_file (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NOCASE); +create index if not exists media_file_sort_album_name + on media_file (coalesce(nullif(sort_album_name,''),order_album_name) collate NOCASE); +-- +goose StatementEnd diff --git a/persistence/collation_test.go b/persistence/collation_test.go index bb1276577..dff91148e 100644 --- a/persistence/collation_test.go +++ b/persistence/collation_test.go @@ -50,9 +50,6 @@ var _ = Describe("Collation", func() { Entry("media_file.order_title", "media_file", "order_title collate nocase"), Entry("media_file.order_album_name", "media_file", "order_album_name collate nocase"), Entry("media_file.order_artist_name", "media_file", "order_artist_name collate nocase"), - Entry("media_file.sort_title", "media_file", "coalesce(nullif(sort_title,''),order_title) collate nocase"), - Entry("media_file.sort_album_name", "media_file", "coalesce(nullif(sort_album_name,''),order_album_name) collate nocase"), - Entry("media_file.sort_artist_name", "media_file", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate nocase"), Entry("media_file.path", "media_file", "path collate nocase"), Entry("playlist.name", "playlist", "name collate nocase"), Entry("radio.name", "radio", "name collate nocase"), diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index 094268783..b4979ca77 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -91,6 +91,16 @@ func NewMediaFileRepository(ctx context.Context, db dbx.Builder) model.MediaFile "recently_added": mediaFileRecentlyAddedSort(), "starred_at": "starred, starred_at", "rated_at": "rating, rated_at", + "year": "year", + "genre": "genre", + "duration": "duration", + "channels": "channels", + "bpm": "bpm", + "path": "path", + "comment": "comment", + "play_count": "play_count", + "play_date": "play_date", + "rating": "rating", }) return r } diff --git a/persistence/sort_index_coverage_test.go b/persistence/sort_index_coverage_test.go new file mode 100644 index 000000000..b5dea231d --- /dev/null +++ b/persistence/sort_index_coverage_test.go @@ -0,0 +1,168 @@ +package persistence + +import ( + "context" + "database/sql" + "fmt" + "maps" + "regexp" + "slices" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/db" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// These tests guard against sort options silently losing index support: adding or +// changing a sort mapping, or dropping/renaming an index in a migration, must not +// reintroduce full-table temp B-tree sorts on the large tables. Those are +// catastrophic on big libraries but invisible on dev-sized ones, which is how the +// unindexed album/artist song sorts went unnoticed for years. +// +// Every sort mapping is checked automatically: the real ORDER BY is built via +// buildSortOrder (both directions) and verified with EXPLAIN QUERY PLAN against +// the migrated test schema. The planner's choice is deterministic even on an +// empty table. A sort passes when the plan has no full "USE TEMP B-TREE FOR +// ORDER BY" step; an incremental sort of tie groups ("... FOR LAST TERM OF ORDER +// BY") is fine, as it only sorts rows with equal leading columns. +// +// A new sort mapping therefore fails this test until a matching index is created. +// The only escape hatch is exceptions, for sorts that genuinely cannot be +// served by a table index (random, annotation-join columns, JSON expressions): +// declaring one requires writing down the reason, making the trade-off visible in +// review. The checks run with the default config: PreferSortTags=true rewrites +// mappings to coalesce expressions with no matching indexes (used by ~0.1% of +// installations, per insights), and is out of scope here. +var _ = Describe("Sort index coverage", func() { + conn := db.Db() + + type repoCase struct { + table string + newRepo func(ctx context.Context) *sqlRepository + // sort mapping -> reason it cannot be served by an index + exceptions map[string]string + } + + cases := []repoCase{ + { + table: "media_file", + newRepo: func(ctx context.Context) *sqlRepository { + return &NewMediaFileRepository(ctx, GetDBXBuilder()).(*mediaFileRepository).sqlRepository + }, + exceptions: map[string]string{ + "random": "not a column sort", + "starred_at": "sorts on annotation join columns", + "rated_at": "sorts on annotation join columns", + "play_count": "sorts on annotation join columns", + "play_date": "sorts on annotation join columns", + "rating": "sorts on annotation join columns", + "comment": "UI-sortable but rarely used; not worth an index", + }, + }, + { + table: "album", + newRepo: func(ctx context.Context) *sqlRepository { + return &NewAlbumRepository(ctx, GetDBXBuilder()).(*albumRepository).sqlRepository + }, + exceptions: map[string]string{ + "random": "not a column sort", + "starred_at": "sorts on annotation join columns", + "rated_at": "sorts on annotation join columns", + "max_year": "coalesce expression over original_date/max_year, no expression index", + }, + }, + { + table: "artist", + newRepo: func(ctx context.Context) *sqlRepository { + return &NewArtistRepository(ctx, GetDBXBuilder()).(*artistRepository).sqlRepository + }, + exceptions: map[string]string{ //nolint:gosec // G101 false positive, same as the artist sortMappings + "starred_at": "sorts on annotation join columns", + "rated_at": "sorts on annotation join columns", + "song_count": "JSON expression over stats column", + "album_count": "JSON expression over stats column", + "size": "JSON expression over stats column", + "maincredit_song_count": "aggregate over JSON stats", + "maincredit_album_count": "aggregate over JSON stats", + "maincredit_size": "aggregate over JSON stats", + }, + }, + } + + newCtx := func() context.Context { + ctx := log.NewContext(GinkgoT().Context()) + return request.WithUser(ctx, model.User{ID: "userid"}) + } + + for _, c := range cases { + It(fmt.Sprintf("uses an index for every sort mapping on %s", c.table), func() { + r := c.newRepo(newCtx()) + for _, sort := range slices.Sorted(maps.Keys(r.sortMappings)) { + if _, ok := c.exceptions[sort]; ok { + continue + } + for _, dir := range []string{"asc", "desc"} { + orderBy := r.buildSortOrder(sort, dir) + Expect(checkSortUsesIndex(conn, c.table, orderBy)).To(Succeed(), + "sort %q (%s) on table %q needs an index. Create one matching its ORDER BY, or, if it cannot be served by an index, add it to exceptions with the reason", + sort, dir, c.table) + } + } + }) + + It(fmt.Sprintf("has no stale exceptions entries for %s", c.table), func() { + r := c.newRepo(newCtx()) + for _, sort := range slices.Sorted(maps.Keys(c.exceptions)) { + Expect(r.sortMappings).To(HaveKey(sort), + "exceptions entry %q on table %q does not match any sort mapping - remove it", sort, c.table) + } + }) + } + + It("uses an index for recently_added when RecentlyAddedByModTime is enabled", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.RecentlyAddedByModTime = true + for _, c := range cases[:2] { // media_file and album + r := c.newRepo(newCtx()) + for _, dir := range []string{"asc", "desc"} { + orderBy := r.buildSortOrder("recently_added", dir) + Expect(checkSortUsesIndex(conn, c.table, orderBy)).To(Succeed(), + "sort recently_added (%s) on table %q", dir, c.table) + } + } + }) +}) + +// Matches the full-sort step only: incremental tie-group sorts are reported as +// "USE TEMP B-TREE FOR LAST TERM OF ORDER BY" (or "LAST N TERMS") and are allowed. +var fullTempBTreeSort = regexp.MustCompile(`USE TEMP B-TREE FOR ORDER BY`) + +func checkSortUsesIndex(conn *sql.DB, table, orderBy string) error { + rows, err := conn.Query(fmt.Sprintf("explain query plan select * from %s order by %s limit 15", table, orderBy)) + if err != nil { + return fmt.Errorf("explain query plan failed for order by %q: %w", orderBy, err) + } + defer rows.Close() + + var details []string + for rows.Next() { + var id, parent, notUsed int + var detail string + if err := rows.Scan(&id, &parent, ¬Used, &detail); err != nil { + return err + } + details = append(details, detail) + } + if err := rows.Err(); err != nil { + return err + } + if slices.ContainsFunc(details, fullTempBTreeSort.MatchString) { + return fmt.Errorf("no index satisfies ORDER BY %s - plan: %v", orderBy, details) + } + return nil +} diff --git a/ui/src/song/SongList.jsx b/ui/src/song/SongList.jsx index d928af549..d44992d0c 100644 --- a/ui/src/song/SongList.jsx +++ b/ui/src/song/SongList.jsx @@ -143,9 +143,11 @@ const SongList = (props) => { return { album: isDesktop && , artist: , - composer: , + composer: , albumArtist: , - trackNumber: isDesktop && , + trackNumber: isDesktop && ( + + ), playCount: isDesktop && ( ),