app/vmselect/promql: follow-up for 930f1ee153

Document the change at docs/CHANGELOG.md
Apply it to histogram_quantile() in the same way as to histogram_share()

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225

app/vmselect/promql/exec_test.go

@@ -3788,6 +3788,27 @@ func TestExecSuccess(t *testing.T) {
 		resultExpected := []netstorage.Result{r}
 		f(q, resultExpected)
 	})
+	t.Run(`histogram_quantile(duplicate-le)`, func(t *testing.T) {
+		// See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225
+		t.Parallel()
+		q := `round(sort(histogram_quantile(0.6,
+			label_set(90, "foo", "bar", "le", "5")
+			or label_set(100, "foo", "bar", "le", "5.0")
+			or label_set(200, "foo", "bar", "le", "6.0")
+			or label_set(300, "foo", "bar", "le", "+Inf")
+		)), 0.1)`
+		r1 := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{4.7, 4.7, 4.7, 4.7, 4.7, 4.7},
+			Timestamps: timestampsExpected,
+		}
+		r1.MetricName.Tags = []storage.Tag{{
+			Key:   []byte("foo"),
+			Value: []byte("bar"),
+		}}
+		resultExpected := []netstorage.Result{r1}
+		f(q, resultExpected)
+	})
 	t.Run(`histogram_quantile(valid)`, func(t *testing.T) {
 		t.Parallel()
 		q := `sort(histogram_quantile(0.6,

app/vmselect/promql/transform.go

@@ -659,6 +659,7 @@ func transformHistogramShare(tfa *transformFuncArg) ([]*timeseries, error) {
 		sort.Slice(xss, func(i, j int) bool {
 			return xss[i].le < xss[j].le
 		})
+		xss = mergeSameLE(xss)
 		dst := xss[0].ts
 		var tsLower, tsUpper *timeseries
 		if len(boundsLabel) > 0 {
@@ -936,10 +937,10 @@ func transformHistogramQuantile(tfa *transformFuncArg) ([]*timeseries, error) {
 	}
 	rvs := make([]*timeseries, 0, len(m))
 	for _, xss := range m {
-		xss = mergeSameLE(xss)
 		sort.Slice(xss, func(i, j int) bool {
 			return xss[i].le < xss[j].le
 		})
+		xss = mergeSameLE(xss)
 		dst := xss[0].ts
 		var tsLower, tsUpper *timeseries
 		if len(boundsLabel) > 0 {
@@ -1007,6 +1008,7 @@ func fixBrokenBuckets(i int, xss []leTimeseries) {
 	if len(xss) < 2 {
 		return
 	}
+	// Fill NaN in upper buckets with the first non-NaN value found in lower buckets.
 	for j := len(xss) - 1; j >= 0; j-- {
 		v := xss[j].ts.Values[i]
 		if !math.IsNaN(v) {
@@ -1018,6 +1020,8 @@ func fixBrokenBuckets(i int, xss []leTimeseries) {
 			break
 		}
 	}
+	// Substitute lower bucket values with upper values if the lower values are NaN
+	// or are bigger than the upper bucket values.
 	vNext := xss[len(xss)-1].ts.Values[i]
 	for j := len(xss) - 2; j >= 0; j-- {
 		v := xss[j].ts.Values[i]
@@ -1030,34 +1034,23 @@ func fixBrokenBuckets(i int, xss []leTimeseries) {
 }
 
 func mergeSameLE(xss []leTimeseries) []leTimeseries {
-	hit := false
-	m := make(map[float64]leTimeseries)
-	for _, xs := range xss {
-		i, ok := m[xs.le]
-		if ok {
-			ts := &timeseries{}
-			ts.CopyFromShallowTimestamps(i.ts)
-			for k := range ts.Values {
-				ts.Values[k] += xs.ts.Values[k]
-			}
-			m[xs.le] = leTimeseries{
-				le: xs.le,
-				ts: ts,
-			}
-			hit = true
+	// Merge buckets with identical le values.
+	// See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225
+	xsDst := xss[0]
+	dst := xss[:1]
+	for j := 1; j < len(xss); j++ {
+		xs := xss[j]
+		if xs.le != xsDst.le {
+			dst = append(dst, xs)
+			xsDst = xs
 			continue
 		}
-		m[xs.le] = xs
+		dstValues := xsDst.ts.Values
+		for k, v := range xs.ts.Values {
+			dstValues[k] += v
+		}
 	}
-	if (!hit) {
-		return xss
-	}
-
-	r := make([]leTimeseries, 0, len(m))
-	for _, v := range m {
-		r = append(r, v)
-	}
-	return r
+	return dst
 }
 
 func transformHour(t time.Time) int {

docs/CHANGELOG.md

@@ -19,6 +19,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 
 * SECURITY: update Go builder to v1.19.3. This fixes [CVE-2022 security issue](https://github.com/golang/go/issues/56328). See [the changelog](https://github.com/golang/go/issues?q=milestone%3AGo1.19.3+label%3ACherryPickApproved).
 
+* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly merge buckets with identical `le` values, but with different string representation of these values when calculating [histogram_quantile](https://docs.victoriametrics.com/MetricsQL.html#histogram_quantile) and [histogram_share](https://docs.victoriametrics.com/MetricsQL.html#histogram_share). For example, `http_request_duration_seconds_bucket{le="5"}` and `http_requests_duration_seconds_bucket{le="5.0"}`. Such buckets may be returned from distinct targets. Thanks to @647-coder for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225).
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): change severity level for log messages about failed attempts for sending data to remote storage from `error` to `warn`. The message for about all failed send attempts remains at `error` severity level.
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly escape string passed to `quotesEscape` [template function](https://docs.victoriametrics.com/vmalert.html#template-functions), so it can be safely embedded into JSON string. This makes obsolete the `crlfEscape` function. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3139) and [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/890) issue.
 * BUGFIX: `vmselect`: expose missing metric `vm_cache_size_max_bytes{type="promql/rollupResult"}` . This metric is used for monitoring rollup cache usage with the query `vm_cache_size_bytes{type="promql/rollupResult"} / vm_cache_size_max_bytes{type="promql/rollupResult"}` in the same way as this is done for other cache types.