From 65751d7665e3f1a85c96f661a10227a77bb655d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Mon, 17 Aug 2026 15:47:41 -0400
Subject: [PATCH] fix(playlists): chunk track deletes to stay under the SQLite
 variable limit (#5977)

PlaylistTrackRepository.Delete built a single IN clause with one bind variable per
track, so removing more tracks than SQLITE_MAX_VARIABLE_NUMBER (32766) failed with
"too many SQL variables". Clients that sync a large playlist by adding the desired
tracks and then removing the stale ones would get the add committed and the removal
rejected, leaving the playlist with both sets of tracks and growing it on every sync.

Delete now works in chunks of 200, the same size addTracks already uses, and renumbers
once after the last chunk. Both callers already run inside a transaction, so the
delete stays atomic.
---
 persistence/playlist_track_repository.go      | 11 ++--
 persistence/playlist_track_repository_test.go | 50 +++++++++++++++++++
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/persistence/playlist_track_repository.go b/persistence/playlist_track_repository.go
index c1f6fcf69..a5e1975fd 100644
--- a/persistence/playlist_track_repository.go
+++ b/persistence/playlist_track_repository.go
@@ -2,6 +2,7 @@ package persistence
 
 import (
 	"database/sql"
+	"slices"
 
 	. "github.com/Masterminds/squirrel"
 	"github.com/deluan/rest"
@@ -224,10 +225,14 @@ func (r *playlistTrackRepository) AddDiscs(discs []model.DiscID) (int, error) {
 	return r.addMediaFileIds(clauses)
 }
 
+// deleteChunkSize keeps each DELETE under SQLITE_MAX_VARIABLE_NUMBER, matching addTracks.
+const deleteChunkSize = 200
+
 func (r *playlistTrackRepository) Delete(ids ...string) error {
-	err := r.delete(And{Eq{"playlist_id": r.playlistId}, Eq{"id": ids}})
-	if err != nil {
-		return err
+	for chunk := range slices.Chunk(ids, deleteChunkSize) {
+		if err := r.delete(And{Eq{"playlist_id": r.playlistId}, Eq{"id": chunk}}); err != nil {
+			return err
+		}
 	}
 
 	return r.playlistRepo.renumber(r.playlistId)
diff --git a/persistence/playlist_track_repository_test.go b/persistence/playlist_track_repository_test.go
index a5c67b92c..88ddbff48 100644
--- a/persistence/playlist_track_repository_test.go
+++ b/persistence/playlist_track_repository_test.go
@@ -1,6 +1,8 @@
 package persistence
 
 import (
+	"strconv"
+
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
@@ -8,6 +10,9 @@ import (
 	. "github.com/onsi/gomega"
 )
 
+// sqliteMaxVariables is SQLITE_MAX_VARIABLE_NUMBER as compiled into the driver
+const sqliteMaxVariables = 32766
+
 var _ = Describe("PlaylistTrackRepository", func() {
 	var repo model.PlaylistTrackRepository
 
@@ -72,4 +77,49 @@ var _ = Describe("PlaylistTrackRepository", func() {
 				To(Equal([]string{songRadioactivity.ID}))
 		})
 	})
+
+	Describe("Delete", func() {
+		var tracks model.PlaylistTrackRepository
+		const numTracks = deleteChunkSize*2 + 1
+
+		positionsUpTo := func(n int) []string {
+			positions := make([]string, 0, n)
+			for i := 1; i <= n; i++ {
+				positions = append(positions, strconv.Itoa(i))
+			}
+			return positions
+		}
+
+		BeforeEach(func() {
+			ctx := log.NewContext(GinkgoT().Context())
+			ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true})
+			plsRepo := NewPlaylistRepository(ctx, GetDBXBuilder())
+
+			pls := model.Playlist{Name: "Chunked Delete", OwnerID: "userid", OwnerName: "userid"}
+			Expect(plsRepo.Put(&pls)).To(Succeed())
+			DeferCleanup(func() { Expect(plsRepo.Delete(pls.ID)).To(Succeed()) })
+
+			tracks = plsRepo.Tracks(pls.ID, false)
+			songIds := make([]string, numTracks)
+			for i := range songIds {
+				songIds[i] = songDayInALife.ID
+			}
+			Expect(tracks.Add(songIds)).To(Equal(numTracks))
+		})
+
+		It("removes positions spanning several chunks, and renumbers what is left", func() {
+			Expect(tracks.Delete(positionsUpTo(numTracks - 1)...)).To(Succeed())
+
+			Expect(tracks.CountAll()).To(Equal(int64(1)))
+			remaining, err := tracks.GetAll(model.QueryOptions{Sort: "id"})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(remaining[0].ID).To(Equal("1"), "the surviving track must be renumbered to position 1")
+		})
+
+		It("accepts more ids than SQLite allows as bind variables", func() {
+			Expect(tracks.Delete(positionsUpTo(sqliteMaxVariables + 100)...)).To(Succeed())
+
+			Expect(tracks.CountAll()).To(BeZero())
+		})
+	})
 })