Similar to topk/bottomk, have sort/sort_desc put NaN at end.

This makes topk and bottomk consistent with the sorting functions,
as per #1271.

promql/functions.go

@@ -170,16 +170,19 @@ func funcIrate(ev *evaluator, args Expressions) model.Value {
 
 // === sort(node model.ValVector) Vector ===
 func funcSort(ev *evaluator, args Expressions) model.Value {
-	byValueSorter := vectorByValueHeap(ev.evalVector(args[0]))
+	// NaN should sort to the bottom, so take descending sort with NaN first and
-	sort.Sort(byValueSorter)
+	// reverse it.
+	byValueSorter := vectorByReverseValueHeap(ev.evalVector(args[0]))
+	sort.Sort(sort.Reverse(byValueSorter))
 	return vector(byValueSorter)
 }
 
 // === sortDesc(node model.ValVector) Vector ===
 func funcSortDesc(ev *evaluator, args Expressions) model.Value {
+	// NaN should sort to the bottom, so take ascending sort with NaN first and
+	// reverse it.
 	byValueSorter := vectorByValueHeap(ev.evalVector(args[0]))
 	sort.Sort(sort.Reverse(byValueSorter))
-
 	return vector(byValueSorter)
 }
 
@@ -201,6 +204,7 @@ func funcTopk(ev *evaluator, args Expressions) model.Value {
 			heap.Push(&topk, el)
 		}
 	}
+	// The heap keeps the lowest value on top, so reverse it.
 	sort.Sort(sort.Reverse(topk))
 	return vector(topk)
 }
@@ -223,6 +227,7 @@ func funcBottomk(ev *evaluator, args Expressions) model.Value {
 			heap.Push(&bottomk, el)
 		}
 	}
+	// The heap keeps the highest value on top, so reverse it.
 	sort.Sort(sort.Reverse(bottomk))
 	return vector(bottomk)
 }

promql/testdata/functions.test

@@ -236,3 +236,39 @@ eval_ordered instant at 50m bottomk(3, http_requests{job="api-server",group="pro
 	http_requests{job="api-server", instance="0", group="production"}	100
 	http_requests{job="api-server", instance="1", group="production"}	200
 	http_requests{job="api-server", instance="2", group="production"}	NaN
+
+
+# Tests for sort/sort_desc.
+clear
+load 5m
+	http_requests{job="api-server", instance="0", group="production"}	0+10x10
+	http_requests{job="api-server", instance="1", group="production"}	0+20x10
+	http_requests{job="api-server", instance="0", group="canary"}		0+30x10
+	http_requests{job="api-server", instance="1", group="canary"}		0+40x10
+	http_requests{job="api-server", instance="2", group="canary"}		NaN NaN NaN NaN NaN NaN NaN NaN NaN NaN
+	http_requests{job="app-server", instance="0", group="production"}	0+50x10
+	http_requests{job="app-server", instance="1", group="production"}	0+60x10
+	http_requests{job="app-server", instance="0", group="canary"}		0+70x10
+	http_requests{job="app-server", instance="1", group="canary"}		0+80x10
+
+eval_ordered instant at 50m sort(http_requests)
+	http_requests{group="production", instance="0", job="api-server"} 100
+	http_requests{group="production", instance="1", job="api-server"} 200
+	http_requests{group="canary", instance="0", job="api-server"} 300
+	http_requests{group="canary", instance="1", job="api-server"} 400
+	http_requests{group="production", instance="0", job="app-server"} 500
+	http_requests{group="production", instance="1", job="app-server"} 600
+	http_requests{group="canary", instance="0", job="app-server"} 700
+	http_requests{group="canary", instance="1", job="app-server"} 800
+	http_requests{group="canary", instance="2", job="api-server"} NaN
+
+eval_ordered instant at 50m sort_desc(http_requests)
+	http_requests{group="canary", instance="1", job="app-server"} 800
+	http_requests{group="canary", instance="0", job="app-server"} 700
+	http_requests{group="production", instance="1", job="app-server"} 600
+	http_requests{group="production", instance="0", job="app-server"} 500
+	http_requests{group="canary", instance="1", job="api-server"} 400
+	http_requests{group="canary", instance="0", job="api-server"} 300
+	http_requests{group="production", instance="1", job="api-server"} 200
+	http_requests{group="production", instance="0", job="api-server"} 100
+	http_requests{group="canary", instance="2", job="api-server"} NaN

promql/testdata/legacy.test

@@ -133,38 +133,6 @@ eval instant at 50m rate(http_requests[25m]) * 25 * 60
 	{group="production", instance="1", job="api-server"} 100 
 	{group="production", instance="1", job="app-server"} 300 
 
-eval_ordered instant at 50m sort(http_requests)
-	http_requests{group="production", instance="0", job="api-server"} 100 
-	http_requests{group="production", instance="1", job="api-server"} 200 
-	http_requests{group="canary", instance="0", job="api-server"} 300 
-	http_requests{group="canary", instance="1", job="api-server"} 400 
-	http_requests{group="production", instance="0", job="app-server"} 500 
-	http_requests{group="production", instance="1", job="app-server"} 600 
-	http_requests{group="canary", instance="0", job="app-server"} 700 
-	http_requests{group="canary", instance="1", job="app-server"} 800 
-
-eval_ordered instant at 50m sort(0 / round(http_requests, 400) + http_requests)
-	{group="production", instance="0", job="api-server"} NaN 
-	{group="production", instance="1", job="api-server"} 200 
-	{group="canary", instance="0", job="api-server"} 300 
-	{group="canary", instance="1", job="api-server"} 400 
-	{group="production", instance="0", job="app-server"} 500 
-	{group="production", instance="1", job="app-server"} 600 
-	{group="canary", instance="0", job="app-server"} 700 
-	{group="canary", instance="1", job="app-server"} 800 
-
-
-eval_ordered instant at 50m sort_desc(http_requests)
-	http_requests{group="canary", instance="1", job="app-server"} 800 
-	http_requests{group="canary", instance="0", job="app-server"} 700 
-	http_requests{group="production", instance="1", job="app-server"} 600 
-	http_requests{group="production", instance="0", job="app-server"} 500 
-	http_requests{group="canary", instance="1", job="api-server"} 400 
-	http_requests{group="canary", instance="0", job="api-server"} 300 
-	http_requests{group="production", instance="1", job="api-server"} 200 
-	http_requests{group="production", instance="0", job="api-server"} 100 
-
-
 
 # Single-letter label names and values.
 eval instant at 50m x{y="testvalue"}