diff --git a/pkg/models/task_collection_test.go b/pkg/models/task_collection_test.go index 6a8c5ebd7..73423001d 100644 --- a/pkg/models/task_collection_test.go +++ b/pkg/models/task_collection_test.go @@ -1481,6 +1481,63 @@ func TestTaskCollection_ReadAll(t *testing.T) { }, wantErr: false, }, + { + // Regression: AND-joined equality filters on the same sub-table must + // NOT be merged into a single EXISTS (each label lives on a separate row). + name: "filter labels AND both must exist", + fields: fields{ + Filter: "labels = 4 && labels = 5", + }, + args: defaultArgs, + want: []*Task{ + task35, // only task with both labels 4 and 5 + }, + wantErr: false, + }, + { + // Regression: AND-joined negative filters must NOT be merged into a + // single NOT EXISTS (would produce trivially-true condition). + name: "filter labels not eq AND both excluded", + fields: fields{ + Filter: "labels != 4 && labels != 5", + }, + args: defaultArgs, + want: []*Task{ + // Tasks that have neither label 4 nor label 5 + task3, + task4, + task5, + task6, + task7, + task8, + task9, + task10, + task11, + task12, + task15, + task16, + task17, + task18, + task19, + task20, + task21, + task22, + task23, + task24, + task25, + task26, + task27, + task28, + task29, + task30, + task31, + task32, + task33, + task39, + task47, + }, + wantErr: false, + }, { name: "filter project_id", fields: fields{ diff --git a/pkg/models/task_search.go b/pkg/models/task_search.go index adab5835e..44cf9c33d 100644 --- a/pkg/models/task_search.go +++ b/pkg/models/task_search.go @@ -89,6 +89,16 @@ var strictComparators = map[taskFilterComparator]bool{ taskFilterComparatorNotEquals: true, } +// isRangeComparator returns true for comparators where combining multiple +// conditions into a single EXISTS subquery is semantically correct (i.e. a +// single row can satisfy both conditions simultaneously). +func isRangeComparator(c taskFilterComparator) bool { + return c == taskFilterComparatorGreater || + c == taskFilterComparatorGreateEquals || + c == taskFilterComparatorLess || + c == taskFilterComparatorLessEquals +} + type taskSearcher interface { Search(opts *taskSearchOptions) (tasks []*Task, totalCount int64, err error) } @@ -182,25 +192,26 @@ func convertFiltersToDBFilterCond(rawFilters []*taskFilter, includeNulls bool) ( continue } - // Collect all consecutive AND-joined filters targeting the same sub-table + // Collect all consecutive AND-joined range filters targeting the same sub-table. + // Only range comparators (>, >=, <, <=) are merged because they express + // conditions a single row can satisfy simultaneously (e.g. reminder > X AND + // reminder < Y). Equality/IN/NOT comparators must remain as separate EXISTS + // subqueries because each matching value lives in its own row (e.g. + // labels = 4 && labels = 5 means two different rows must each exist). group := []*taskFilter{f} - for i+1 < len(rawFilters) { - next := rawFilters[i+1] - nextSubTable, nextOk := subTableFilters[next.field] - if !nextOk || nextSubTable.Table != subTableFilterParams.Table || next.join != filterConcatAnd { - break + if isRangeComparator(f.comparator) { + for i+1 < len(rawFilters) { + next := rawFilters[i+1] + nextSubTable, nextOk := subTableFilters[next.field] + if !nextOk || nextSubTable.Table != subTableFilterParams.Table || next.join != filterConcatAnd { + break + } + if !isRangeComparator(next.comparator) { + break + } + group = append(group, next) + i++ } - if next.field == "assignees" && next.comparator == taskFilterComparatorLike { - break - } - // Don't mix positive and negative filters (EXISTS vs NOT EXISTS) - currentIsNegative := f.comparator == taskFilterComparatorNotEquals || f.comparator == taskFilterComparatorNotIn - nextIsNegative := next.comparator == taskFilterComparatorNotEquals || next.comparator == taskFilterComparatorNotIn - if currentIsNegative != nextIsNegative { - break - } - group = append(group, next) - i++ } // Build the combined condition for all filters in the group @@ -232,7 +243,7 @@ func convertFiltersToDBFilterCond(rawFilters []*taskFilter, includeNulls bool) ( filterSubQuery := subTableFilterParams.ToBaseSubQuery().And(combinedInnerCond) var filter builder.Cond - if group[0].comparator == taskFilterComparatorNotEquals || group[0].comparator == taskFilterComparatorNotIn { + if f.comparator == taskFilterComparatorNotEquals || f.comparator == taskFilterComparatorNotIn { filter = builder.NotExists(filterSubQuery) } else { filter = builder.Exists(filterSubQuery) @@ -243,8 +254,10 @@ func convertFiltersToDBFilterCond(rawFilters []*taskFilter, includeNulls bool) ( } dbFilters = append(dbFilters, filter) - // Use the join from the last filter in the group (how this group connects to the next filter) - dbFilterJoins = append(dbFilterJoins, group[len(group)-1].join) + // Use the join from the first filter in the group: f.join describes how + // this group connects to the previous element (matches the convention + // where dbFilterJoins[i+1] combines dbFilters[i] with dbFilters[i+1]). + dbFilterJoins = append(dbFilterJoins, f.join) continue }