diff --git a/Makefile b/Makefile index b60fcfa8..2b19b601 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,10 @@ include .bingo/Variables.mk FILES_TO_FMT ?= $(shell find . -path ./vendor -prune -o -name '*.go' -print) MDOX_VALIDATE_CONFIG ?= .mdox.validate.yaml +# if macos, use gsed +SED ?= $(shell which gsed 2>/dev/null || which sed) + + define require_clean_work_tree @git update-index -q --ignore-submodules --refresh diff --git a/engine/engine_test.go b/engine/engine_test.go index fd7e2d28..46bfa392 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -1073,6 +1073,34 @@ func TestQueriesAgainstOldEngine(t *testing.T) { + on() group_left() sum(http_requests_total{ns="nginx"})`, }, + { + name: "binop with positive matcher using regex, only one side has data", + load: `load 30s + metric{} 1+2x5 + metric{} 1+2x20`, + query: `sum(rate(metric{err=~".+"}[5m])) / sum(rate(metric{}[5m]))`, + }, + { + name: "binop with positive matcher using regex, both sides have data", + load: `load 30s + metric{} 1+2x5 + metric{err="FooBarKey"} 1+2x20`, + query: `sum(rate(metric{err=~".+"}[5m])) / sum(rate(metric{}[5m]))`, + }, + { + name: "binop with negative matcher using regex, only one side has data", + load: `load 30s + metric{} 1+2x5 + metric{} 1+2x20`, + query: `sum(rate(metric{err!~".+"}[5m])) / sum(rate(metric{}[5m]))`, + }, + { + name: "binop with negative matcher using regex, both sides have data", + load: `load 30s + metric{} 1+2x5 + metric{err="FooBarKey"} 1+2x20`, + query: `sum(rate(metric{err!~".+"}[5m])) / sum(rate(metric{}[5m]))`, + }, { name: "scalar func with NaN", load: `load 30s diff --git a/execution/storage/filter.go b/execution/storage/filter.go index b35ca7b5..4af0f1a0 100644 --- a/execution/storage/filter.go +++ b/execution/storage/filter.go @@ -46,12 +46,9 @@ func (f filter) Matches(series storage.Series) bool { return true } - for _, l := range series.Labels() { - m, ok := f.matcherSet[l.Name] - if !ok { - continue - } - if !m.Matches(l.Value) { + for name, m := range f.matcherSet { + label := series.Labels().Get(name) + if !m.Matches(label) { return false } } diff --git a/execution/storage/filter_test.go b/execution/storage/filter_test.go new file mode 100644 index 00000000..af9c1db9 --- /dev/null +++ b/execution/storage/filter_test.go @@ -0,0 +1,88 @@ +// Copyright (c) The Thanos Community Authors. +// Licensed under the Apache License 2.0. + +package storage_test + +import ( + "testing" + + "github.com/prometheus/prometheus/model/labels" + promstg "github.com/prometheus/prometheus/storage" + "github.com/prometheus/prometheus/tsdb/chunkenc" + + "github.com/thanos-community/promql-engine/execution/storage" +) + +func TestFilter_Matches(t *testing.T) { + testCases := []struct { + name string + matchers []*labels.Matcher + series promstg.Series + expected bool + }{ + { + name: "empty matchers", + matchers: []*labels.Matcher{}, + series: &mockLabelSeries{labels: labels.FromStrings("foo", "bar")}, + expected: true, + }, + { + name: "no match", + matchers: []*labels.Matcher{labels.MustNewMatcher(labels.MatchEqual, "foo", "bar")}, + series: &mockLabelSeries{labels: labels.FromStrings("foo", "baz")}, + }, + { + name: "regex match", + matchers: []*labels.Matcher{labels.MustNewMatcher(labels.MatchRegexp, "foo", "ba.")}, + series: &mockLabelSeries{labels: labels.FromStrings("foo", "bar")}, + expected: true, + }, + { + name: "regex no match", + matchers: []*labels.Matcher{labels.MustNewMatcher(labels.MatchRegexp, "foo", "ba.")}, + series: &mockLabelSeries{labels: labels.FromStrings("foo", "nope")}, + }, + { + name: "multiple matchers", + matchers: []*labels.Matcher{labels.MustNewMatcher(labels.MatchEqual, "foo", "bar"), labels.MustNewMatcher(labels.MatchEqual, "baz", "qux")}, + series: &mockLabelSeries{labels: labels.FromStrings("foo", "bar", "baz", "qux")}, + expected: true, + }, + { + name: "single regex matcher, with label name not present", + matchers: []*labels.Matcher{labels.MustNewMatcher(labels.MatchRegexp, "foo", ".*")}, + series: &mockLabelSeries{labels: labels.FromStrings("bar", "baz")}, + expected: true, + }, + { + name: "single regex matcher, with label name not present, negative regex", + matchers: []*labels.Matcher{labels.MustNewMatcher(labels.MatchNotRegexp, "foo", ".*")}, + series: &mockLabelSeries{labels: labels.FromStrings("bar", "baz")}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + f := storage.NewFilter(tc.matchers) + if got := f.Matches(tc.series); got != tc.expected { + if tc.expected { + t.Errorf("expected %s to match %s, but it did not.", tc.series.Labels().String(), tc.matchers) + } else { + t.Errorf("expected %s to not match %s, but it did.", tc.series.Labels().String(), tc.matchers) + } + } + }) + } +} + +type mockLabelSeries struct { + labels labels.Labels +} + +func (s *mockLabelSeries) Labels() labels.Labels { + return s.labels +} + +func (s *mockLabelSeries) Iterator() chunkenc.Iterator { + return nil +}