From cf0948b4ebeb8a880ac88c009687eb238bb6502d Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Fri, 7 Jun 2024 11:43:15 +0200 Subject: [PATCH 1/4] fix(orFilters): fix multiple or filters would get wrong filtertype --- pkg/logql/syntax/ast.go | 15 +++++++-------- pkg/logql/syntax/ast_test.go | 11 +++++++++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/logql/syntax/ast.go b/pkg/logql/syntax/ast.go index 6e3f18b7cc8e6..e5e80b4d0c172 100644 --- a/pkg/logql/syntax/ast.go +++ b/pkg/logql/syntax/ast.go @@ -366,14 +366,6 @@ func newLineFilterExpr(ty log.LineMatchType, op, match string) *LineFilterExpr { func newOrLineFilter(left, right *LineFilterExpr) *LineFilterExpr { right.Ty = left.Ty - if left.Ty == log.LineMatchEqual || left.Ty == log.LineMatchRegexp || left.Ty == log.LineMatchPattern { - left.Or = right - right.IsOrChild = true - return left - } - - // !(left or right) == (!left and !right). - // NOTE: Consider, we have chain of "or", != "foo" or "bar" or "baz" // we parse from right to left, so first time left="bar", right="baz", and we don't know the actual `Ty` (equal: |=, notequal: !=, regex: |~, etc). So // it will have default (0, LineMatchEqual). @@ -385,6 +377,13 @@ func newOrLineFilter(left, right *LineFilterExpr) *LineFilterExpr { tmp = tmp.Or } + if left.Ty == log.LineMatchEqual || left.Ty == log.LineMatchRegexp || left.Ty == log.LineMatchPattern { + left.Or = right + right.IsOrChild = true + return left + } + + // !(left or right) == (!left and !right). return newNestedLineFilterExpr(left, right) } diff --git a/pkg/logql/syntax/ast_test.go b/pkg/logql/syntax/ast_test.go index 9090fc98b7558..228ebc165d058 100644 --- a/pkg/logql/syntax/ast_test.go +++ b/pkg/logql/syntax/ast_test.go @@ -545,11 +545,18 @@ func Test_FilterMatcher(t *testing.T) { []linecheck{{"foo", false}, {"bar", true}, {"127.0.0.2", true}, {"127.0.0.1", false}}, }, { - `{app="foo"} |> "foo" or "bar"`, + `{app="foo"} |> "<_>foo<_>" or "<_>bar<_>"`, []*labels.Matcher{ mustNewMatcher(labels.MatchEqual, "app", "foo"), }, - []linecheck{{"foo", true}, {"bar", true}, {"none", false}}, + []linecheck{{"test foo test", true}, {"test bar test", true}, {"none", false}}, + }, + { + `{app="foo"} |> "<_>foo<_>" or "<_>bar<_>" or "<_>baz<_>"`, + []*labels.Matcher{ + mustNewMatcher(labels.MatchEqual, "app", "foo"), + }, + []linecheck{{"test foo test", true}, {"test bar test", true}, {"test baz test", true}, {"none", false}}, }, { `{app="foo"} !> "foo" or "bar"`, From 4f9e8dafc60895d004fc0ce5f6dbd8aa1c6fa2b2 Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Fri, 7 Jun 2024 12:23:01 +0200 Subject: [PATCH 2/4] fix(orFilters): test types in nested or filters --- pkg/logql/syntax/ast_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/logql/syntax/ast_test.go b/pkg/logql/syntax/ast_test.go index 228ebc165d058..a9dec7d586bd6 100644 --- a/pkg/logql/syntax/ast_test.go +++ b/pkg/logql/syntax/ast_test.go @@ -543,7 +543,7 @@ func Test_FilterMatcher(t *testing.T) { mustNewMatcher(labels.MatchEqual, "app", "foo"), }, []linecheck{{"foo", false}, {"bar", true}, {"127.0.0.2", true}, {"127.0.0.1", false}}, - }, + }, { `{app="foo"} |> "<_>foo<_>" or "<_>bar<_>"`, []*labels.Matcher{ @@ -625,6 +625,18 @@ func TestOrLineFilterTypes(t *testing.T) { _ = newOrLineFilter(left, right) require.Equal(t, tt.ty, right.Ty) + require.Equal(t, tt.ty, left.Ty) + }) + + t.Run("right inherits left's type with multiple or filters", func(t *testing.T) { + f1 := &LineFilterExpr{LineFilter: LineFilter{Ty: tt.ty, Match: "something"}} + f2 := &LineFilterExpr{LineFilter: LineFilter{Ty: log.LineMatchEqual, Match: "something"}} + f3 := &LineFilterExpr{LineFilter: LineFilter{Ty: log.LineMatchEqual, Match: "something"}} + + _ = newOrLineFilter(f1, newOrLineFilter(f2, f3)) + require.Equal(t, tt.ty, f1.Ty) + require.Equal(t, tt.ty, f2.Ty) + require.Equal(t, tt.ty, f3.Ty) }) } } From 92ed73074c7a9e31b57c73092e0c91f9419d04fe Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Fri, 7 Jun 2024 12:29:38 +0200 Subject: [PATCH 3/4] fix(orFilters): lint --- pkg/logql/syntax/ast_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/logql/syntax/ast_test.go b/pkg/logql/syntax/ast_test.go index a9dec7d586bd6..d75ff2d0261b6 100644 --- a/pkg/logql/syntax/ast_test.go +++ b/pkg/logql/syntax/ast_test.go @@ -543,7 +543,7 @@ func Test_FilterMatcher(t *testing.T) { mustNewMatcher(labels.MatchEqual, "app", "foo"), }, []linecheck{{"foo", false}, {"bar", true}, {"127.0.0.2", true}, {"127.0.0.1", false}}, - }, + }, { `{app="foo"} |> "<_>foo<_>" or "<_>bar<_>"`, []*labels.Matcher{ From 62ec857b95057b0a4ede999eca4216a32ca23830 Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Fri, 7 Jun 2024 12:37:49 +0200 Subject: [PATCH 4/4] fix(orFilters): add parser test --- pkg/logql/syntax/parser_test.go | 60 +++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/pkg/logql/syntax/parser_test.go b/pkg/logql/syntax/parser_test.go index f12309f2b24a5..4c2a85203938b 100644 --- a/pkg/logql/syntax/parser_test.go +++ b/pkg/logql/syntax/parser_test.go @@ -3173,6 +3173,66 @@ var ParseTestCases = []struct { }, }, }, + { + in: `{app="foo"} |= "foo" or "bar" or "baz"`, + exp: &PipelineExpr{ + Left: newMatcherExpr([]*labels.Matcher{mustNewMatcher(labels.MatchEqual, "app", "foo")}), + MultiStages: MultiStageExpr{ + &LineFilterExpr{ + LineFilter: LineFilter{ + Ty: log.LineMatchEqual, + Match: "foo", + }, + Or: newOrLineFilter( + &LineFilterExpr{ + LineFilter: LineFilter{ + Ty: log.LineMatchEqual, + Match: "bar", + }, + IsOrChild: true, + }, + &LineFilterExpr{ + LineFilter: LineFilter{ + Ty: log.LineMatchEqual, + Match: "baz", + }, + IsOrChild: true, + }), + IsOrChild: false, + }, + }, + }, + }, + { + in: `{app="foo"} |> "foo" or "bar" or "baz"`, + exp: &PipelineExpr{ + Left: newMatcherExpr([]*labels.Matcher{mustNewMatcher(labels.MatchEqual, "app", "foo")}), + MultiStages: MultiStageExpr{ + &LineFilterExpr{ + LineFilter: LineFilter{ + Ty: log.LineMatchPattern, + Match: "foo", + }, + Or: newOrLineFilter( + &LineFilterExpr{ + LineFilter: LineFilter{ + Ty: log.LineMatchPattern, + Match: "bar", + }, + IsOrChild: true, + }, + &LineFilterExpr{ + LineFilter: LineFilter{ + Ty: log.LineMatchPattern, + Match: "baz", + }, + IsOrChild: true, + }), + IsOrChild: false, + }, + }, + }, + }, } func TestParse(t *testing.T) {