Skip to content

Commit

Permalink
Update vectorAggEvaluator to fail for expressions without grouping
Browse files Browse the repository at this point in the history
  • Loading branch information
ssncferreira committed Mar 4, 2022
1 parent 757b4e1 commit 6612e97
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions pkg/logql/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
})
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down

0 comments on commit 6612e97

Please sign in to comment.