mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
perf(smartplaylists): merge negated artist/tag rules into one NOT EXISTS
* fix(smartplaylists): merge negated artist/tag rules in AND groups Smart playlists with many negated role/tag conditions ANDed together (e.g. 100+ "isNot artist" rules, issue #5511) generated one correlated NOT EXISTS subquery per rule, scanning media_file_artists for every candidate row. On large libraries this took minutes and triggered API timeouts and SQLite lock contention. By De Morgan, "NOT EXISTS(role=X) AND NOT EXISTS(role=Y)" is equivalent to "NOT EXISTS(role=X OR role=Y)", so multiple negated conditions for the same field can be collapsed into a single batched NOT EXISTS. This mirrors the existing OR-group merge that #5515 added for positive conditions. The shared grouping/batching logic is extracted into mergeSameFieldConds, parameterized by polarity, so the OR/positive and AND/negated paths reuse one algorithm instead of duplicating it. roleCondGroup/tagCondGroup gain a 'not' flag to emit the negated subquery. Benchmark (323k tracks, 120 isNot artist rules, reporter's exact shape): merged ~54ms vs unmerged ~8.7s steady-state (~160x faster). * docs: trim redundant comments on merge helpers The De Morgan explanation was repeated across three doc comments. Keep it in one place (mergeNegatedJsonConds, where negation is introduced) and reduce the shared core and group-type comments to concise one-liners.
This commit is contained in:
parent
c466f6b612
commit
f3887df334
@ -140,7 +140,7 @@ func (c smartPlaylistCriteria) exprSQL(expr criteria.Expression) (squirrel.Sqliz
|
||||
}
|
||||
and = append(and, cond)
|
||||
}
|
||||
return and, nil
|
||||
return mergeNegatedJsonConds(and), nil
|
||||
case criteria.Any:
|
||||
or := squirrel.Or{}
|
||||
for _, child := range e {
|
||||
@ -454,6 +454,25 @@ const jsonCondBatchSize = 350
|
||||
// This turns N separate correlated subqueries into ceil(N/batchSize), dramatically
|
||||
// improving performance for smart playlists with many patterns.
|
||||
func mergeJsonConds(or squirrel.Or) squirrel.Sqlizer {
|
||||
if merged, ok := mergeSameFieldConds(or, false); ok {
|
||||
return squirrel.Or(merged)
|
||||
}
|
||||
return or
|
||||
}
|
||||
|
||||
// mergeNegatedJsonConds is the AND-group counterpart to mergeJsonConds, merging negated
|
||||
// conditions. By De Morgan, "NOT EXISTS(X) AND NOT EXISTS(Y)" == "NOT EXISTS(X OR Y)".
|
||||
func mergeNegatedJsonConds(and squirrel.And) squirrel.Sqlizer {
|
||||
if merged, ok := mergeSameFieldConds(and, true); ok {
|
||||
return squirrel.And(merged)
|
||||
}
|
||||
return and
|
||||
}
|
||||
|
||||
// mergeSameFieldConds groups roleCond/tagCond entries that share a field and the requested
|
||||
// polarity, replacing each group of 2+ with batched roleCondGroup/tagCondGroup subqueries.
|
||||
// Returns the rewritten conditions and whether any merge happened.
|
||||
func mergeSameFieldConds(conds []squirrel.Sqlizer, negated bool) ([]squirrel.Sqlizer, bool) {
|
||||
type condEntry struct {
|
||||
index int
|
||||
cond squirrel.Sqlizer
|
||||
@ -465,10 +484,10 @@ func mergeJsonConds(or squirrel.Or) squirrel.Sqlizer {
|
||||
tag string
|
||||
}
|
||||
groups := make(map[string]*group)
|
||||
for i, s := range or {
|
||||
for i, s := range conds {
|
||||
switch c := s.(type) {
|
||||
case roleCond:
|
||||
if c.not || c.cond == nil {
|
||||
if c.not != negated || c.cond == nil {
|
||||
continue
|
||||
}
|
||||
g, exists := groups["role:"+c.role]
|
||||
@ -478,7 +497,7 @@ func mergeJsonConds(or squirrel.Or) squirrel.Sqlizer {
|
||||
}
|
||||
g.entries = append(g.entries, condEntry{index: i, cond: c.cond})
|
||||
case tagCond:
|
||||
if c.not || c.cond == nil {
|
||||
if c.not != negated || c.cond == nil {
|
||||
continue
|
||||
}
|
||||
g, exists := groups["tag:"+c.tag]
|
||||
@ -490,7 +509,6 @@ func mergeJsonConds(or squirrel.Or) squirrel.Sqlizer {
|
||||
}
|
||||
}
|
||||
|
||||
merged := false
|
||||
remove := make(map[int]bool)
|
||||
var additions []squirrel.Sqlizer
|
||||
for _, key := range slices.Sorted(maps.Keys(groups)) {
|
||||
@ -498,45 +516,42 @@ func mergeJsonConds(or squirrel.Or) squirrel.Sqlizer {
|
||||
if len(g.entries) < 2 {
|
||||
continue
|
||||
}
|
||||
merged = true
|
||||
for _, e := range g.entries {
|
||||
remove[e.index] = true
|
||||
}
|
||||
conds := make([]squirrel.Sqlizer, len(g.entries))
|
||||
batchConds := make([]squirrel.Sqlizer, len(g.entries))
|
||||
for i, e := range g.entries {
|
||||
conds[i] = e.cond
|
||||
remove[e.index] = true
|
||||
batchConds[i] = e.cond
|
||||
}
|
||||
if g.isRole {
|
||||
role := key[len("role:"):]
|
||||
for batch := range slices.Chunk(conds, jsonCondBatchSize) {
|
||||
additions = append(additions, roleCondGroup{role: role, conds: batch})
|
||||
for batch := range slices.Chunk(batchConds, jsonCondBatchSize) {
|
||||
additions = append(additions, roleCondGroup{role: role, conds: batch, not: negated})
|
||||
}
|
||||
} else {
|
||||
for batch := range slices.Chunk(conds, jsonCondBatchSize) {
|
||||
additions = append(additions, tagCondGroup{tag: g.tag, numeric: g.numeric, conds: batch})
|
||||
for batch := range slices.Chunk(batchConds, jsonCondBatchSize) {
|
||||
additions = append(additions, tagCondGroup{tag: g.tag, numeric: g.numeric, conds: batch, not: negated})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if !merged {
|
||||
return or
|
||||
if len(remove) == 0 {
|
||||
return conds, false
|
||||
}
|
||||
|
||||
result := make(squirrel.Or, 0, len(or)-len(remove)+len(additions))
|
||||
for i, s := range or {
|
||||
result := make([]squirrel.Sqlizer, 0, len(conds)-len(remove)+len(additions))
|
||||
for i, s := range conds {
|
||||
if !remove[i] {
|
||||
result = append(result, s)
|
||||
}
|
||||
}
|
||||
result = append(result, additions...)
|
||||
return result
|
||||
return append(result, additions...), true
|
||||
}
|
||||
|
||||
// roleCondGroup represents multiple role conditions for the same role, merged into
|
||||
// a single EXISTS subquery for performance.
|
||||
// roleCondGroup represents multiple role conditions for the same role, merged into a single
|
||||
// (optionally negated) EXISTS subquery for performance.
|
||||
type roleCondGroup struct {
|
||||
role string
|
||||
conds []squirrel.Sqlizer
|
||||
not bool
|
||||
}
|
||||
|
||||
func (g roleCondGroup) ToSql() (string, []any, error) {
|
||||
@ -551,15 +566,19 @@ func (g roleCondGroup) ToSql() (string, []any, error) {
|
||||
allArgs = append(allArgs, args...)
|
||||
}
|
||||
cond := roleExistsSQL("(" + strings.Join(innerParts, " OR ") + ")")
|
||||
if g.not {
|
||||
cond = "not " + cond
|
||||
}
|
||||
return cond, allArgs, nil
|
||||
}
|
||||
|
||||
// tagCondGroup represents multiple tag conditions for the same tag, merged into
|
||||
// a single EXISTS subquery for performance.
|
||||
// tagCondGroup represents multiple tag conditions for the same tag, merged into a single
|
||||
// (optionally negated) EXISTS subquery for performance.
|
||||
type tagCondGroup struct {
|
||||
tag string
|
||||
numeric bool
|
||||
conds []squirrel.Sqlizer
|
||||
not bool
|
||||
}
|
||||
|
||||
func (g tagCondGroup) ToSql() (string, []any, error) {
|
||||
@ -578,6 +597,9 @@ func (g tagCondGroup) ToSql() (string, []any, error) {
|
||||
}
|
||||
cond := fmt.Sprintf("exists (select 1 from json_tree(media_file.tags, '$.%s') where key='value' and (%s))",
|
||||
g.tag, strings.Join(innerParts, " OR "))
|
||||
if g.not {
|
||||
cond = "not " + cond
|
||||
}
|
||||
return cond, allArgs, nil
|
||||
}
|
||||
|
||||
|
||||
@ -60,6 +60,61 @@ func BenchmarkSmartPlaylistRole(b *testing.B) {
|
||||
})
|
||||
}
|
||||
|
||||
// BenchmarkSmartPlaylistNegatedRole compares performance for smart playlists with many
|
||||
// negated role conditions ANDed together (e.g. 500 "isNot artist" rules, issue #5511)
|
||||
// between the current implementation (merged NOT EXISTS via criteria pipeline) and the
|
||||
// old baseline (one separate NOT EXISTS subquery per pattern).
|
||||
func BenchmarkSmartPlaylistNegatedRole(b *testing.B) {
|
||||
configtest.SetupConfig()
|
||||
tmpDir := b.TempDir()
|
||||
conf.Server.DbPath = filepath.Join(tmpDir, "bench-smartpl-neg.db")
|
||||
cleanup := db.Init(context.Background())
|
||||
defer cleanup()
|
||||
log.SetLevel(log.LevelFatal)
|
||||
|
||||
conn := dbx.NewFromDB(db.Db(), db.Dialect)
|
||||
ctx := log.NewContext(context.Background())
|
||||
user := model.User{ID: "bench-user", UserName: "bench", Name: "Bench User", IsAdmin: true}
|
||||
ctx = request.WithUser(ctx, user)
|
||||
|
||||
setupBenchData(b, ctx, conn, user)
|
||||
criteria.AddRoles([]string{"artist"})
|
||||
|
||||
// Build the criteria expression: 500 "isNot artist" patterns in an AND group
|
||||
allExprs := make(criteria.All, benchNumPatterns)
|
||||
for i := range benchNumPatterns {
|
||||
allExprs[i] = criteria.IsNot{"artist": fmt.Sprintf("Artist %04d", i)}
|
||||
}
|
||||
expr := criteria.Criteria{Expression: allExprs, Sort: "title", Limit: 500}
|
||||
|
||||
b.Run("Current", func(b *testing.B) {
|
||||
benchmarkCriteriaPipeline(b, ctx, expr)
|
||||
})
|
||||
b.Run("Baseline_UnmergedNotExists", func(b *testing.B) {
|
||||
benchmarkUnmergedNegatedJSONTree(b, ctx)
|
||||
})
|
||||
}
|
||||
|
||||
// benchmarkUnmergedNegatedJSONTree builds the old-style query with N separate negated
|
||||
// json_tree EXISTS subqueries ANDed together (the pre-optimization baseline).
|
||||
func benchmarkUnmergedNegatedJSONTree(b *testing.B, ctx context.Context) {
|
||||
b.Helper()
|
||||
|
||||
var sb strings.Builder
|
||||
sb.WriteString("SELECT media_file.id FROM media_file WHERE (")
|
||||
args := make([]any, 0, benchNumPatterns)
|
||||
for i := range benchNumPatterns {
|
||||
if i > 0 {
|
||||
sb.WriteString(" AND ")
|
||||
}
|
||||
sb.WriteString("not exists (select 1 from json_tree(media_file.participants, '$.artist') where key='name' and value = ?)")
|
||||
args = append(args, fmt.Sprintf("Artist %04d", i))
|
||||
}
|
||||
sb.WriteString(") ORDER BY media_file.title LIMIT 500")
|
||||
|
||||
runBenchQuery(b, ctx, sb.String(), args)
|
||||
}
|
||||
|
||||
// benchmarkCriteriaPipeline runs the criteria through the actual production code path:
|
||||
// newSmartPlaylistCriteria → Where() → ToSql(), then executes the resulting query.
|
||||
func benchmarkCriteriaPipeline(b *testing.B, ctx context.Context, expr criteria.Criteria) {
|
||||
|
||||
@ -408,6 +408,97 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
||||
Expect(sql).To(ContainSubstring("value LIKE ? OR value LIKE ?"))
|
||||
Expect(args).To(HaveLen(2 + 2 + 1)) // 2 tag patterns + 2 role patterns + 1 role name
|
||||
})
|
||||
|
||||
It("merges negated role conditions in an AND group into a single NOT EXISTS", func() {
|
||||
expr := criteria.All{
|
||||
criteria.IsNot{"artist": "Beatles"},
|
||||
criteria.IsNot{"artist": "Kraftwerk"},
|
||||
}
|
||||
sqlizer, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: expr}).Where()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
sql, args, err := sqlizer.ToSql()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
// A single NOT EXISTS with both names ORed inside (De Morgan)
|
||||
Expect(strings.Count(sql, "not exists")).To(Equal(1))
|
||||
Expect(sql).To(ContainSubstring("artist.name = ? OR artist.name = ?"))
|
||||
Expect(args).To(HaveExactElements("artist", "Beatles", "Kraftwerk"))
|
||||
})
|
||||
|
||||
It("merges negated notContains role conditions in an AND group", func() {
|
||||
expr := criteria.All{
|
||||
criteria.NotContains{"artist": "Beatles"},
|
||||
criteria.NotContains{"artist": "Kraftwerk"},
|
||||
}
|
||||
sqlizer, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: expr}).Where()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
sql, args, err := sqlizer.ToSql()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(strings.Count(sql, "not exists")).To(Equal(1))
|
||||
Expect(sql).To(ContainSubstring("artist.name LIKE ? OR artist.name LIKE ?"))
|
||||
Expect(args).To(HaveExactElements("artist", "%Beatles%", "%Kraftwerk%"))
|
||||
})
|
||||
|
||||
It("merges negated tag conditions in an AND group into a single NOT EXISTS", func() {
|
||||
expr := criteria.All{
|
||||
criteria.NotContains{"genre": "Rock"},
|
||||
criteria.NotContains{"genre": "Metal"},
|
||||
}
|
||||
sqlizer, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: expr}).Where()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
sql, args, err := sqlizer.ToSql()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(strings.Count(sql, "not exists")).To(Equal(1))
|
||||
Expect(sql).To(ContainSubstring("value LIKE ? OR value LIKE ?"))
|
||||
Expect(args).To(HaveExactElements("%Rock%", "%Metal%"))
|
||||
})
|
||||
|
||||
It("does not merge a single negated condition with a positive one of the same role in AND", func() {
|
||||
// AND of mixed polarity must not be collapsed: NOT EXISTS(a) AND EXISTS(b)
|
||||
// is not equivalent to any single merged subquery.
|
||||
expr := criteria.All{
|
||||
criteria.Contains{"artist": "Beatles"},
|
||||
criteria.IsNot{"artist": "Kraftwerk"},
|
||||
}
|
||||
sqlizer, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: expr}).Where()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
sql, _, err := sqlizer.ToSql()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
// One positive EXISTS and one negated NOT EXISTS, kept separate
|
||||
Expect(strings.Count(sql, "not exists")).To(Equal(1))
|
||||
Expect(strings.Count(sql, "exists")).To(Equal(2)) // "not exists" contains "exists"
|
||||
})
|
||||
|
||||
It("does not merge negated conditions of different roles in AND", func() {
|
||||
expr := criteria.All{
|
||||
criteria.IsNot{"artist": "Beatles"},
|
||||
criteria.IsNot{"composer": "Lennon"},
|
||||
}
|
||||
sqlizer, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: expr}).Where()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
sql, _, err := sqlizer.ToSql()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(strings.Count(sql, "not exists")).To(Equal(2))
|
||||
})
|
||||
|
||||
It("batches large negated AND groups to avoid SQLite expression tree depth limit", func() {
|
||||
allExprs := make(criteria.All, jsonCondBatchSize+1)
|
||||
for i := range allExprs {
|
||||
allExprs[i] = criteria.IsNot{"artist": fmt.Sprintf("Artist%d", i)}
|
||||
}
|
||||
sqlizer, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: allExprs}).Where()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
sql, args, err := sqlizer.ToSql()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
// Two NOT EXISTS subqueries (one batch of jsonCondBatchSize, one of 1)
|
||||
Expect(strings.Count(sql, "not exists")).To(Equal(2))
|
||||
Expect(args).To(HaveLen(2 + jsonCondBatchSize + 1))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("joins", func() {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user