From 1f0f7eee1adf4cd4572bf7340620f0a9f98225dd Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 12 May 2026 09:50:01 -0400 Subject: [PATCH] fix(playlists): unblock Discover by removing SELECT DISTINCT + ORDER BY plan error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cross-user bucket query combined SELECT DISTINCT with ORDER BY md5(...), which Postgres rejects at plan time (SQLSTATE 42P10). buildDiscoverCandidates returned that error on first bucket failure, so the whole Discover playlist was skipped every nightly run — even though the random bucket pool was healthy. Switched to GROUP BY so the md5 ordering expression no longer needs to appear in the select list, and hardened the function so future single- bucket failures degrade gracefully via slot redistribution instead of taking out the whole playlist. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/db/dbq/discover.sql.go | 8 +++++++- internal/db/queries/discover.sql | 8 +++++++- internal/playlists/discover.go | 20 +++++++++++++++----- internal/playlists/system.go | 5 ++++- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/internal/db/dbq/discover.sql.go b/internal/db/dbq/discover.sql.go index 3b9ad897..f4f8266a 100644 --- a/internal/db/dbq/discover.sql.go +++ b/internal/db/dbq/discover.sql.go @@ -12,7 +12,7 @@ import ( ) const listCrossUserLikedTracksForDiscover = `-- name: ListCrossUserLikedTracksForDiscover :many -SELECT DISTINCT t.id, t.album_id, t.artist_id +SELECT t.id, t.album_id, t.artist_id FROM general_likes gl JOIN tracks t ON t.id = gl.track_id WHERE gl.user_id != $1 @@ -30,6 +30,7 @@ SELECT DISTINCT t.id, t.album_id, t.artist_id SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) + GROUP BY t.id, t.album_id, t.artist_id ORDER BY md5(t.id::text || $2::text) LIMIT 60 ` @@ -50,6 +51,11 @@ type ListCrossUserLikedTracksForDiscoverRow struct { // rows (the caller's slot redistribution rolls the deficit into the // other two buckets). // $1 = user_id, $2 = date string for md5 ordering. +// +// GROUP BY (not SELECT DISTINCT) so the md5 ORDER BY expression need +// not appear in the select list — Postgres rejects DISTINCT + ORDER BY +// by-expression at plan time (SQLSTATE 42P10), which previously caused +// the entire Discover build to fail silently. func (q *Queries) ListCrossUserLikedTracksForDiscover(ctx context.Context, arg ListCrossUserLikedTracksForDiscoverParams) ([]ListCrossUserLikedTracksForDiscoverRow, error) { rows, err := q.db.Query(ctx, listCrossUserLikedTracksForDiscover, arg.UserID, arg.Column2) if err != nil { diff --git a/internal/db/queries/discover.sql b/internal/db/queries/discover.sql index 8d13392f..3f3762d4 100644 --- a/internal/db/queries/discover.sql +++ b/internal/db/queries/discover.sql @@ -52,7 +52,12 @@ SELECT t.id, t.album_id, t.artist_id -- rows (the caller's slot redistribution rolls the deficit into the -- other two buckets). -- $1 = user_id, $2 = date string for md5 ordering. -SELECT DISTINCT t.id, t.album_id, t.artist_id +-- +-- GROUP BY (not SELECT DISTINCT) so the md5 ORDER BY expression need +-- not appear in the select list — Postgres rejects DISTINCT + ORDER BY +-- by-expression at plan time (SQLSTATE 42P10), which previously caused +-- the entire Discover build to fail silently. +SELECT t.id, t.album_id, t.artist_id FROM general_likes gl JOIN tracks t ON t.id = gl.track_id WHERE gl.user_id != $1 @@ -70,6 +75,7 @@ SELECT DISTINCT t.id, t.album_id, t.artist_id SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) + GROUP BY t.id, t.album_id, t.artist_id ORDER BY md5(t.id::text || $2::text) LIMIT 60; diff --git a/internal/playlists/discover.go b/internal/playlists/discover.go index ec97ce04..f86be276 100644 --- a/internal/playlists/discover.go +++ b/internal/playlists/discover.go @@ -2,7 +2,7 @@ package playlists import ( "context" - "fmt" + "log/slog" "github.com/jackc/pgx/v5/pgtype" @@ -34,24 +34,34 @@ type discoverTrack struct { // Returns up to discoverTotalSlots track IDs in the order they should // appear in the playlist (round-robin interleaved across buckets). // Empty slice when the library has no eligible tracks at all. -func buildDiscoverCandidates(ctx context.Context, q *dbq.Queries, userID pgtype.UUID, dateStr string) ([]rankedCandidate, error) { +// +// Individual bucket query failures are logged and treated as empty — +// the redistribution algorithm rolls the deficit into the surviving +// buckets so one broken bucket can't silently kill the whole playlist. +func buildDiscoverCandidates(ctx context.Context, q *dbq.Queries, logger *slog.Logger, userID pgtype.UUID, dateStr string) ([]rankedCandidate, error) { dormantRows, err := q.ListDormantArtistTracksForDiscover(ctx, dbq.ListDormantArtistTracksForDiscoverParams{ UserID: userID, Column2: dateStr, }) if err != nil { - return nil, fmt.Errorf("dormant bucket: %w", err) + logger.Warn("discover: dormant bucket failed; continuing with empty pool", + "user_id", uuidStringPL(userID), "err", err) + dormantRows = nil } crossUserRows, err := q.ListCrossUserLikedTracksForDiscover(ctx, dbq.ListCrossUserLikedTracksForDiscoverParams{ UserID: userID, Column2: dateStr, }) if err != nil { - return nil, fmt.Errorf("cross-user bucket: %w", err) + logger.Warn("discover: cross-user bucket failed; continuing with empty pool", + "user_id", uuidStringPL(userID), "err", err) + crossUserRows = nil } randomRows, err := q.ListRandomUnheardTracksForDiscover(ctx, dbq.ListRandomUnheardTracksForDiscoverParams{ UserID: userID, Column2: dateStr, }) if err != nil { - return nil, fmt.Errorf("random bucket: %w", err) + logger.Warn("discover: random bucket failed; continuing with empty pool", + "user_id", uuidStringPL(userID), "err", err) + randomRows = nil } // Adapt sqlc-generated rows into the internal struct, then apply diff --git a/internal/playlists/system.go b/internal/playlists/system.go index 4d76fd73..e0a5b474 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -294,7 +294,10 @@ func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog. } // 4. Discover: surface unheard tracks, biased toward dormant artists. - discoverTracks, derr := buildDiscoverCandidates(ctx, q, userID, dateStr) + // Individual bucket failures are logged inside buildDiscoverCandidates + // and redistribute as deficit; the function only returns an error for + // unrecoverable conditions, so a remaining derr here is genuine. + discoverTracks, derr := buildDiscoverCandidates(ctx, q, logger, userID, dateStr) if derr != nil { logger.Warn("system playlist: discover candidates load failed; skipping", "user_id", uuidStringPL(userID), "err", derr)