From ad11b8d83d910068b37bfbf96b212f4ead12806d Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Sat, 27 Aug 2022 01:35:46 +0300 Subject: [PATCH] app/vmselect/promql: follow-up after 2d71b4859cb9dc26b9fc2057f7960715b3929857 - Use getScalar() function for obtaining the expected scalar from phi arg - Reduce the error message returned to the user when incorrect phi is passed to histogram_quantiles - Improve the description of this bugfix in the docs/CHANGELOG.md Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3026 --- app/vmselect/promql/eval.go | 4 +++- app/vmselect/promql/exec_test.go | 2 +- app/vmselect/promql/transform.go | 9 +++++---- docs/CHANGELOG.md | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/vmselect/promql/eval.go b/app/vmselect/promql/eval.go index 63f45cd55..d59770538 100644 --- a/app/vmselect/promql/eval.go +++ b/app/vmselect/promql/eval.go @@ -312,7 +312,9 @@ func evalTransformFunc(qt *querytracer.Tracer, ec *EvalConfig, fe *metricsql.Fun } rv, err := tf(tfa) if err != nil { - return nil, fmt.Errorf(`cannot evaluate %q: %w`, fe.AppendString(nil), err) + return nil, &UserReadableError{ + Err: fmt.Errorf(`cannot evaluate %q: %w`, fe.AppendString(nil), err), + } } return rv, nil } diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index 61688b82f..171aae0dc 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -7932,7 +7932,7 @@ func TestExecError(t *testing.T) { f(`limit_offet(1, (alias(1,"foo"),alias(2,"bar")), 10)`) f(`round(1, 1 or label_set(2, "xx", "foo"))`) f(`histogram_quantile(1 or label_set(2, "xx", "foo"), 1)`) - f(`histogram_quantiles("le", 1 or label_set(2, "xx", "foo"))`) + f(`histogram_quantiles("foo", 1 or label_set(2, "xxx", "foo"), 2)`) f(`label_set(1, 2, 3)`) f(`label_set(1, "foo", (label_set(1, "foo", bar") or label_set(2, "xxx", "yy")))`) f(`label_set(1, "foo", 3)`) diff --git a/app/vmselect/promql/transform.go b/app/vmselect/promql/transform.go index 52c72198b..b2696f775 100644 --- a/app/vmselect/promql/transform.go +++ b/app/vmselect/promql/transform.go @@ -825,11 +825,12 @@ func transformHistogramQuantiles(tfa *transformFuncArg) ([]*timeseries, error) { tssOrig := args[len(args)-1] // Calculate quantile individually per each phi. var rvs []*timeseries - for _, phiArg := range phiArgs { - if len(phiArg) == 0 || (phiArg[0].Values[0] < 0 || phiArg[0].Values[0] > 1) { - return nil, fmt.Errorf("got unexpected phi arg. it should containes only numbers from range [0..1]") + for i, phiArg := range phiArgs { + phis, err := getScalar(phiArg, i) + if err != nil { + return nil, fmt.Errorf("cannot parse phi: %w", err) } - phiStr := fmt.Sprintf("%g", phiArg[0].Values[0]) + phiStr := fmt.Sprintf("%g", phis[0]) tss := copyTimeseries(tssOrig) tfaTmp := &transformFuncArg{ ec: tfa.ec, diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f0b786a3d..651354e69 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -44,7 +44,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: prevent from excess CPU usage when the storage enters [read-only mode](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#readonly-mode). * BUGFIX: improve performance for requests to [/api/v1/labels](https://docs.victoriametrics.com/url-examples.html#apiv1labels) and [/api/v1/label/.../values](https://docs.victoriametrics.com/url-examples.html#apiv1labelvalues) when the filter in the `match[]` query arg matches small number of time series. The performance for this case has been reduced in [v1.78.0](https://docs.victoriametrics.com/CHANGELOG.html#v1780). See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2978) and [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1533) issues. * BUGFIX: increase the default limit on the number of concurrent merges for small parts from 8 to 16. This should help resolving potential issues with heavy data ingestion. See [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/2673#issuecomment-1218185978) from @lukepalmer . -* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): fix panic when user defines wrong param in `histogram_quantiles` function. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3026). +* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): fix panic when incorrect arg is passed as `phi` into [histogram_quantiles](https://docs.victoriametrics.com/MetricsQL.html#histogram_quantiles) function. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3026). ## [v1.80.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.80.0)