From 71571a8ec47a23b838e5798e1d4d5461a6f4a8bf Mon Sep 17 00:00:00 2001
From: beorn7 <beorn@soundcloud.com>
Date: Wed, 24 Aug 2016 18:37:09 +0200
Subject: [PATCH] promql: Fix (and simplify) populating iterators
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This was only relevant so far for the benchmark suite as it would
recycle Expr for repetitions. However, the append is unnecessary as
each node is only inspected once when populating iterators, and
population must always start from scratch.

This also introduces error checking during benchmarks and fixes the so
far undetected test errors during benchmarking.

Also, remove a style nit (two golint warnings less…).
---
 promql/bench.go          |  4 +++-
 promql/engine.go         | 20 +++++---------------
 promql/functions.go      |  4 ++--
 promql/functions_test.go |  6 +++---
 4 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/promql/bench.go b/promql/bench.go
index a60091e315..b628aa2a89 100644
--- a/promql/bench.go
+++ b/promql/bench.go
@@ -39,7 +39,9 @@ func (b *Benchmark) Run() {
 	b.b.ReportAllocs()
 	b.b.ResetTimer()
 	for i := 0; i < b.b.N; i++ {
-		b.t.RunAsBenchmark(b)
+		if err := b.t.RunAsBenchmark(b); err != nil {
+			b.b.Error(err)
+		}
 		b.iterCount++
 	}
 }
diff --git a/promql/engine.go b/promql/engine.go
index bb566850eb..d9c33d4139 100644
--- a/promql/engine.go
+++ b/promql/engine.go
@@ -481,41 +481,31 @@ func (ng *Engine) populateIterators(s *EvalStmt) error {
 	Inspect(s.Expr, func(node Node) bool {
 		switch n := node.(type) {
 		case *VectorSelector:
-			var iterators []local.SeriesIterator
-			var err error
 			if s.Start.Equal(s.End) {
-				iterators, err = ng.querier.QueryInstant(
+				n.iterators, queryErr = ng.querier.QueryInstant(
 					s.Start.Add(-n.Offset),
 					StalenessDelta,
 					n.LabelMatchers...,
 				)
 			} else {
-				iterators, err = ng.querier.QueryRange(
+				n.iterators, queryErr = ng.querier.QueryRange(
 					s.Start.Add(-n.Offset-StalenessDelta),
 					s.End.Add(-n.Offset),
 					n.LabelMatchers...,
 				)
 			}
-			if err != nil {
-				queryErr = err
+			if queryErr != nil {
 				return false
 			}
-			for _, it := range iterators {
-				n.iterators = append(n.iterators, it)
-			}
 		case *MatrixSelector:
-			iterators, err := ng.querier.QueryRange(
+			n.iterators, queryErr = ng.querier.QueryRange(
 				s.Start.Add(-n.Offset-n.Range),
 				s.End.Add(-n.Offset),
 				n.LabelMatchers...,
 			)
-			if err != nil {
-				queryErr = err
+			if queryErr != nil {
 				return false
 			}
-			for _, it := range iterators {
-				n.iterators = append(n.iterators, it)
-			}
 		}
 		return true
 	})
diff --git a/promql/functions.go b/promql/functions.go
index e4d896e237..9e49dc3600 100644
--- a/promql/functions.go
+++ b/promql/functions.go
@@ -522,7 +522,7 @@ func funcStddevOverTime(ev *evaluator, args Expressions) model.Value {
 		for _, v := range values {
 			sum += v.Value
 			squaredSum += v.Value * v.Value
-			count += 1
+			count++
 		}
 		avg := sum / count
 		return model.SampleValue(math.Sqrt(float64(squaredSum/count - avg*avg)))
@@ -536,7 +536,7 @@ func funcStdvarOverTime(ev *evaluator, args Expressions) model.Value {
 		for _, v := range values {
 			sum += v.Value
 			squaredSum += v.Value * v.Value
-			count += 1
+			count++
 		}
 		avg := sum / count
 		return squaredSum/count - avg*avg
diff --git a/promql/functions_test.go b/promql/functions_test.go
index bbb51df379..41a5d32167 100644
--- a/promql/functions_test.go
+++ b/promql/functions_test.go
@@ -22,7 +22,7 @@ load 5m
     http_requests{path="/foo"}    0+10x8064
 
 eval instant at 4w holt_winters(http_requests[4w], 0.3, 0.3)
-    {path="/foo"} 20160
+    {path="/foo"} 80640
 `
 
 	bench := NewBenchmark(b, input)
@@ -51,7 +51,7 @@ load 1m
     http_requests{path="/foo"}    0+10x1440
 
 eval instant at 1d holt_winters(http_requests[1d], 0.3, 0.3)
-    {path="/foo"} 20160
+    {path="/foo"} 14400
 `
 
 	bench := NewBenchmark(b, input)
@@ -65,7 +65,7 @@ load 1m
     http_requests{path="/foo"}    0+10x1440
 
 eval instant at 1d changes(http_requests[1d])
-    {path="/foo"} 20160
+    {path="/foo"} 1440
 `
 
 	bench := NewBenchmark(b, input)