Skip to content

Commit ba2c0ac

Browse files
authored
fix MergeSelectsOptimizer for contradictory matchers (#624)
Signed-off-by: Ahmed Hassan <[email protected]>
1 parent bb56ceb commit ba2c0ac

File tree

2 files changed

+16
-15
lines changed

2 files changed

+16
-15
lines changed

logicalplan/merge_selects.go

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,8 @@ func replaceMatchers(selectors matcherHeap, expr *Node) {
7575
filters := make([]*labels.Matcher, len(matchers))
7676
copy(filters, matchers)
7777

78-
// All replacements are done on metrics name only,
79-
// so we can drop the explicit metric name selector.
80-
filters = dropMatcher(labels.MetricName, filters)
81-
82-
// Drop filters which are already present as matchers in the replacement selector.
83-
for _, s := range replacement {
84-
for _, f := range filters {
85-
if s.Name == f.Name && s.Value == f.Value && s.Type == f.Type {
86-
filters = dropMatcher(f.Name, filters)
87-
}
88-
}
89-
}
78+
// Drop filters which are already present as matchers in the replacement selector including metric name selector.
79+
filters = dropMatcher(replacement, filters)
9080

9181
switch e := (*node).(type) {
9282
case *VectorSelector:
@@ -99,12 +89,19 @@ func replaceMatchers(selectors matcherHeap, expr *Node) {
9989
})
10090
}
10191

102-
func dropMatcher(matcherName string, originalMatchers []*labels.Matcher) []*labels.Matcher {
103-
res := slices.Clone(originalMatchers)
92+
func dropMatcher(toDrop []*labels.Matcher, original []*labels.Matcher) []*labels.Matcher {
93+
res := slices.Clone(original)
10494
i := 0
10595
for i < len(res) {
10696
l := res[i]
107-
if l.Name == matcherName {
97+
remove := false
98+
for _, m := range toDrop {
99+
if l.Name == m.Name && l.Type == m.Type && l.Value == m.Value {
100+
remove = true
101+
break
102+
}
103+
}
104+
if remove {
108105
res = slices.Delete(res, i, i+1)
109106
} else {
110107
i++

logicalplan/merge_selects_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ func TestMergeSelects(t *testing.T) {
3434
expr: `X{a="b"}/floor(X)`,
3535
expected: `filter([a="b"], X) / floor(X)`,
3636
},
37+
{
38+
expr: `X{a!~"b",a=~"b",c="d"}/X{a=~"b"}`,
39+
expected: `filter([a!~"b" c="d"], X{a=~"b"}) / X{a=~"b"}`,
40+
},
3741
{
3842
expr: `quantile by (pod) (scalar(min(http_requests_total)), http_requests_total)`,
3943
expected: `quantile by (pod) (scalar(min(http_requests_total)), http_requests_total)`,

0 commit comments

Comments
 (0)