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

@@ -3789,6 +3789,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

@@ -665,6 +665,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 {
@@ -942,10 +943,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 {
@@ -1013,6 +1014,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) {
@@ -1024,6 +1026,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]
@@ -1036,34 +1040,23 @@ func fixBrokenBuckets(i int, xss []leTimeseries) {
 }
 
 func mergeSameLE(xss []leTimeseries) []leTimeseries {
-	hit := false
+	// Merge buckets with identical le values.
-	m := make(map[float64]leTimeseries)
+	// See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225
-	for _, xs := range xss {
+	xsDst := xss[0]
-		i, ok := m[xs.le]
+	dst := xss[:1]
-		if ok {
+	for j := 1; j < len(xss); j++ {
-			ts := &timeseries{}
+		xs := xss[j]
-			ts.CopyFromShallowTimestamps(i.ts)
+		if xs.le != xsDst.le {
-			for k := range ts.Values {
+			dst = append(dst, xs)
-				ts.Values[k] += xs.ts.Values[k]
+			xsDst = xs
-			}
-			m[xs.le] = leTimeseries{
-				le: xs.le,
-				ts: ts,
-			}
-			hit = true
 			continue
 		}
-		m[xs.le] = xs
+		dstValues := xsDst.ts.Values
+		for k, v := range xs.ts.Values {
+			dstValues[k] += v
+		}
 	}
-	if (!hit) {
+	return dst
-		return xss
-	}
-
-	r := make([]leTimeseries, 0, len(m))
-	for _, v := range m {
-		r = append(r, v)
-	}
-	return r
 }
 
 func transformHour(t time.Time) int {

docs/CHANGELOG.md

@@ -44,6 +44,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): fix integration with Grafana via `-vmalert.proxyURL`, which has been broken in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820). See [this issue](https://github.com/VictoriaMetrics/helm-charts/issues/391).
 * BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): set default region to `us-east-1` if `AWS_REGION` environment variable isn't set. The issue was introduced in [vmbackup v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820). See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3224).
 * BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager.html): fix deletion of old backups at [Azure blob storage](https://azure.microsoft.com/en-us/products/storage/blobs/).
+* 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).
 
 ## [v1.82.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.82.0)