fix(persistence): keep song pages correct past the large-offset threshold

Past DevOffsetOptimize (50,000 by default) GetAll and the legacy LIKE
search exclude earlier rows with a "rowid NOT IN (subquery)". The
subquery selected only the rowid, so its ORDER BY could not use the
outer column names and SQLite matched them against every joined table.
Sorting songs by id, path, updatedAt or comment failed with "ambiguous
column name", so /api/song returned 500 for those sorts. Album legacy
search failed the same way on name.

The subquery and the outer query also ordered tied rows differently, so
sorts with many equal values (starred, rating, playCount, ...) repeated
some songs and skipped others once past the threshold, without errors.

The subquery now keeps the outer projection, so ORDER BY resolves the
same way in both queries. Paginated queries get a rowid tie-breaker, so
the order is total and every page agrees on where ties go. Every index
ends with the rowid, so index-served sorts stay index-served. createdAt
and updatedAt now sort by (date, id) to match their (date, id) indexes;
with the date alone, the rowid tie-breaker would need a temp B-tree.

Fixes #6285

persistence/mediafile_repository.go

@@ -96,7 +96,8 @@ func NewMediaFileRepository(db dbx.Builder) model.MediaFileRepository {
 		"album_artist":   "order_album_artist_name, order_album_name, release_date, disc_number, track_number",
 		"album":          "order_album_name, album_id, disc_number, track_number, order_artist_name, " + naturalSort("media_file.title"),
 		"random":         "random",
-		"created_at":     "media_file.created_at",
+		"created_at":     "media_file.created_at, media_file.id",
+		"updated_at":     "media_file.updated_at, media_file.id",
 		"recently_added": mediaFileRecentlyAddedSort(),
 		"starred_at":     "starred, starred_at",
 		"rated_at":       "rating, rated_at",

persistence/sql_base_repository.go

@@ -409,9 +409,7 @@ func wrapCursor[D, T any](cursor iter.Seq2[D, error], toModel func(D) *T) iter.S
 // queryWithStableResults is a helper function to execute a query and return an iterator that will yield its results
 // from a cursor, guaranteeing that the results will be stable, even if the underlying data changes.
 func queryWithStableResults[T any](ctx context.Context, r sqlRepository, sq SelectBuilder, options ...model.QueryOptions) (iter.Seq2[T, error], error) {
-	if len(options) > 0 && options[0].Offset > 0 {
-		sq = r.optimizePagination(sq, options[0])
-	}
+	sq = r.paginate(sq, options...)
 	query, args, err := r.toSQL(sq)
 	if err != nil {
 		return nil, err
@@ -439,9 +437,7 @@ func queryWithStableResults[T any](ctx context.Context, r sqlRepository, sq Sele
 }
 
 func (r sqlRepository) queryAll(ctx context.Context, sq SelectBuilder, response any, options ...model.QueryOptions) error {
-	if len(options) > 0 && options[0].Offset > 0 {
-		sq = r.optimizePagination(sq, options[0])
-	}
+	sq = r.paginate(sq, options...)
 	query, args, err := r.toSQL(sq)
 	if err != nil {
 		return err
@@ -472,15 +468,28 @@ func (r sqlRepository) queryAllSlice(ctx context.Context, sq SelectBuilder, resp
 	return err
 }
 
+// paginate adds a rowid tie-breaker, so the order is total and rows with equal sort values land on the
+// same page every time. Indexes already end with the rowid, so index-served sorts stay index-served.
+func (r sqlRepository) paginate(sq SelectBuilder, options ...model.QueryOptions) SelectBuilder {
+	if len(options) == 0 || (options[0].Max == 0 && options[0].Offset == 0) {
+		return sq
+	}
+	order := "asc"
+	if strings.EqualFold(strings.TrimSpace(options[0].Order), "desc") {
+		order = "desc"
+	}
+	return r.optimizePagination(sq.OrderBy(r.tableName+".rowid "+order), options[0])
+}
+
 // optimizePagination uses a less inefficient pagination, by not using OFFSET.
 // See https://gist.github.com/ssokolow/262503
 func (r sqlRepository) optimizePagination(sq SelectBuilder, options model.QueryOptions) SelectBuilder {
 	if options.Offset > conf.Server.DevOffsetOptimize {
 		sq = sq.RemoveOffset()
-		rowidSq := sq.RemoveColumns().Columns(r.tableName + ".rowid")
-		rowidSq = rowidSq.Limit(uint64(options.Offset))
+		// Keep the outer projection, so ORDER BY names resolve to the same columns in both queries.
+		rowidSq := sq.Column(r.tableName + ".rowid as _paginated_rowid").Limit(uint64(options.Offset))
 		rowidSql, args, _ := rowidSq.ToSql()
-		sq = sq.Where(r.tableName+".rowid not in ("+rowidSql+")", args...)
+		sq = sq.Where(r.tableName+".rowid not in (SELECT _paginated_rowid FROM ("+rowidSql+"))", args...)
 	}
 	return sq
 }

persistence/sql_base_repository_test.go

@@ -2,8 +2,12 @@ package persistence
 
 import (
 	"context"
+	"slices"
 
 	"github.com/Masterminds/squirrel"
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/conf/configtest"
+	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
 	"github.com/navidrome/navidrome/utils/hasher"
@@ -421,3 +425,133 @@ var _ = Describe("sqlRepository", func() {
 		})
 	})
 })
+
+var _ = Describe("paginated queries", func() {
+	var ctx context.Context
+	var mr model.MediaFileRepository
+
+	BeforeEach(func() {
+		ctx = request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
+		DeferCleanup(configtest.SetupConfig())
+		mr = NewMediaFileRepository(GetDBXBuilder())
+	})
+
+	Describe("SQL", func() {
+		var r sqlRepository
+		BeforeEach(func() {
+			r = sqlRepository{tableName: "media_file", sortMappings: map[string]string{"name": "title"}}
+			conf.Server.DevOffsetOptimize = 10
+		})
+
+		DescribeTable("adds a rowid tie-breaker in the requested direction",
+			func(options model.QueryOptions, expectedOrder string) {
+				sql, _, err := r.paginate(squirrel.Select("*").From("media_file"), options).ToSql()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(sql).To(HaveSuffix(expectedOrder))
+			},
+			Entry("no sort", model.QueryOptions{Max: 5}, "ORDER BY media_file.rowid asc"),
+			Entry("desc", model.QueryOptions{Sort: "name", Order: "desc", Max: 5}, "ORDER BY media_file.rowid desc"),
+		)
+
+		It("leaves unpaginated queries alone", func() {
+			sql, _, _ := r.paginate(squirrel.Select("*").From("media_file"), model.QueryOptions{Sort: "name"}).ToSql()
+			Expect(sql).To(Equal("SELECT * FROM media_file"))
+		})
+
+		It("uses the rowid subquery only past the threshold, keeping the outer projection", func() {
+			sq := squirrel.Select("media_file.*", "library.path as library_path").From("media_file").OrderBy("id asc")
+			atThreshold, _, _ := r.paginate(sq.Limit(5).Offset(10), model.QueryOptions{Max: 5, Offset: 10}).ToSql()
+			Expect(atThreshold).ToNot(ContainSubstring("not in"))
+
+			past, _, _ := r.paginate(sq.Limit(5).Offset(11), model.QueryOptions{Max: 5, Offset: 11}).ToSql()
+			Expect(past).To(ContainSubstring("media_file.rowid not in (SELECT _paginated_rowid FROM (" +
+				"SELECT media_file.*, library.path as library_path, media_file.rowid as _paginated_rowid FROM media_file"))
+			Expect(past).ToNot(ContainSubstring("OFFSET"))
+		})
+	})
+
+	// walk pages through the repository, crossing the optimizer threshold
+	walk := func(ctx context.Context, sort, order string, pageSize int) []string {
+		var ids []string
+		for offset := 0; ; offset += pageSize {
+			page, err := mr.GetAll(ctx, model.QueryOptions{Sort: sort, Order: order, Offset: offset, Max: pageSize, Seed: "seed"})
+			Expect(err).ToNot(HaveOccurred(), "sort=%s order=%s offset=%d", sort, order, offset)
+			for _, m := range page {
+				ids = append(ids, m.ID)
+			}
+			if len(page) < pageSize {
+				return ids
+			}
+		}
+	}
+
+	DescribeTable("returns every song exactly once, in the same order as a single page",
+		func(sort string) {
+			conf.Server.DevOffsetOptimize = 2
+			for _, order := range []string{"asc", "desc"} {
+				expected := walk(ctx, sort, order, 1000)
+				Expect(len(expected)).To(BeNumerically(">=", len(testSongs)))
+				Expect(slices.Compact(slices.Sorted(slices.Values(expected)))).To(HaveLen(len(expected)))
+				// Page size 2 hits offset == threshold (plain OFFSET), size 3 hits offset > threshold first.
+				for _, pageSize := range []int{2, 3} {
+					Expect(walk(ctx, sort, order, pageSize)).To(Equal(expected), "sort=%s order=%s page=%d", sort, order, pageSize)
+				}
+			}
+		},
+		Entry("id", "id"),
+		Entry("path", "path"),
+		Entry("updatedAt", "updatedAt"),
+		Entry("comment", "comment"),
+		Entry("title", "title"),
+		Entry("createdAt", "createdAt"),
+		Entry("starred (ties)", "starred"),
+		Entry("playCount (ties)", "playCount"),
+		Entry("rating (ties)", "rating"),
+		Entry("album", "album"),
+		Entry("seeded random", "random"),
+	)
+
+	It("keeps the library filter on every page", func() {
+		_, otherLib, restrictedUser := restrictedFixture("paginate")
+		hidden := model.MediaFile{ID: "paginate-hidden", Title: "Hidden", LibraryID: otherLib.ID, Path: p("other/hidden.mp3")}
+		Expect(mr.Put(ctx, &hidden)).To(Succeed())
+		DeferCleanup(func() { _ = mr.Delete(ctx, hidden.ID) })
+		conf.Server.DevOffsetOptimize = 2
+
+		Expect(walk(ctx, "id", "asc", 1000)).To(ContainElement(hidden.ID))
+		restrictedCtx := request.WithUser(ctx, restrictedUser)
+		expected := walk(restrictedCtx, "id", "asc", 1000)
+		Expect(expected).ToNot(BeEmpty())
+		Expect(expected).ToNot(ContainElement(hidden.ID))
+		Expect(walk(restrictedCtx, "id", "asc", 3)).To(Equal(expected))
+	})
+
+	Describe("legacy search", func() {
+		BeforeEach(func() {
+			conf.Server.Search.Backend = "legacy"
+			conf.Server.DevOffsetOptimize = 0
+		})
+
+		It("pages albums whose name is shared with the joined library", func() {
+			alr := NewAlbumRepository(GetDBXBuilder())
+			all, err := alr.Search(ctx, "abbey", model.QueryOptions{Max: 10})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(all).To(HaveLen(2))
+			second, err := alr.Search(ctx, "abbey", model.QueryOptions{Max: 1, Offset: 1})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(second).To(HaveLen(1))
+			Expect(second[0].ID).To(Equal(all[1].ID))
+		})
+
+		It("pages artists", func() {
+			arr := NewArtistRepository(GetDBXBuilder())
+			all, err := arr.Search(ctx, "the", model.QueryOptions{Max: 10})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(len(all)).To(BeNumerically(">=", 2))
+			second, err := arr.Search(ctx, "the", model.QueryOptions{Max: 1, Offset: 1})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(second).To(HaveLen(1))
+			Expect(second[0].ID).To(Equal(all[1].ID))
+		})
+	})
+})