perf(smartplaylist): batch-load genres instead of per-row correlated subquery
Evaluate now issues a lean main SELECT over the joined metadata tables with no genre column, then batch-fetches genres with a single query using WHERE recording_id IN (...). Previously the track_metadata view's correlated GROUP_CONCAT subquery ran per row and scaled with library size rather than result size, producing multi-second load times for 100-track smart playlists. - Inline the metadata joins instead of using the track_metadata view, so the per-row GROUP_CONCAT never runs on the hot path. Other callers of the view (search, library listing) are unaffected. - Route all genre operators (is/is_not/is_any_of/contains/etc.) through a recording_genres subquery against af.recording_id. Previously text operators like "contains" matched against the view's concatenated genre column, which is no longer in scope. - Sort-by-genre falls back to Go-side sort after the batch genre merge since there is no single SQL column to sort on. - Log main_ms / genres_ms / total_ms at Debug for future tuning. - Add (*DB).Logger() accessor so smartplaylist can reuse the DB's structured logger without changing Evaluate's signature. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -148,6 +148,12 @@ func (d *DB) QueryContext(query string, args ...any) (*sql.Rows, error) {
|
|||||||
return d.db.QueryContext(d.Ctx, query, args...)
|
return d.db.QueryContext(d.Ctx, query, args...)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Logger returns the structured logger bound to this DB. Callers can
|
||||||
|
// use it to emit timing or diagnostic logs from query-adjacent code.
|
||||||
|
func (d *DB) Logger() *slog.Logger {
|
||||||
|
return d.logger
|
||||||
|
}
|
||||||
|
|
||||||
// applyPRAGMAs configures SQLite connection settings. Called by both
|
// applyPRAGMAs configures SQLite connection settings. Called by both
|
||||||
// NewDB and NewTestDB to ensure identical behavior.
|
// NewDB and NewTestDB to ensure identical behavior.
|
||||||
func applyPRAGMAs(ctx context.Context, db *sql.DB) error {
|
func applyPRAGMAs(ctx context.Context, db *sql.DB) error {
|
||||||
@@ -1222,7 +1228,7 @@ func migration9SmartPlaylists(
|
|||||||
// migration10PlayHistory adds play history tracking:
|
// migration10PlayHistory adds play history tracking:
|
||||||
// - play_history table for timestamped play log
|
// - play_history table for timestamped play log
|
||||||
// - play_count and last_played columns on audio_files
|
// - play_count and last_played columns on audio_files
|
||||||
// - Recreates track_metadata VIEW to expose the new columns
|
// - Recreates track_metadata VIEW to expose the new columns.
|
||||||
func migration10PlayHistory(
|
func migration10PlayHistory(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
db *sql.DB,
|
db *sql.DB,
|
||||||
|
|||||||
@@ -8,8 +8,10 @@ import (
|
|||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"sort"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
|
"time"
|
||||||
|
|
||||||
"yellowjacket/backend/database"
|
"yellowjacket/backend/database"
|
||||||
"yellowjacket/backend/library"
|
"yellowjacket/backend/library"
|
||||||
@@ -47,38 +49,38 @@ type RuleSet struct {
|
|||||||
// names. Field names MUST come from this map — never interpolated
|
// names. Field names MUST come from this map — never interpolated
|
||||||
// from user input.
|
// from user input.
|
||||||
var fieldMap = map[string]string{
|
var fieldMap = map[string]string{
|
||||||
"title": "title",
|
"title": "title",
|
||||||
"artist": "artist_name",
|
"artist": "artist_name",
|
||||||
"album": "album",
|
"album": "album",
|
||||||
"genre": "genre",
|
"genre": "genre",
|
||||||
"year": "year",
|
"year": "year",
|
||||||
"composer": "composer",
|
"composer": "composer",
|
||||||
"file_type": "file_type",
|
"file_type": "file_type",
|
||||||
"duration": "length_milliseconds",
|
"duration": "length_milliseconds",
|
||||||
"sample_rate": "sample_rate",
|
"sample_rate": "sample_rate",
|
||||||
"bit_depth": "bit_depth",
|
"bit_depth": "bit_depth",
|
||||||
"channels": "channels",
|
"channels": "channels",
|
||||||
"bitrate": "bitrate",
|
"bitrate": "bitrate",
|
||||||
"file_size": "file_size",
|
"file_size": "file_size",
|
||||||
"library": "library_id",
|
"library": "library_id",
|
||||||
"track_number": "track_number",
|
"track_number": "track_number",
|
||||||
"disc_number": "disc_number",
|
"disc_number": "disc_number",
|
||||||
"play_count": "play_count",
|
"play_count": "play_count",
|
||||||
"days_since_played": "days_since_played",
|
"days_since_played": "days_since_played",
|
||||||
}
|
}
|
||||||
|
|
||||||
// numericFields identifies fields that accept numeric operators.
|
// numericFields identifies fields that accept numeric operators.
|
||||||
var numericFields = map[string]bool{
|
var numericFields = map[string]bool{
|
||||||
"year": true,
|
"year": true,
|
||||||
"duration": true,
|
"duration": true,
|
||||||
"sample_rate": true,
|
"sample_rate": true,
|
||||||
"bit_depth": true,
|
"bit_depth": true,
|
||||||
"channels": true,
|
"channels": true,
|
||||||
"bitrate": true,
|
"bitrate": true,
|
||||||
"file_size": true,
|
"file_size": true,
|
||||||
"library": true,
|
"library": true,
|
||||||
"track_number": true,
|
"track_number": true,
|
||||||
"disc_number": true,
|
"disc_number": true,
|
||||||
"play_count": true,
|
"play_count": true,
|
||||||
"days_since_played": true,
|
"days_since_played": true,
|
||||||
}
|
}
|
||||||
@@ -103,16 +105,8 @@ var numericOperators = map[string]bool{
|
|||||||
"between": true,
|
"between": true,
|
||||||
}
|
}
|
||||||
|
|
||||||
// genreExactOps require a subquery against recording_genres JOIN
|
// genreDelimiter matches the GROUP_CONCAT delimiter used when
|
||||||
// genres instead of matching the concatenated genre column.
|
// batch-loading genres for matched tracks.
|
||||||
var genreExactOps = map[string]bool{
|
|
||||||
"is": true,
|
|
||||||
"is_not": true,
|
|
||||||
"is_any_of": true,
|
|
||||||
}
|
|
||||||
|
|
||||||
// genreDelimiter matches the GROUP_CONCAT delimiter in
|
|
||||||
// track_metadata_view.sql.
|
|
||||||
const genreDelimiter = "||"
|
const genreDelimiter = "||"
|
||||||
|
|
||||||
// BuildWhereClause builds a parameterized SQL WHERE clause from a
|
// BuildWhereClause builds a parameterized SQL WHERE clause from a
|
||||||
@@ -143,9 +137,13 @@ func BuildWhereClause(rules []Rule) (string, []any, error) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Genre exact-match operators use a subquery.
|
// All genre operators use a subquery against recording_genres.
|
||||||
if rule.Field == "genre" && genreExactOps[rule.Operator] {
|
// The smart playlist main query does not project a genre
|
||||||
cond, condArgs, err := buildGenreSubquery(rule)
|
// column — genres are batch-loaded after the main query — so
|
||||||
|
// even text operators like "contains" must filter through the
|
||||||
|
// link table rather than a concatenated column.
|
||||||
|
if rule.Field == "genre" {
|
||||||
|
cond, condArgs, err := buildGenreCondition(rule)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", nil, err
|
return "", nil, err
|
||||||
}
|
}
|
||||||
@@ -201,23 +199,43 @@ func validateOperator(op string, isNumeric bool) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// buildGenreSubquery generates a subquery condition against
|
// buildGenreCondition generates a subquery condition against
|
||||||
// recording_genres JOIN genres for exact genre matching.
|
// recording_genres JOIN genres for every supported text operator.
|
||||||
func buildGenreSubquery(rule Rule) (string, []any, error) {
|
// The outer query is expected to expose the `recording_id` column of
|
||||||
subquery := `af.id IN (
|
// the audio file (aliased through the smart playlist query), which is
|
||||||
|
// compared against recording_genres.recording_id.
|
||||||
|
func buildGenreCondition(rule Rule) (string, []any, error) {
|
||||||
|
inHead := `af.recording_id IN (
|
||||||
|
SELECT rg_sub.recording_id FROM recording_genres rg_sub
|
||||||
|
JOIN genres g ON rg_sub.genre_id = g.id
|
||||||
|
WHERE `
|
||||||
|
notInHead := `af.recording_id NOT IN (
|
||||||
SELECT rg_sub.recording_id FROM recording_genres rg_sub
|
SELECT rg_sub.recording_id FROM recording_genres rg_sub
|
||||||
JOIN genres g ON rg_sub.genre_id = g.id
|
JOIN genres g ON rg_sub.genre_id = g.id
|
||||||
WHERE `
|
WHERE `
|
||||||
|
|
||||||
switch rule.Operator {
|
switch rule.Operator {
|
||||||
case "is":
|
case "is":
|
||||||
return subquery + "g.name = ? COLLATE NOCASE)", []any{rule.Value}, nil
|
return inHead + "g.name = ? COLLATE NOCASE)", []any{rule.Value}, nil
|
||||||
|
|
||||||
case "is_not":
|
case "is_not":
|
||||||
return `af.id NOT IN (
|
return notInHead + "g.name = ? COLLATE NOCASE)", []any{rule.Value}, nil
|
||||||
SELECT rg_sub.recording_id FROM recording_genres rg_sub
|
|
||||||
JOIN genres g ON rg_sub.genre_id = g.id
|
case "contains":
|
||||||
WHERE g.name = ? COLLATE NOCASE)`, []any{rule.Value}, nil
|
return inHead + "g.name LIKE ?)",
|
||||||
|
[]any{"%" + rule.Value + "%"}, nil
|
||||||
|
|
||||||
|
case "does_not_contain":
|
||||||
|
return notInHead + "g.name LIKE ?)",
|
||||||
|
[]any{"%" + rule.Value + "%"}, nil
|
||||||
|
|
||||||
|
case "starts_with":
|
||||||
|
return inHead + "g.name LIKE ?)",
|
||||||
|
[]any{rule.Value + "%"}, nil
|
||||||
|
|
||||||
|
case "ends_with":
|
||||||
|
return inHead + "g.name LIKE ?)",
|
||||||
|
[]any{"%" + rule.Value}, nil
|
||||||
|
|
||||||
case "is_any_of":
|
case "is_any_of":
|
||||||
var values []string
|
var values []string
|
||||||
@@ -246,7 +264,7 @@ func buildGenreSubquery(rule Rule) (string, []any, error) {
|
|||||||
condArgs[i] = v
|
condArgs[i] = v
|
||||||
}
|
}
|
||||||
|
|
||||||
return subquery + "g.name IN (" +
|
return inHead + "g.name IN (" +
|
||||||
strings.Join(placeholders, ", ") + "))", condArgs, nil
|
strings.Join(placeholders, ", ") + "))", condArgs, nil
|
||||||
|
|
||||||
default:
|
default:
|
||||||
@@ -304,12 +322,12 @@ func buildDaysSincePlayedCondition(rule Rule) (string, []any, error) {
|
|||||||
return "last_played IS NOT NULL AND " + expr + " < ?", []any{v}, nil
|
return "last_played IS NOT NULL AND " + expr + " < ?", []any{v}, nil
|
||||||
|
|
||||||
case "between":
|
case "between":
|
||||||
min, max, err := parseBetweenValue(rule.Field, rule.Value)
|
lo, hi, err := parseBetweenValue(rule.Field, rule.Value)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", nil, err
|
return "", nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
return expr + " BETWEEN ? AND ?", []any{min, max}, nil
|
return expr + " BETWEEN ? AND ?", []any{lo, hi}, nil
|
||||||
|
|
||||||
default:
|
default:
|
||||||
return "", nil, fmt.Errorf(
|
return "", nil, fmt.Errorf(
|
||||||
@@ -506,11 +524,82 @@ func parseBetweenValue(
|
|||||||
return lo, hi, nil
|
return lo, hi, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Evaluate runs the rule set against the track_metadata view and
|
// leanTrackQuery is the smart-playlist projection: the same columns
|
||||||
// returns matching tracks.
|
// the `track_metadata` view would expose minus the correlated-subquery
|
||||||
|
// `genre` aggregate that made the view expensive to scan. Genres are
|
||||||
|
// batch-loaded after this query returns.
|
||||||
|
//
|
||||||
|
// Wrapping the joins in a subquery aliased `af` lets WHERE/ORDER BY
|
||||||
|
// clauses reference the projected names (`title`, `year`, etc.) the
|
||||||
|
// same way they would against the view. SQLite flattens this subquery
|
||||||
|
// so the runtime cost is equivalent to querying the underlying tables
|
||||||
|
// directly.
|
||||||
|
const leanTrackQuery = `SELECT
|
||||||
|
af.recording_id,
|
||||||
|
af.file_path,
|
||||||
|
af.length_milliseconds,
|
||||||
|
af.title,
|
||||||
|
af.artist_name,
|
||||||
|
af.track_number,
|
||||||
|
af.disc_number,
|
||||||
|
af.album,
|
||||||
|
af.year,
|
||||||
|
af.composer,
|
||||||
|
af.file_type,
|
||||||
|
af.sample_rate,
|
||||||
|
af.bit_depth,
|
||||||
|
af.channels,
|
||||||
|
af.bitrate,
|
||||||
|
af.file_size,
|
||||||
|
af.play_count,
|
||||||
|
COALESCE(af.last_played, '') AS last_played
|
||||||
|
FROM (
|
||||||
|
SELECT
|
||||||
|
af.id,
|
||||||
|
af.recording_id,
|
||||||
|
af.file_path,
|
||||||
|
af.length_milliseconds,
|
||||||
|
COALESCE(r.name, '') AS title,
|
||||||
|
COALESCE(ac.text, '') AS artist_name,
|
||||||
|
r.track_number,
|
||||||
|
r.disc_number,
|
||||||
|
COALESCE(rg.name, '') AS album,
|
||||||
|
COALESCE(r.year, 0) AS year,
|
||||||
|
COALESCE(r.composer, '') AS composer,
|
||||||
|
COALESCE(ft.extension, '') AS file_type,
|
||||||
|
af.sample_rate,
|
||||||
|
af.bit_depth,
|
||||||
|
af.channels,
|
||||||
|
af.bitrate,
|
||||||
|
af.file_size,
|
||||||
|
af.library_id,
|
||||||
|
af.play_count,
|
||||||
|
af.last_played
|
||||||
|
FROM audio_files af
|
||||||
|
LEFT JOIN recordings r ON af.recording_id = r.id
|
||||||
|
LEFT JOIN artist_credit ac ON r.artist_credit_id = ac.id
|
||||||
|
LEFT JOIN (
|
||||||
|
SELECT recording_id,
|
||||||
|
MIN(release_group_id) AS release_group_id
|
||||||
|
FROM release_group_recordings
|
||||||
|
GROUP BY recording_id
|
||||||
|
) rgr ON r.id = rgr.recording_id
|
||||||
|
LEFT JOIN release_groups rg ON rgr.release_group_id = rg.id
|
||||||
|
LEFT JOIN file_types ft ON af.file_type_id = ft.id
|
||||||
|
) af`
|
||||||
|
|
||||||
|
// Evaluate runs the rule set against the library and returns matching
|
||||||
|
// tracks. It issues two queries: one lean SELECT over the joined
|
||||||
|
// metadata tables (no genre) and a batched follow-up to attach genres
|
||||||
|
// to the matched tracks. This avoids the per-row correlated genre
|
||||||
|
// subquery in `track_metadata_view`, which scaled with library size
|
||||||
|
// rather than result size.
|
||||||
func Evaluate(
|
func Evaluate(
|
||||||
db *database.DB, ruleSet RuleSet,
|
db *database.DB, ruleSet RuleSet,
|
||||||
) ([]library.Track, error) {
|
) ([]library.Track, error) {
|
||||||
|
start := time.Now()
|
||||||
|
logger := db.Logger()
|
||||||
|
|
||||||
where, args, err := BuildWhereClause(ruleSet.Rules)
|
where, args, err := BuildWhereClause(ruleSet.Rules)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf(
|
return nil, fmt.Errorf(
|
||||||
@@ -518,63 +607,58 @@ func Evaluate(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Sort-by-genre has no single SQL column to sort on (genres are
|
||||||
|
// many-to-one per track). Detect it up front so we can sort in
|
||||||
|
// Go after genres are merged.
|
||||||
|
sortByGenre := ruleSet.SortField == "genre"
|
||||||
|
|
||||||
// SAFETY: Dynamic WHERE clause built from whitelisted field
|
// SAFETY: Dynamic WHERE clause built from whitelisted field
|
||||||
// names and parameterized values only. Sort field is validated
|
// names and parameterized values only. Sort field is validated
|
||||||
// against fieldMap. No user-supplied strings are interpolated.
|
// against fieldMap. No user-supplied strings are interpolated.
|
||||||
query := `SELECT
|
query := leanTrackQuery
|
||||||
file_path,
|
|
||||||
length_milliseconds,
|
|
||||||
title,
|
|
||||||
artist_name,
|
|
||||||
track_number,
|
|
||||||
disc_number,
|
|
||||||
album,
|
|
||||||
genre,
|
|
||||||
year,
|
|
||||||
composer,
|
|
||||||
file_type,
|
|
||||||
sample_rate,
|
|
||||||
bit_depth,
|
|
||||||
channels,
|
|
||||||
bitrate,
|
|
||||||
file_size,
|
|
||||||
play_count,
|
|
||||||
COALESCE(last_played, '') AS last_played
|
|
||||||
FROM track_metadata af`
|
|
||||||
|
|
||||||
if where != "" {
|
if where != "" {
|
||||||
query += "\nWHERE " + where
|
query += "\nWHERE " + where
|
||||||
}
|
}
|
||||||
|
|
||||||
// Sort.
|
// Sort.
|
||||||
if ruleSet.SortField != "" {
|
applyLimitInSQL := ruleSet.Limit > 0
|
||||||
if ruleSet.SortField == "random" {
|
|
||||||
query += "\nORDER BY RANDOM()"
|
|
||||||
} else {
|
|
||||||
sortCol, ok := fieldMap[ruleSet.SortField]
|
|
||||||
if !ok {
|
|
||||||
return nil, fmt.Errorf(
|
|
||||||
"%w: %q", errInvalidSortField,
|
|
||||||
ruleSet.SortField,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
dir := "ASC"
|
switch {
|
||||||
if strings.EqualFold(ruleSet.SortDir, "DESC") {
|
case sortByGenre:
|
||||||
dir = "DESC"
|
// Sort applied in Go after the batch genre merge. If a LIMIT
|
||||||
}
|
// was requested we also defer it so the ordering is computed
|
||||||
|
// over the full candidate set.
|
||||||
|
applyLimitInSQL = false
|
||||||
|
|
||||||
query += "\nORDER BY " + sortCol + " " + dir
|
case ruleSet.SortField == "random":
|
||||||
|
query += "\nORDER BY RANDOM()"
|
||||||
|
|
||||||
|
case ruleSet.SortField != "":
|
||||||
|
sortCol, ok := fieldMap[ruleSet.SortField]
|
||||||
|
if !ok {
|
||||||
|
return nil, fmt.Errorf(
|
||||||
|
"%w: %q", errInvalidSortField,
|
||||||
|
ruleSet.SortField,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
dir := "ASC"
|
||||||
|
if strings.EqualFold(ruleSet.SortDir, "DESC") {
|
||||||
|
dir = "DESC"
|
||||||
|
}
|
||||||
|
|
||||||
|
query += "\nORDER BY " + sortCol + " " + dir
|
||||||
}
|
}
|
||||||
|
|
||||||
// Limit.
|
if applyLimitInSQL {
|
||||||
if ruleSet.Limit > 0 {
|
|
||||||
query += "\nLIMIT ?"
|
query += "\nLIMIT ?"
|
||||||
|
|
||||||
args = append(args, ruleSet.Limit)
|
args = append(args, ruleSet.Limit)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
mainStart := time.Now()
|
||||||
|
|
||||||
rows, err := db.QueryContext(query, args...)
|
rows, err := db.QueryContext(query, args...)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf(
|
return nil, fmt.Errorf(
|
||||||
@@ -582,17 +666,75 @@ func Evaluate(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
defer func() { _ = rows.Close() }()
|
tracks, recordingIDs, err := scanTracks(rows)
|
||||||
|
|
||||||
return scanTracks(rows)
|
_ = rows.Close()
|
||||||
|
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
mainDuration := time.Since(mainStart)
|
||||||
|
|
||||||
|
// Batch-load genres for every matched recording_id in one query
|
||||||
|
// instead of the per-row correlated subquery the view used.
|
||||||
|
genreStart := time.Now()
|
||||||
|
|
||||||
|
genresByRecording, err := fetchGenres(db, recordingIDs)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
for i, rid := range recordingIDs {
|
||||||
|
if g, ok := genresByRecording[rid]; ok {
|
||||||
|
tracks[i].Genre = splitGenres(g)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
genreDuration := time.Since(genreStart)
|
||||||
|
|
||||||
|
// Apply genre-sort and deferred LIMIT in Go if needed.
|
||||||
|
if sortByGenre {
|
||||||
|
dir := 1
|
||||||
|
if strings.EqualFold(ruleSet.SortDir, "DESC") {
|
||||||
|
dir = -1
|
||||||
|
}
|
||||||
|
|
||||||
|
sort.SliceStable(tracks, func(i, j int) bool {
|
||||||
|
return dir*strings.Compare(
|
||||||
|
strings.Join(tracks[i].Genre, genreDelimiter),
|
||||||
|
strings.Join(tracks[j].Genre, genreDelimiter),
|
||||||
|
) < 0
|
||||||
|
})
|
||||||
|
|
||||||
|
if ruleSet.Limit > 0 && len(tracks) > ruleSet.Limit {
|
||||||
|
tracks = tracks[:ruleSet.Limit]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
logger.Debug(
|
||||||
|
"smart playlist evaluated",
|
||||||
|
"tracks", len(tracks),
|
||||||
|
"main_ms", mainDuration.Milliseconds(),
|
||||||
|
"genres_ms", genreDuration.Milliseconds(),
|
||||||
|
"total_ms", time.Since(start).Milliseconds(),
|
||||||
|
)
|
||||||
|
|
||||||
|
return tracks, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// scanTracks reads all rows from a query result into a Track slice.
|
// scanTracks reads all rows from a lean-query result into parallel
|
||||||
func scanTracks(rows *sql.Rows) ([]library.Track, error) {
|
// slices: the Track values (minus genres, which are attached later)
|
||||||
var tracks []library.Track
|
// and the recording_id for each, used for the batched genre fetch.
|
||||||
|
func scanTracks(rows *sql.Rows) ([]library.Track, []int64, error) {
|
||||||
|
var (
|
||||||
|
tracks []library.Track
|
||||||
|
recordingIDs []int64
|
||||||
|
)
|
||||||
|
|
||||||
for rows.Next() {
|
for rows.Next() {
|
||||||
var (
|
var (
|
||||||
|
recordingID sql.NullInt64
|
||||||
filePath string
|
filePath string
|
||||||
lengthMs int64
|
lengthMs int64
|
||||||
title string
|
title string
|
||||||
@@ -600,7 +742,6 @@ func scanTracks(rows *sql.Rows) ([]library.Track, error) {
|
|||||||
trackNumber sql.NullInt64
|
trackNumber sql.NullInt64
|
||||||
discNumber sql.NullInt64
|
discNumber sql.NullInt64
|
||||||
album string
|
album string
|
||||||
genre string
|
|
||||||
year int64
|
year int64
|
||||||
composer string
|
composer string
|
||||||
fileType string
|
fileType string
|
||||||
@@ -614,14 +755,14 @@ func scanTracks(rows *sql.Rows) ([]library.Track, error) {
|
|||||||
)
|
)
|
||||||
|
|
||||||
if err := rows.Scan(
|
if err := rows.Scan(
|
||||||
&filePath, &lengthMs, &title, &artistName,
|
&recordingID, &filePath, &lengthMs, &title, &artistName,
|
||||||
&trackNumber, &discNumber,
|
&trackNumber, &discNumber,
|
||||||
&album, &genre, &year, &composer, &fileType,
|
&album, &year, &composer, &fileType,
|
||||||
&sampleRate, &bitDepth, &channels,
|
&sampleRate, &bitDepth, &channels,
|
||||||
&bitrate, &fileSize,
|
&bitrate, &fileSize,
|
||||||
&playCount, &lastPlayed,
|
&playCount, &lastPlayed,
|
||||||
); err != nil {
|
); err != nil {
|
||||||
return nil, fmt.Errorf(
|
return nil, nil, fmt.Errorf(
|
||||||
"could not scan smart playlist row: %w", err,
|
"could not scan smart playlist row: %w", err,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -634,7 +775,6 @@ func scanTracks(rows *sql.Rows) ([]library.Track, error) {
|
|||||||
TrackNumber: trackNumber.Int64,
|
TrackNumber: trackNumber.Int64,
|
||||||
DiscNumber: discNumber.Int64,
|
DiscNumber: discNumber.Int64,
|
||||||
Album: album,
|
Album: album,
|
||||||
Genre: splitGenres(genre),
|
|
||||||
Year: year,
|
Year: year,
|
||||||
Composer: composer,
|
Composer: composer,
|
||||||
FileType: fileType,
|
FileType: fileType,
|
||||||
@@ -646,15 +786,101 @@ func scanTracks(rows *sql.Rows) ([]library.Track, error) {
|
|||||||
PlayCount: playCount,
|
PlayCount: playCount,
|
||||||
LastPlayed: lastPlayed,
|
LastPlayed: lastPlayed,
|
||||||
})
|
})
|
||||||
|
recordingIDs = append(recordingIDs, recordingID.Int64)
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := rows.Err(); err != nil {
|
if err := rows.Err(); err != nil {
|
||||||
return nil, fmt.Errorf(
|
return nil, nil, fmt.Errorf(
|
||||||
"smart playlist row iteration error: %w", err,
|
"smart playlist row iteration error: %w", err,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
return tracks, nil
|
return tracks, recordingIDs, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// fetchGenres batch-loads the GROUP_CONCAT-joined genre string for
|
||||||
|
// every recording_id in ids using a single IN-list query. Returns a
|
||||||
|
// map from recording_id to the concatenated genre string.
|
||||||
|
func fetchGenres(
|
||||||
|
db *database.DB, ids []int64,
|
||||||
|
) (map[int64]string, error) {
|
||||||
|
if len(ids) == 0 {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Deduplicate to keep the IN list minimal.
|
||||||
|
seen := make(map[int64]struct{}, len(ids))
|
||||||
|
unique := make([]int64, 0, len(ids))
|
||||||
|
|
||||||
|
for _, id := range ids {
|
||||||
|
if id == 0 {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, ok := seen[id]; ok {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
seen[id] = struct{}{}
|
||||||
|
|
||||||
|
unique = append(unique, id)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(unique) == 0 {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
placeholders := make([]string, len(unique))
|
||||||
|
args := make([]any, len(unique))
|
||||||
|
|
||||||
|
for i, id := range unique {
|
||||||
|
placeholders[i] = "?"
|
||||||
|
args[i] = id
|
||||||
|
}
|
||||||
|
|
||||||
|
// SAFETY: placeholders are static "?" tokens; every value is
|
||||||
|
// parameterized.
|
||||||
|
query := `SELECT rg_sub.recording_id,
|
||||||
|
GROUP_CONCAT(g.name, '` + genreDelimiter + `')
|
||||||
|
FROM recording_genres rg_sub
|
||||||
|
JOIN genres g ON rg_sub.genre_id = g.id
|
||||||
|
WHERE rg_sub.recording_id IN (` +
|
||||||
|
strings.Join(placeholders, ", ") + `)
|
||||||
|
GROUP BY rg_sub.recording_id`
|
||||||
|
|
||||||
|
rows, err := db.QueryContext(query, args...)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf(
|
||||||
|
"smart playlist genre fetch failed: %w", err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
defer func() { _ = rows.Close() }()
|
||||||
|
|
||||||
|
result := make(map[int64]string, len(unique))
|
||||||
|
|
||||||
|
for rows.Next() {
|
||||||
|
var (
|
||||||
|
rid int64
|
||||||
|
names string
|
||||||
|
)
|
||||||
|
|
||||||
|
if err := rows.Scan(&rid, &names); err != nil {
|
||||||
|
return nil, fmt.Errorf(
|
||||||
|
"could not scan smart playlist genre row: %w", err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
result[rid] = names
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
|
return nil, fmt.Errorf(
|
||||||
|
"smart playlist genre iteration error: %w", err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return result, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// ParseRuleSet parses a JSON string into a validated RuleSet.
|
// ParseRuleSet parses a JSON string into a validated RuleSet.
|
||||||
|
|||||||
@@ -608,7 +608,7 @@ func TestBuildWhereClause_GenreIsAnyOfProducesSubquery(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestBuildWhereClause_GenreContainsUsesLIKE(t *testing.T) {
|
func TestBuildWhereClause_GenreContainsUsesSubquery(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
@@ -618,17 +618,21 @@ func TestBuildWhereClause_GenreContainsUsesLIKE(t *testing.T) {
|
|||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// contains on genre should use LIKE on the concatenated column,
|
// Since the smart playlist query no longer projects a concatenated
|
||||||
// NOT a subquery.
|
// genre column, "contains" filters genres via recording_genres
|
||||||
if strings.Contains(clause, "recording_genres") {
|
// with g.name LIKE applied to individual genre rows.
|
||||||
|
if !strings.Contains(clause, "recording_genres") {
|
||||||
t.Errorf(
|
t.Errorf(
|
||||||
"genre 'contains' should use LIKE, not subquery: %q",
|
"genre 'contains' should use recording_genres subquery: %q",
|
||||||
clause,
|
clause,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
if clause != "genre LIKE ?" {
|
if !strings.Contains(clause, "g.name LIKE ?") {
|
||||||
t.Errorf("clause = %q, want %q", clause, "genre LIKE ?")
|
t.Errorf(
|
||||||
|
"genre 'contains' should filter with g.name LIKE ?: %q",
|
||||||
|
clause,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(args) != 1 || args[0] != "%Rock%" {
|
if len(args) != 1 || args[0] != "%Rock%" {
|
||||||
@@ -671,10 +675,18 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
|
|||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if clause != "genre LIKE ? AND genre NOT LIKE ?" {
|
// Genre text ops combine via AND across subqueries against
|
||||||
t.Errorf("clause = %q, want %q",
|
// recording_genres; the exact SQL shape is asserted elsewhere.
|
||||||
clause,
|
if !strings.Contains(clause, " AND ") {
|
||||||
"genre LIKE ? AND genre NOT LIKE ?")
|
t.Errorf("clause should combine rules with AND: %q", clause)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !strings.Contains(clause, "af.recording_id IN") {
|
||||||
|
t.Errorf("clause should include positive IN subquery: %q", clause)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !strings.Contains(clause, "af.recording_id NOT IN") {
|
||||||
|
t.Errorf("clause should include NOT IN subquery: %q", clause)
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(args) != 2 ||
|
if len(args) != 2 ||
|
||||||
|
|||||||
Reference in New Issue
Block a user