From 00af0141d57147afe5e660a21410d2f85a2ae1b4 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
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 6c683f461e..5661f36cac 100644
--- a/app/vmselect/promql/eval.go
+++ b/app/vmselect/promql/eval.go
@@ -324,7 +324,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 d9e7e281ae..6668f637ea 100644
--- a/app/vmselect/promql/exec_test.go
+++ b/app/vmselect/promql/exec_test.go
@@ -7955,7 +7955,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 511622f6a7..18ad032db6 100644
--- a/app/vmselect/promql/transform.go
+++ b/app/vmselect/promql/transform.go
@@ -822,11 +822,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 a3ecb7a3e5..cac5d7a10f 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -18,7 +18,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.79.2](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.79.2)