From c901a6472f82c8d4dd1ab4821bcb7418a6050f85 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Tue, 30 Jul 2019 22:04:16 +0300 Subject: [PATCH] app/vmselect/promql: return NaN values if invalid bucket counts are passed to histogram_quantile Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/136 --- app/vmselect/promql/exec_test.go | 20 ++++++++++++++++++++ app/vmselect/promql/transform.go | 12 ++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index 9af546031b..02a1e000d7 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -2168,6 +2168,26 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r1, r2} f(q, resultExpected) }) + t.Run(`histogram_quantile(negative-bucket-count)`, func(t *testing.T) { + t.Parallel() + q := `sort(histogram_quantile(0.6, + label_set(90, "foo", "bar", "le", "10") + or label_set(-100, "foo", "bar", "le", "30") + or label_set(300, "foo", "bar", "le", "+Inf") + ))` + resultExpected := []netstorage.Result{} + f(q, resultExpected) + }) + t.Run(`histogram_quantile(nan-bucket-count)`, func(t *testing.T) { + t.Parallel() + q := `sort(histogram_quantile(0.6, + label_set(90, "foo", "bar", "le", "10") + or label_set(NaN, "foo", "bar", "le", "30") + or label_set(300, "foo", "bar", "le", "+Inf") + ))` + resultExpected := []netstorage.Result{} + f(q, resultExpected) + }) t.Run(`median_over_time()`, func(t *testing.T) { t.Parallel() q := `median_over_time({})` diff --git a/app/vmselect/promql/transform.go b/app/vmselect/promql/transform.go index bc04a9f89b..8e46fd7813 100644 --- a/app/vmselect/promql/transform.go +++ b/app/vmselect/promql/transform.go @@ -331,13 +331,17 @@ func transformHistogramQuantile(tfa *transformFuncArg) ([]*timeseries, error) { return inf } vReq := xss[len(xss)-1].ts.Values[i] * phi + if math.IsNaN(vReq) || vReq < 0 { + // Broken bucket. + return nan + } for _, xs := range xss { v := xs.ts.Values[i] - le := xs.le - if v <= vPrev { - v = vPrev - le = lePrev + if math.IsNaN(v) || v < 0 { + // Broken bucket. + return nan } + le := xs.le if v < vReq { vPrev = v lePrev = le