mirror of
https://github.com/navidrome/navidrome.git
synced 2026-07-09 17:18:45 +00:00
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
Some checks are pending
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (Windows) (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions
Some checks are pending
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (Windows) (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
This commit is contained in:
parent
56f0518830
commit
63a5954e4f
4 changed files with 292 additions and 74 deletions
|
|
@ -96,20 +96,32 @@ func normalizeBoolFields(m map[string]any) {
|
|||
}
|
||||
}
|
||||
|
||||
func normalizeBoolValue(v any) any {
|
||||
// ToBool coerces a criteria value to a bool, accepting the forms criteria values take: a real bool,
|
||||
// a strconv.ParseBool-parseable string, or a JSON number that is exactly 0 or 1. Any other value
|
||||
// (other numbers, slices, nil, unparseable strings) returns ok=false so callers can handle it.
|
||||
func ToBool(v any) (bool, bool) {
|
||||
switch val := v.(type) {
|
||||
case bool:
|
||||
return val, true
|
||||
case string:
|
||||
if b, err := strconv.ParseBool(val); err == nil {
|
||||
return b
|
||||
}
|
||||
b, err := strconv.ParseBool(val)
|
||||
return b, err == nil
|
||||
case float64:
|
||||
if val == 1 {
|
||||
return true
|
||||
}
|
||||
if val == 0 {
|
||||
return false
|
||||
switch val {
|
||||
case 1:
|
||||
return true, true
|
||||
case 0:
|
||||
return false, true
|
||||
}
|
||||
}
|
||||
return false, false
|
||||
}
|
||||
|
||||
// normalizeBoolValue leaves non-boolean values unchanged so they flow through to their own validation.
|
||||
func normalizeBoolValue(v any) any {
|
||||
if b, ok := ToBool(v); ok {
|
||||
return b
|
||||
}
|
||||
return v
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -126,4 +126,25 @@ var _ = Describe("Operators", func() {
|
|||
gomega.Expect(obj[0]).To(gomega.Equal(IsPresent{"genre": true}))
|
||||
})
|
||||
})
|
||||
|
||||
DescribeTable("ToBool",
|
||||
func(in any, wantVal, wantOk bool) {
|
||||
got, ok := ToBool(in)
|
||||
gomega.Expect(ok).To(gomega.Equal(wantOk))
|
||||
gomega.Expect(got).To(gomega.Equal(wantVal))
|
||||
},
|
||||
Entry("real bool true", true, true, true),
|
||||
Entry("real bool false", false, false, true),
|
||||
Entry("string true", "true", true, true),
|
||||
Entry("string false", "false", false, true),
|
||||
Entry("string 1", "1", true, true),
|
||||
Entry("string t", "t", true, true),
|
||||
Entry("string 0", "0", false, true),
|
||||
Entry("string unparseable", "yes", false, false),
|
||||
Entry("float64 1", float64(1), true, true),
|
||||
Entry("float64 0", float64(0), false, true),
|
||||
Entry("float64 other", float64(2), false, false),
|
||||
Entry("slice", []any{true}, false, false),
|
||||
Entry("nil", nil, false, false),
|
||||
)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -28,10 +28,11 @@ func (j smartPlaylistJoinType) has(other smartPlaylistJoinType) bool {
|
|||
}
|
||||
|
||||
type smartPlaylistField struct {
|
||||
expr string
|
||||
order string
|
||||
joinType smartPlaylistJoinType
|
||||
emptyValues []string // additional values that encode "missing" for string columns (e.g. '[]' for lyrics)
|
||||
expr string
|
||||
order string
|
||||
joinType smartPlaylistJoinType
|
||||
emptyValues []string // additional values that encode "missing" for string columns (e.g. '[]' for lyrics)
|
||||
coalesceDefault any // missing-row default for a nullable annotation column; nil = none. See annotationCond.
|
||||
}
|
||||
|
||||
type smartPlaylistCriteria struct {
|
||||
|
|
@ -89,22 +90,22 @@ var smartPlaylistFields = map[string]smartPlaylistField{
|
|||
"samplerate": {expr: "media_file.sample_rate"},
|
||||
"bpm": {expr: "media_file.bpm"},
|
||||
"channels": {expr: "media_file.channels"},
|
||||
"loved": {expr: "COALESCE(annotation.starred, false)"},
|
||||
"loved": {expr: "annotation.starred", coalesceDefault: false},
|
||||
"dateloved": {expr: "annotation.starred_at"},
|
||||
"lastplayed": {expr: "annotation.play_date"},
|
||||
"daterated": {expr: "annotation.rated_at"},
|
||||
"playcount": {expr: "COALESCE(annotation.play_count, 0)"},
|
||||
"rating": {expr: "COALESCE(annotation.rating, 0)"},
|
||||
"playcount": {expr: "annotation.play_count", coalesceDefault: 0},
|
||||
"rating": {expr: "annotation.rating", coalesceDefault: 0},
|
||||
"averagerating": {expr: "media_file.average_rating"},
|
||||
"albumrating": {expr: "COALESCE(album_annotation.rating, 0)", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumloved": {expr: "COALESCE(album_annotation.starred, false)", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumplaycount": {expr: "COALESCE(album_annotation.play_count, 0)", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumrating": {expr: "album_annotation.rating", coalesceDefault: 0, joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumloved": {expr: "album_annotation.starred", coalesceDefault: false, joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumplaycount": {expr: "album_annotation.play_count", coalesceDefault: 0, joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumlastplayed": {expr: "album_annotation.play_date", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumdateloved": {expr: "album_annotation.starred_at", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"albumdaterated": {expr: "album_annotation.rated_at", joinType: smartPlaylistJoinAlbumAnnotation},
|
||||
"artistrating": {expr: "COALESCE(artist_annotation.rating, 0)", joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistloved": {expr: "COALESCE(artist_annotation.starred, false)", joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistplaycount": {expr: "COALESCE(artist_annotation.play_count, 0)", joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistrating": {expr: "artist_annotation.rating", coalesceDefault: 0, joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistloved": {expr: "artist_annotation.starred", coalesceDefault: false, joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistplaycount": {expr: "artist_annotation.play_count", coalesceDefault: 0, joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistlastplayed": {expr: "artist_annotation.play_date", joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistdateloved": {expr: "artist_annotation.starred_at", joinType: smartPlaylistJoinArtistAnnotation},
|
||||
"artistdaterated": {expr: "artist_annotation.rated_at", joinType: smartPlaylistJoinArtistAnnotation},
|
||||
|
|
@ -152,27 +153,17 @@ func (c smartPlaylistCriteria) exprSQL(expr criteria.Expression) (squirrel.Sqliz
|
|||
}
|
||||
return mergeJsonConds(or), nil
|
||||
case criteria.Is:
|
||||
return mapExpr(e, func(fields map[string]any) squirrel.Sqlizer {
|
||||
return squirrel.Eq(fields)
|
||||
}, false)
|
||||
return comparisonExpr(e, cmpEq)
|
||||
case criteria.IsNot:
|
||||
return isNotExpr(e)
|
||||
case criteria.Gt:
|
||||
return mapExpr(e, func(fields map[string]any) squirrel.Sqlizer {
|
||||
return squirrel.Gt(fields)
|
||||
}, false)
|
||||
return comparisonExpr(e, cmpGt)
|
||||
case criteria.Lt:
|
||||
return mapExpr(e, func(fields map[string]any) squirrel.Sqlizer {
|
||||
return squirrel.Lt(fields)
|
||||
}, false)
|
||||
return comparisonExpr(e, cmpLt)
|
||||
case criteria.Before:
|
||||
return mapExpr(e, func(fields map[string]any) squirrel.Sqlizer {
|
||||
return squirrel.Lt(fields)
|
||||
}, false)
|
||||
return comparisonExpr(e, cmpLt)
|
||||
case criteria.After:
|
||||
return mapExpr(e, func(fields map[string]any) squirrel.Sqlizer {
|
||||
return squirrel.Gt(fields)
|
||||
}, false)
|
||||
return comparisonExpr(e, cmpGt)
|
||||
case criteria.Contains:
|
||||
return likeExpr(e, "%%%v%%", false)
|
||||
case criteria.NotContains:
|
||||
|
|
@ -204,11 +195,7 @@ func isNotExpr(values map[string]any) (squirrel.Sqlizer, error) {
|
|||
if _, value, info, ok := singleField(values); ok && (info.IsTag || info.IsRole) {
|
||||
return jsonExpr(info, squirrel.Eq{"value": value}, true), nil
|
||||
}
|
||||
fields, err := sqlFields(values)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return squirrel.NotEq(fields), nil
|
||||
return comparisonExpr(values, cmpNe)
|
||||
}
|
||||
|
||||
func missingExpr(values map[string]any, checkAbsence bool) (squirrel.Sqlizer, error) {
|
||||
|
|
@ -259,20 +246,17 @@ func missingExpr(values map[string]any, checkAbsence bool) (squirrel.Sqlizer, er
|
|||
}
|
||||
}
|
||||
|
||||
func mapExpr(values map[string]any, makeCond func(map[string]any) squirrel.Sqlizer, negateJSON bool) (squirrel.Sqlizer, error) {
|
||||
if _, value, info, ok := singleField(values); ok && (info.IsTag || info.IsRole) {
|
||||
return jsonExpr(info, makeCond(map[string]any{"value": value}), negateJSON), nil
|
||||
}
|
||||
fields, err := sqlFields(values)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return makeCond(fields), nil
|
||||
}
|
||||
|
||||
func likeExpr(values map[string]any, pattern string, negate bool) (squirrel.Sqlizer, error) {
|
||||
if _, value, info, ok := singleField(values); ok && (info.IsTag || info.IsRole) {
|
||||
return jsonExpr(info, squirrel.Like{"value": fmt.Sprintf(pattern, value)}, negate), nil
|
||||
if _, value, info, ok := singleField(values); ok {
|
||||
if info.IsTag || info.IsRole {
|
||||
return jsonExpr(info, squirrel.Like{"value": fmt.Sprintf(pattern, value)}, negate), nil
|
||||
}
|
||||
// LIKE can't use the column index, so annotation fields keep the COALESCE form: a NULL
|
||||
// column never matches LIKE, which would silently drop missing-annotation rows the original
|
||||
// COALESCE form included.
|
||||
if f, isAnnotation := annotationField(info); isAnnotation {
|
||||
return likeCond(f.coalesced(), fmt.Sprintf(pattern, value), negate), nil
|
||||
}
|
||||
}
|
||||
fields, err := sqlFields(values)
|
||||
if err != nil {
|
||||
|
|
@ -292,23 +276,36 @@ func likeExpr(values map[string]any, pattern string, negate bool) (squirrel.Sqli
|
|||
return lk, nil
|
||||
}
|
||||
|
||||
func likeCond(col, pattern string, negate bool) squirrel.Sqlizer {
|
||||
if negate {
|
||||
return squirrel.NotLike{col: pattern}
|
||||
}
|
||||
return squirrel.Like{col: pattern}
|
||||
}
|
||||
|
||||
func rangeExpr(values map[string]any) (squirrel.Sqlizer, error) {
|
||||
fields, err := sqlFields(values)
|
||||
field, value, info, ok := singleField(values)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("invalid field in criteria: %s", field)
|
||||
}
|
||||
if info.IsTag || info.IsRole {
|
||||
// Tags/roles are multi-valued JSON, so splitting a range into two independent EXISTS
|
||||
// subqueries would let different values satisfy each bound. Ranges are unsupported there.
|
||||
return nil, fmt.Errorf("range operator not supported for tag/role field: %s", field)
|
||||
}
|
||||
s := reflect.ValueOf(value)
|
||||
if s.Kind() != reflect.Slice || s.Len() != 2 {
|
||||
return nil, fmt.Errorf("range criteria for %q must be a [min, max] pair, got: %v", field, value)
|
||||
}
|
||||
low, err := comparisonExpr(map[string]any{field: s.Index(0).Interface()}, cmpGe)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
and := squirrel.And{}
|
||||
for field, value := range fields {
|
||||
s := reflect.ValueOf(value)
|
||||
if s.Kind() != reflect.Slice || s.Len() != 2 {
|
||||
return nil, fmt.Errorf("invalid range for 'in' operator: %s", value)
|
||||
}
|
||||
and = append(and,
|
||||
squirrel.GtOrEq{field: s.Index(0).Interface()},
|
||||
squirrel.LtOrEq{field: s.Index(1).Interface()},
|
||||
)
|
||||
high, err := comparisonExpr(map[string]any{field: s.Index(1).Interface()}, cmpLe)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return and, nil
|
||||
return squirrel.And{low, high}, nil
|
||||
}
|
||||
|
||||
func periodExpr(values map[string]any, negate bool) (squirrel.Sqlizer, error) {
|
||||
|
|
@ -638,6 +635,139 @@ func fieldExpr(name string) (string, bool) {
|
|||
return field.expr, ok
|
||||
}
|
||||
|
||||
// comparator is a scalar SQL comparison operator used by smart playlist criteria.
|
||||
type comparator struct {
|
||||
build func(map[string]any) squirrel.Sqlizer
|
||||
satisfy func(a, b float64) bool // the operator as a predicate, to reason about a column's COALESCE default
|
||||
// ordering is false only for = and <>, the only operators with a clean bare form over bool columns.
|
||||
ordering bool
|
||||
}
|
||||
|
||||
var (
|
||||
cmpEq = comparator{build: func(f map[string]any) squirrel.Sqlizer { return squirrel.Eq(f) }, satisfy: func(a, b float64) bool { return a == b }}
|
||||
cmpNe = comparator{build: func(f map[string]any) squirrel.Sqlizer { return squirrel.NotEq(f) }, satisfy: func(a, b float64) bool { return a != b }}
|
||||
cmpGt = comparator{build: func(f map[string]any) squirrel.Sqlizer { return squirrel.Gt(f) }, satisfy: func(a, b float64) bool { return a > b }, ordering: true}
|
||||
cmpGe = comparator{build: func(f map[string]any) squirrel.Sqlizer { return squirrel.GtOrEq(f) }, satisfy: func(a, b float64) bool { return a >= b }, ordering: true}
|
||||
cmpLt = comparator{build: func(f map[string]any) squirrel.Sqlizer { return squirrel.Lt(f) }, satisfy: func(a, b float64) bool { return a < b }, ordering: true}
|
||||
cmpLe = comparator{build: func(f map[string]any) squirrel.Sqlizer { return squirrel.LtOrEq(f) }, satisfy: func(a, b float64) bool { return a <= b }, ordering: true}
|
||||
)
|
||||
|
||||
// annotationField returns the field definition only for nullable annotation columns that have a
|
||||
// COALESCE default (playcount, rating, loved). Date annotation columns (lastplayed, dateloved, ...)
|
||||
// have no default and return ok=false.
|
||||
func annotationField(info criteria.FieldInfo) (smartPlaylistField, bool) {
|
||||
f, ok := smartPlaylistFields[info.Name()]
|
||||
if !ok || f.coalesceDefault == nil {
|
||||
return smartPlaylistField{}, false
|
||||
}
|
||||
return f, true
|
||||
}
|
||||
|
||||
// coalesced wraps the field in COALESCE(col, default) (bare expression if it has no default). Used
|
||||
// where index-friendliness does not apply (ORDER BY, list comparisons) and the missing-row-as-default
|
||||
// semantics must be kept.
|
||||
func (f smartPlaylistField) coalesced() string {
|
||||
if f.coalesceDefault == nil {
|
||||
return f.expr
|
||||
}
|
||||
return fmt.Sprintf("COALESCE(%s, %s)", f.expr, sqlLiteral(f.coalesceDefault))
|
||||
}
|
||||
|
||||
func comparisonExpr(values map[string]any, cmp comparator) (squirrel.Sqlizer, error) {
|
||||
if _, value, info, ok := singleField(values); ok {
|
||||
if info.IsTag || info.IsRole {
|
||||
return jsonExpr(info, cmp.build(map[string]any{"value": value}), false), nil
|
||||
}
|
||||
if f, isAnnotation := annotationField(info); isAnnotation {
|
||||
return annotationCond(f, cmp, value), nil
|
||||
}
|
||||
}
|
||||
fields, err := sqlFields(values)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return cmp.build(fields), nil
|
||||
}
|
||||
|
||||
// annotationCond builds a comparison against a nullable annotation column (see smartPlaylistField
|
||||
// for why). When bareNullInclusion can reason about the comparison exactly it emits the
|
||||
// index-friendly bare `col <cmp> ?`, adding `OR col IS NULL` only when the default would match;
|
||||
// otherwise it falls back to the COALESCE form, which can't use the index but is always equivalent.
|
||||
func annotationCond(f smartPlaylistField, cmp comparator, value any) squirrel.Sqlizer {
|
||||
wrapNull, ok := bareNullInclusion(f.coalesceDefault, cmp, value)
|
||||
if !ok {
|
||||
return cmp.build(map[string]any{f.coalesced(): value})
|
||||
}
|
||||
base := cmp.build(map[string]any{f.expr: value})
|
||||
if !wrapNull {
|
||||
return base
|
||||
}
|
||||
// The default satisfies the predicate, so missing rows (NULL) must be included. All comparators
|
||||
// (including <>) evaluate to false against NULL, so an explicit IS NULL restores them.
|
||||
return squirrel.Or{base, squirrel.Eq{f.expr: nil}}
|
||||
}
|
||||
|
||||
// bareNullInclusion decides whether the index-friendly bare form is safe for this comparison and,
|
||||
// if so, whether missing (NULL) rows must be re-included via `OR col IS NULL`. It returns ok=false
|
||||
// when the bare form can't be proven equivalent to COALESCE(col, default) <cmp> ? — i.e. the value
|
||||
// isn't a scalar the comparator can order exactly (lists, bool ordering, unparseable values) — in
|
||||
// which case the caller keeps the COALESCE form. When ok=true, wrapNull is true iff the default
|
||||
// value itself satisfies the predicate (so missing rows would match and must be preserved).
|
||||
func bareNullInclusion(defaultVal any, cmp comparator, value any) (wrapNull, ok bool) {
|
||||
if b, isBool := defaultVal.(bool); isBool {
|
||||
// Bool columns have an exact bare form only for equality; ordering operators fall back to
|
||||
// COALESCE. Mapping both bools to 0/1 lets cmp.satisfy reuse the numeric eq/ne predicate.
|
||||
if cmp.ordering {
|
||||
return false, false
|
||||
}
|
||||
v, okV := criteria.ToBool(value)
|
||||
if !okV {
|
||||
return false, false
|
||||
}
|
||||
return cmp.satisfy(boolToFloat(b), boolToFloat(v)), true
|
||||
}
|
||||
d, okD := toFloat(defaultVal)
|
||||
v, okV := toFloat(value)
|
||||
if !okD || !okV {
|
||||
// Non-scalar or unparseable value: no exact bare form, keep COALESCE.
|
||||
return false, false
|
||||
}
|
||||
return cmp.satisfy(d, v), true
|
||||
}
|
||||
|
||||
func boolToFloat(b bool) float64 {
|
||||
if b {
|
||||
return 1
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
// toFloat coerces a scalar criteria value to float64. Criteria values come from JSON (float64,
|
||||
// string) or are built in Go (int, float64), so only those types are handled; anything else —
|
||||
// including a slice or an unparseable string — reports ok=false so the caller keeps the COALESCE
|
||||
// form instead of an index-friendly bare comparison.
|
||||
func toFloat(v any) (float64, bool) {
|
||||
switch n := v.(type) {
|
||||
case float64:
|
||||
return n, true
|
||||
case int:
|
||||
return float64(n), true
|
||||
case int64:
|
||||
return float64(n), true
|
||||
case string:
|
||||
f, err := strconv.ParseFloat(n, 64)
|
||||
return f, err == nil
|
||||
default:
|
||||
return 0, false
|
||||
}
|
||||
}
|
||||
|
||||
// sqlLiteral renders an annotation field's COALESCE default (0 or false) as a SQL literal for ORDER
|
||||
// BY. %v renders both bool and numeric defaults correctly (false/true, 0).
|
||||
func sqlLiteral(v any) string {
|
||||
return fmt.Sprintf("%v", v)
|
||||
}
|
||||
|
||||
func fieldJoinType(name string) smartPlaylistJoinType {
|
||||
info, ok := criteria.LookupField(name)
|
||||
if !ok {
|
||||
|
|
@ -705,7 +835,9 @@ func sortExpr(sortField string) (string, bool) {
|
|||
if !ok || field.expr == "" {
|
||||
return "", false
|
||||
}
|
||||
mapped = field.expr
|
||||
// Sorting keeps the COALESCE default so missing-annotation rows sort as that default
|
||||
// (filtering drops COALESCE for index use, but ORDER BY has no index to preserve here).
|
||||
mapped = field.coalesced()
|
||||
}
|
||||
if info.Numeric {
|
||||
mapped = fmt.Sprintf("CAST(%s AS REAL)", mapped)
|
||||
|
|
|
|||
|
|
@ -30,16 +30,16 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
|||
},
|
||||
Entry("all group",
|
||||
criteria.All{criteria.Contains{"title": "love"}, criteria.Gt{"rating": 3}},
|
||||
"(media_file.title LIKE ? AND COALESCE(annotation.rating, 0) > ?)", "%love%", 3),
|
||||
"(media_file.title LIKE ? AND annotation.rating > ?)", "%love%", 3),
|
||||
Entry("any group",
|
||||
criteria.Any{criteria.Is{"title": "Low Rider"}, criteria.Is{"album": "Best Of"}},
|
||||
"(media_file.title = ? OR media_file.album = ?)", "Low Rider", "Best Of"),
|
||||
Entry("is string", criteria.Is{"title": "Low Rider"}, "media_file.title = ?", "Low Rider"),
|
||||
Entry("is bool", criteria.Is{"loved": true}, "COALESCE(annotation.starred, false) = ?", true),
|
||||
Entry("is bool", criteria.Is{"loved": true}, "annotation.starred = ?", true),
|
||||
Entry("is numeric list", criteria.Is{"library_id": []int{1, 2}}, "media_file.library_id IN (?,?)", 1, 2),
|
||||
Entry("is not", criteria.IsNot{"title": "Low Rider"}, "media_file.title <> ?", "Low Rider"),
|
||||
Entry("gt", criteria.Gt{"playCount": 10}, "COALESCE(annotation.play_count, 0) > ?", 10),
|
||||
Entry("lt", criteria.Lt{"playCount": 10}, "COALESCE(annotation.play_count, 0) < ?", 10),
|
||||
Entry("gt", criteria.Gt{"playCount": 10}, "annotation.play_count > ?", 10),
|
||||
Entry("lt", criteria.Lt{"playCount": 10}, "(annotation.play_count < ? OR annotation.play_count IS NULL)", 10),
|
||||
Entry("contains", criteria.Contains{"title": "Low Rider"}, "media_file.title LIKE ?", "%Low Rider%"),
|
||||
Entry("not contains", criteria.NotContains{"title": "Low Rider"}, "media_file.title NOT LIKE ?", "%Low Rider%"),
|
||||
Entry("starts with", criteria.StartsWith{"title": "Low Rider"}, "media_file.title LIKE ?", "Low Rider%"),
|
||||
|
|
@ -49,8 +49,51 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
|||
Entry("after", criteria.After{"lastPlayed": time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)}, "annotation.play_date > ?", time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)),
|
||||
Entry("in playlist", criteria.InPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1),
|
||||
Entry("not in playlist", criteria.NotInPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id NOT IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1),
|
||||
Entry("album annotation", criteria.Gt{"albumRating": 3}, "COALESCE(album_annotation.rating, 0) > ?", 3),
|
||||
Entry("artist annotation", criteria.Is{"artistLoved": true}, "COALESCE(artist_annotation.starred, false) = ?", true),
|
||||
Entry("album annotation", criteria.Gt{"albumRating": 3}, "album_annotation.rating > ?", 3),
|
||||
Entry("artist annotation", criteria.Is{"artistLoved": true}, "artist_annotation.starred = ?", true),
|
||||
// Annotation fields use a COALESCE default (0 for numeric, false for bool) so that tracks
|
||||
// with no annotation row behave as that default. To keep the annotation index usable, the
|
||||
// COALESCE is dropped when the compared value cannot match the default (the missing-row
|
||||
// case is then naturally excluded); otherwise an explicit `OR col IS NULL` preserves it.
|
||||
Entry("is safe (value != default)", criteria.Is{"playCount": 3}, "annotation.play_count = ?", 3),
|
||||
Entry("is unsafe (value == default)", criteria.Is{"playCount": 0},
|
||||
"(annotation.play_count = ? OR annotation.play_count IS NULL)", 0),
|
||||
Entry("is bool false (value == default)", criteria.Is{"loved": false},
|
||||
"(annotation.starred = ? OR annotation.starred IS NULL)", false),
|
||||
Entry("gt safe (value >= default)", criteria.Gt{"playCount": 0}, "annotation.play_count > ?", 0),
|
||||
Entry("gt unsafe (value < default)", criteria.Gt{"playCount": -1},
|
||||
"(annotation.play_count > ? OR annotation.play_count IS NULL)", -1),
|
||||
Entry("lt safe (value <= default)", criteria.Lt{"playCount": 0}, "annotation.play_count < ?", 0),
|
||||
Entry("lt unsafe (value > default)", criteria.Lt{"playCount": 5},
|
||||
"(annotation.play_count < ? OR annotation.play_count IS NULL)", 5),
|
||||
Entry("isNot annotation keeps null match", criteria.IsNot{"playCount": 3},
|
||||
"(annotation.play_count <> ? OR annotation.play_count IS NULL)", 3),
|
||||
Entry("isNot annotation value == default", criteria.IsNot{"playCount": 0},
|
||||
"annotation.play_count <> ?", 0),
|
||||
Entry("in range spanning default", criteria.InTheRange{"playCount": []int{-1, 5}},
|
||||
"((annotation.play_count >= ? OR annotation.play_count IS NULL) AND (annotation.play_count <= ? OR annotation.play_count IS NULL))", -1, 5),
|
||||
Entry("in range above default", criteria.InTheRange{"playCount": []int{1, 5}},
|
||||
"(annotation.play_count >= ? AND (annotation.play_count <= ? OR annotation.play_count IS NULL))", 1, 5),
|
||||
// A list value can't drive the index and a default-inclusive list has per-element NULL
|
||||
// semantics, so the COALESCE form is kept to stay equivalent to the original.
|
||||
Entry("is list keeps coalesce", criteria.Is{"playCount": []int{0, 3}},
|
||||
"COALESCE(annotation.play_count, 0) IN (?,?)", 0, 3),
|
||||
// LIKE operators can't use the column index, so annotation fields keep the COALESCE form to
|
||||
// match missing-annotation rows exactly as before (a NULL column never matches LIKE).
|
||||
Entry("contains annotation keeps coalesce", criteria.Contains{"playCount": 0},
|
||||
"COALESCE(annotation.play_count, 0) LIKE ?", "%0%"),
|
||||
Entry("starts with annotation keeps coalesce", criteria.StartsWith{"rating": 5},
|
||||
"COALESCE(annotation.rating, 0) LIKE ?", "5%"),
|
||||
Entry("not contains annotation keeps coalesce", criteria.NotContains{"playCount": 0},
|
||||
"COALESCE(annotation.play_count, 0) NOT LIKE ?", "%0%"),
|
||||
// Bool annotation fields only have a clean index-friendly form for equality; ordering
|
||||
// comparators keep the COALESCE form so the missing-row default is honored exactly.
|
||||
Entry("gt bool keeps coalesce", criteria.Gt{"loved": false},
|
||||
"COALESCE(annotation.starred, false) > ?", false),
|
||||
// A list value on a bool field is non-scalar, so it keeps the COALESCE form too (same as the
|
||||
// numeric list case) — otherwise a NULL column would diverge from the original.
|
||||
Entry("is bool list keeps coalesce", criteria.Is{"loved": []any{true}},
|
||||
"COALESCE(annotation.starred, false) IN (?)", true),
|
||||
Entry("tag is", criteria.Is{"genre": "Rock"}, "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value = ?)", "Rock"),
|
||||
Entry("tag is not", criteria.IsNot{"genre": "Rock"}, "not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value = ?)", "Rock"),
|
||||
Entry("tag contains", criteria.Contains{"genre": "Rock"}, "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value LIKE ?)", "%Rock%"),
|
||||
|
|
@ -220,6 +263,16 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
|||
Expect(err).To(MatchError(ContainSubstring("invalid boolean value for 'missing' expression")))
|
||||
})
|
||||
|
||||
It("returns an error for a range over a tag/role field", func() {
|
||||
_, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: criteria.InTheRange{"rate": []int{1, 5}}}).Where()
|
||||
Expect(err).To(MatchError(ContainSubstring("range operator not supported for tag/role field")))
|
||||
})
|
||||
|
||||
It("returns a clear error for a malformed range value", func() {
|
||||
_, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: criteria.InTheRange{"playCount": []int{1, 2, 3}}}).Where()
|
||||
Expect(err).To(MatchError(ContainSubstring("must be a [min, max] pair")))
|
||||
})
|
||||
|
||||
Describe("sort", func() {
|
||||
It("sorts by regular fields", func() {
|
||||
Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "title"}).OrderBy()).To(Equal("media_file.title asc"))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue