From 6612e977cfb7f26de8bffcb51ef678882fc477e5 Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Fri, 4 Mar 2022 13:05:44 +0000 Subject: [PATCH] Update vectorAggEvaluator to fail for expressions without grouping --- pkg/logql/evaluator.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/logql/evaluator.go b/pkg/logql/evaluator.go index 0d0bb2f19970..78113f9a2cf0 100644 --- a/pkg/logql/evaluator.go +++ b/pkg/logql/evaluator.go @@ -178,7 +178,7 @@ func (ev *DefaultEvaluator) StepEvaluator( &logproto.SampleQueryRequest{ Start: q.Start().Add(-rangExpr.Left.Interval).Add(-rangExpr.Left.Offset), End: q.End().Add(-rangExpr.Left.Offset), - Selector: e.String(), // intentionally send the the vector for reducing labels. + Selector: e.String(), // intentionally send the vector for reducing labels. Shards: q.Shards(), }, }) @@ -217,6 +217,9 @@ func vectorAggEvaluator( expr *VectorAggregationExpr, q Params, ) (StepEvaluator, error) { + if expr.Grouping == nil { + return nil, errors.Errorf("aggregation operator '%q' without grouping", expr.Operation) + } nextEvaluator, err := ev.StepEvaluator(ctx, ev, expr.Left, q) if err != nil { return nil, err @@ -553,7 +556,7 @@ func binOpStepEvaluator( ctx, cancel := context.WithCancel(ctx) g := errgroup.Group{} - // We have two non literal legs, + // We have two non-literal legs, // load them in parallel g.Go(func() error { var err error @@ -669,7 +672,7 @@ func vectorBinop(op string, opts *BinOpOptions, lhs, rhs promql.Vector, lsigs, r matchedSigs := make(map[uint64]map[uint64]struct{}) results := make(promql.Vector, 0) - // Add all rhs samples to a map so we can easily find matches later. + // Add all rhs samples to a map, so we can easily find matches later. for i, sample := range rhs { sig := rsigs[i] if rightSigs[sig] != nil { @@ -1059,7 +1062,7 @@ func mergeBinOp(op string, left, right *promql.Sample, filter, isVectorCompariso if filter { // if a filter-enabled vector-wise comparison has returned non-nil, - // ensure we return the left hand side's value (2) instead of the + // ensure we return the left-hand side's value (2) instead of the // comparison operator's result (1: the truthy answer) if res != nil { return left @@ -1069,7 +1072,7 @@ func mergeBinOp(op string, left, right *promql.Sample, filter, isVectorCompariso } // literalStepEvaluator merges a literal with a StepEvaluator. Since order matters in -// non commutative operations, inverted should be true when the literalExpr is not the left argument. +// non-commutative operations, inverted should be true when the literalExpr is not the left argument. func literalStepEvaluator( op string, lit *LiteralExpr,