Change max/min over_time to handle NaNs properly (#4386)

We only want to return a NaN if the NaN is the only value

Signed-off-by: Thomas Jackson <jacksontj.89@gmail.com>

Fixes #4385

promql/functions.go

@@ -391,9 +391,11 @@ func funcCountOverTime(vals []Value, args Expressions, enh *EvalNodeHelper) Vect
 // === max_over_time(Matrix ValueTypeMatrix) Vector ===
 func funcMaxOverTime(vals []Value, args Expressions, enh *EvalNodeHelper) Vector {
 	return aggrOverTime(vals, enh, func(values []Point) float64 {
-		max := math.Inf(-1)
+		max := values[0].V
 		for _, v := range values {
-			max = math.Max(max, v.V)
+			if v.V > max || math.IsNaN(max) {
+				max = v.V
+			}
 		}
 		return max
 	})
@@ -402,9 +404,11 @@ func funcMaxOverTime(vals []Value, args Expressions, enh *EvalNodeHelper) Vector
 // === min_over_time(Matrix ValueTypeMatrix) Vector ===
 func funcMinOverTime(vals []Value, args Expressions, enh *EvalNodeHelper) Vector {
 	return aggrOverTime(vals, enh, func(values []Point) float64 {
-		min := math.Inf(1)
+		min := values[0].V
 		for _, v := range values {
-			min = math.Min(min, v.V)
+			if v.V < min || math.IsNaN(min) {
+				min = v.V
+			}
 		}
 		return min
 	})

promql/testdata/functions.test

@@ -521,4 +521,28 @@ load 5m
   testmetric1{src="a",dst="b"} 0
   testmetric2{src="a",dst="b"} 1
 
-eval_fail instant at 0m changes({__name__=~'testmetric1|testmetric2'}[5m])
+eval_fail instant at 0m changes({__name__=~'testmetric1|testmetric2'}[5m])
+
+# Tests for *_over_time
+clear
+
+load 10s
+	data{type="numbers"} 2 0 3
+	data{type="some_nan"} 2 0 NaN
+	data{type="some_nan2"} 2 NaN 1
+	data{type="some_nan3"} NaN 0 1
+	data{type="only_nan"} NaN NaN NaN
+
+eval instant at 1m min_over_time(data[1m])
+	{type="numbers"} 0
+	{type="some_nan"} 0
+	{type="some_nan2"} 1
+	{type="some_nan3"} 0
+	{type="only_nan"} NaN
+
+eval instant at 1m max_over_time(data[1m])
+	{type="numbers"} 3
+	{type="some_nan"} 2
+	{type="some_nan2"} 2
+	{type="some_nan3"} 1
+	{type="only_nan"} NaN