From bac1f28cad24be969a5f3eb8f5c0aa0e833aba42 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Wed, 30 Dec 2015 14:06:51 +0000 Subject: [PATCH] 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 | 11 ++++++++--- promql/testdata/functions.test | 36 ++++++++++++++++++++++++++++++++++ promql/testdata/legacy.test | 32 ------------------------------ 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/promql/functions.go b/promql/functions.go index 16d8f24db..ebf44a06c 100644 --- a/promql/functions.go +++ b/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])) - sort.Sort(byValueSorter) + // NaN should sort to the bottom, so take descending sort with NaN first and + // 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) } diff --git a/promql/testdata/functions.test b/promql/testdata/functions.test index d1c262e11..d2bf59e02 100644 --- a/promql/testdata/functions.test +++ b/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 diff --git a/promql/testdata/legacy.test b/promql/testdata/legacy.test index b29df70a9..6e47e3225 100644 --- a/promql/testdata/legacy.test +++ b/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"}