fix(orFilters): fix multiple or filters would get wrong filtertype (#…

…13169)
grafana · Jun 7, 2024 · 9981e9e · 9981e9e
1 parent c996349
commit 9981e9e
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 10 deletions.
diff --git 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
@@ -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"`,
@@ -618,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)
 		})
 	}
 }

diff --git 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) {