Improve matchers validations. (#3564)

1- It currently 500.
2- Happens too late, at ingester level and querier. (better at the frontend)
3- Improve API surface level, we don't need to expose this validation for anything else than ParseLogSelector.

see #3216

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

pkg/chunkenc/memchunk_test.go

@@ -999,7 +999,7 @@ func BenchmarkBufferedIteratorLabels(b *testing.B) {
 	} {
 		b.Run(test, func(b *testing.B) {
 			b.ReportAllocs()
-			expr, err := logql.ParseSampleExpr(test, true)
+			expr, err := logql.ParseSampleExpr(test)
 			if err != nil {
 				b.Fatal(err)
 			}

pkg/logql/ast.go

@@ -54,13 +54,13 @@ type SelectSampleParams struct {
 // Expr returns the SampleExpr from the SelectSampleParams.
 // The `LogSelectorExpr` can then returns all matchers and filters to use for that request.
 func (s SelectSampleParams) Expr() (SampleExpr, error) {
-	return ParseSampleExpr(s.Selector, true)
+	return ParseSampleExpr(s.Selector)
 }
 
 // LogSelector returns the LogSelectorExpr from the SelectParams.
 // The `LogSelectorExpr` can then returns all matchers and filters to use for that request.
 func (s SelectSampleParams) LogSelector() (LogSelectorExpr, error) {
-	expr, err := ParseSampleExpr(s.Selector, true)
+	expr, err := ParseSampleExpr(s.Selector)
 	if err != nil {
 		return nil, err
 	}

pkg/logql/functions_test.go

@@ -96,7 +96,7 @@ func Test_Extractor(t *testing.T) {
 			`,
 	} {
 		t.Run(tc, func(t *testing.T) {
-			expr, err := ParseSampleExpr(tc, true)
+			expr, err := ParseSampleExpr(tc)
 			require.Nil(t, err)
 			_, err = expr.Extractor()
 			require.Nil(t, err)

pkg/logql/log/parser_hints_test.go

@@ -209,7 +209,7 @@ func Test_ParserHints(t *testing.T) {
 		tt := tt
 		t.Run(tt.expr, func(t *testing.T) {
 			t.Parallel()
-			expr, err := logql.ParseSampleExpr(tt.expr, true)
+			expr, err := logql.ParseSampleExpr(tt.expr)
 			require.NoError(t, err)
 
 			ex, err := expr.Extractor()

pkg/logql/optimize.go

@@ -15,7 +15,7 @@ func optimizeSampleExpr(expr SampleExpr) (SampleExpr, error) {
 	}
 	// clone the expr.
 	q := expr.String()
-	expr, err := ParseSampleExpr(q, true)
+	expr, err := ParseSampleExpr(q)
 	if err != nil {
 		return nil, err
 	}

pkg/logql/optimize_test.go

@@ -7,11 +7,10 @@ import (
 )
 
 func Test_optimizeSampleExpr(t *testing.T) {
-
 	tests := []struct {
 		in, expected string
 	}{
-		//noop
+		// noop
 		{`1`, `1`},
 		{`1 + 1`, `2`},
 		{`topk(10,sum by(name)(rate({region="us-east1"}[5m])))`, `topk(10,sum by(name)(rate({region="us-east1"}[5m])))`},
@@ -28,7 +27,7 @@ func Test_optimizeSampleExpr(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.in, func(t *testing.T) {
-			e, err := ParseSampleExpr(tt.in, true)
+			e, err := ParseSampleExpr(tt.in)
 			require.NoError(t, err)
 			got, err := optimizeSampleExpr(e)
 			require.NoError(t, err)

pkg/logql/parser.go

@@ -58,7 +58,18 @@ func (p *parser) Parse() (Expr, error) {
 }
 
 // ParseExpr parses a string and returns an Expr.
-func ParseExpr(input string) (expr Expr, err error) {
+func ParseExpr(input string) (Expr, error) {
+	expr, err := parseExprWithoutValidation(input)
+	if err != nil {
+		return nil, err
+	}
+	if err := validateExpr(expr); err != nil {
+		return nil, err
+	}
+	return expr, nil
+}
+
+func parseExprWithoutValidation(input string) (expr Expr, err error) {
 	if len(input) >= maxInputSize {
 		return nil, newParseError(fmt.Sprintf("input size too long (%d > %d)", len(input), maxInputSize), 0, 0)
 	}
@@ -83,14 +94,32 @@ func ParseExpr(input string) (expr Expr, err error) {
 	return p.Parse()
 }
 
-// checkEqualityMatchers checks whether a query would touch all the streams in the query range or uses at least one matcher to select specific streams.
-func checkEqualityMatchers(matchers []*labels.Matcher) error {
+func validateExpr(expr Expr) error {
+	switch e := expr.(type) {
+	case SampleExpr:
+		err := validateSampleExpr(e)
+		if err != nil {
+			return err
+		}
+	case LogSelectorExpr:
+		err := validateMatchers(e.Matchers())
+		if err != nil {
+			return err
+		}
+	default:
+		return newParseError(fmt.Sprintf("unexpected expression type: %v", e), 0, 0)
+	}
+	return nil
+}
+
+// validateMatchers checks whether a query would touch all the streams in the query range or uses at least one matcher to select specific streams.
+func validateMatchers(matchers []*labels.Matcher) error {
 	if len(matchers) == 0 {
 		return nil
 	}
 	_, matchers = util.SplitFiltersAndMatchers(matchers)
 	if len(matchers) == 0 {
-		return errors.New(errAtleastOneEqualityMatcherRequired)
+		return newParseError(errAtleastOneEqualityMatcherRequired, 0, 0)
 	}
 
 	return nil
@@ -111,7 +140,7 @@ func ParseMatchers(input string) ([]*labels.Matcher, error) {
 }
 
 // ParseSampleExpr parses a string and returns the sampleExpr
-func ParseSampleExpr(input string, equalityMatcherRequired bool) (SampleExpr, error) {
+func ParseSampleExpr(input string) (SampleExpr, error) {
 	expr, err := ParseExpr(input)
 	if err != nil {
 		return nil, err
@@ -121,44 +150,36 @@ func ParseSampleExpr(input string, equalityMatcherRequired bool) (SampleExpr, er
 		return nil, errors.New("only sample expression supported")
 	}
 
-	if equalityMatcherRequired {
-		err = sampleExprCheckEqualityMatchers(sampleExpr)
-		if err != nil {
-			return nil, err
-		}
-	}
 	return sampleExpr, nil
 }
 
-func sampleExprCheckEqualityMatchers(expr SampleExpr) error {
+func validateSampleExpr(expr SampleExpr) error {
 	switch e := expr.(type) {
 	case *binOpExpr:
-		if err := sampleExprCheckEqualityMatchers(e.SampleExpr); err != nil {
+		if err := validateSampleExpr(e.SampleExpr); err != nil {
 			return err
 		}
 
-		return sampleExprCheckEqualityMatchers(e.RHS)
+		return validateSampleExpr(e.RHS)
 	case *literalExpr:
 		return nil
 	default:
-		return checkEqualityMatchers(expr.Selector().Matchers())
+		return validateMatchers(expr.Selector().Matchers())
 	}
 }
 
 // ParseLogSelector parses a log selector expression `{app="foo"} |= "filter"`
-func ParseLogSelector(input string, equalityMatcherRequired bool) (LogSelectorExpr, error) {
-	expr, err := ParseExpr(input)
+func ParseLogSelector(input string, validate bool) (LogSelectorExpr, error) {
+	expr, err := parseExprWithoutValidation(input)
 	if err != nil {
 		return nil, err
 	}
 	logSelector, ok := expr.(LogSelectorExpr)
 	if !ok {
 		return nil, errors.New("only log selector is supported")
 	}
-
-	if equalityMatcherRequired {
-		err = checkEqualityMatchers(logSelector.Matchers())
-		if err != nil {
+	if validate {
+		if err := validateExpr(expr); err != nil {
 			return nil, err
 		}
 	}