Fix topk and bottomk operations with int <= 0 (#6937)

* Fix topk and bottomk operations with int <= 0

* Fix VectorAggregationExpr String method

* Update rangemapper_test expression

* Add tests

* Update CHANGELOG.md

CHANGELOG.md

@@ -15,6 +15,7 @@
 * [5406](https://github.com/grafana/loki/pull/5406) **ctovena**: Revise the configuration parameters that configure the usage report to grafana.com.
 
 ##### Fixes
+* [6937](https://github.com/grafana/loki/pull/6937) **ssncferreira**: Fix topk and bottomk expressions with parameter <= 0.
 * [6358](https://github.com/grafana/loki/pull/6358) **taharah**: Fixes sigv4 authentication for the Ruler's remote write configuration by allowing both a global and per tenant configuration.
 * [6375](https://github.com/grafana/loki/pull/6375) **dannykopping**: Fix bug that prevented users from using the `json` parser after a `line_format` pipeline stage.
 ##### Changes

pkg/logql/rangemapper_test.go

@@ -1737,4 +1737,6 @@ func Test_FailQuery(t *testing.T) {
 	require.NoError(t, err)
 	_, _, err = rvm.Parse(`{app="foo"} |= "err"`)
 	require.Error(t, err)
+	_, _, err = rvm.Parse(`topk(0, sum(count_over_time({app="foo"} | json |  __error__="" [15m])))`)
+	require.Error(t, err)
 }

pkg/logql/syntax/ast.go

@@ -869,9 +869,13 @@ func mustNewVectorAggregationExpr(left SampleExpr, operation string, gr *Groupin
 		if params == nil {
 			panic(logqlmodel.NewParseError(fmt.Sprintf("parameter required for operation %s", operation), 0, 0))
 		}
-		if p, err = strconv.Atoi(*params); err != nil {
+		p, err = strconv.Atoi(*params)
+		if err != nil {
 			panic(logqlmodel.NewParseError(fmt.Sprintf("invalid parameter %s(%s,", operation, *params), 0, 0))
 		}
+		if p <= 0 {
+			panic(logqlmodel.NewParseError(fmt.Sprintf("invalid parameter (must be greater than 0) %s(%s", operation, *params), 0, 0))
+		}
 
 	default:
 		if params != nil {
@@ -924,10 +928,16 @@ func canInjectVectorGrouping(vecOp, rangeOp string) bool {
 
 func (e *VectorAggregationExpr) String() string {
 	var params []string
-	if e.Params != 0 {
+	switch e.Operation {
+	// bottomK and topk can have first parameter as 0
+	case OpTypeBottomK, OpTypeTopK:
 		params = []string{fmt.Sprintf("%d", e.Params), e.Left.String()}
-	} else {
-		params = []string{e.Left.String()}
+	default:
+		if e.Params != 0 {
+			params = []string{fmt.Sprintf("%d", e.Params), e.Left.String()}
+		} else {
+			params = []string{e.Left.String()}
+		}
 	}
 	return formatOperation(e.Operation, e.Grouping, params...)
 }

pkg/logql/syntax/ast_test.go

@@ -180,6 +180,21 @@ func Test_SampleExpr_String(t *testing.T) {
 	}
 }
 
+func Test_SampleExpr_String_Fail(t *testing.T) {
+	t.Parallel()
+	for _, tc := range []string{
+		`topk(0, sum(rate({region="us-east1"}[5m])) by (name))`,
+		`topk by (name)(0,sum(rate({region="us-east1"}[5m])))`,
+		`bottomk(0, sum(rate({region="us-east1"}[5m])) by (name))`,
+		`bottomk by (name)(0,sum(rate({region="us-east1"}[5m])))`,
+	} {
+		t.Run(tc, func(t *testing.T) {
+			_, err := ParseExpr(tc)
+			require.ErrorContains(t, err, "parse error : invalid parameter (must be greater than 0)")
+		})
+	}
+}
+
 func TestMatcherGroups(t *testing.T) {
 	for i, tc := range []struct {
 		query string