add more unit test coverage for querying native histograms (#6031)

Signed-off-by: Ben Ye <[email protected]>

Author: yeya24

pkg/ingester/client/compat.go

@@ -113,25 +113,6 @@ func ToQueryResponse(matrix model.Matrix) *QueryResponse {
 	return resp
 }
 
-// FromQueryResponse unpacks a QueryResponse proto.
-func FromQueryResponse(resp *QueryResponse) model.Matrix {
-	m := make(model.Matrix, 0, len(resp.Timeseries))
-	for _, ts := range resp.Timeseries {
-		var ss model.SampleStream
-		ss.Metric = cortexpb.FromLabelAdaptersToMetric(ts.Labels)
-		ss.Values = make([]model.SamplePair, 0, len(ts.Samples))
-		for _, s := range ts.Samples {
-			ss.Values = append(ss.Values, model.SamplePair{
-				Value:     model.SampleValue(s.Value),
-				Timestamp: model.Time(s.TimestampMs),
-			})
-		}
-		m = append(m, &ss)
-	}
-
-	return m
-}
-
 // ToMetricsForLabelMatchersRequest builds a MetricsForLabelMatchersRequest proto
 func ToMetricsForLabelMatchersRequest(from, to model.Time, matchers []*labels.Matcher) (*MetricsForLabelMatchersRequest, error) {
 	ms, err := toLabelMatchers(matchers)

pkg/ingester/client/compat_test.go

@@ -1,9 +1,7 @@
 package client
 
 import (
-	"fmt"
 	"reflect"
-	"sort"
 	"strconv"
 	"testing"
 
@@ -75,36 +73,6 @@ func matchersEqual(expected, actual []*labels.Matcher) bool {
 	return true
 }
 
-func buildTestMatrix(numSeries int, samplesPerSeries int, offset int) model.Matrix {
-	m := make(model.Matrix, 0, numSeries)
-	for i := 0; i < numSeries; i++ {
-		ss := model.SampleStream{
-			Metric: model.Metric{
-				model.MetricNameLabel: model.LabelValue(fmt.Sprintf("testmetric_%d", i)),
-				model.JobLabel:        "testjob",
-			},
-			Values: make([]model.SamplePair, 0, samplesPerSeries),
-		}
-		for j := 0; j < samplesPerSeries; j++ {
-			ss.Values = append(ss.Values, model.SamplePair{
-				Timestamp: model.Time(i + j + offset),
-				Value:     model.SampleValue(i + j + offset),
-			})
-		}
-		m = append(m, &ss)
-	}
-	sort.Sort(m)
-	return m
-}
-
-func TestQueryResponse(t *testing.T) {
-	want := buildTestMatrix(10, 10, 10)
-	have := FromQueryResponse(ToQueryResponse(want))
-	if !reflect.DeepEqual(have, want) {
-		t.Fatalf("Bad FromQueryResponse(ToQueryResponse) round trip")
-	}
-}
-
 // The main usecase for `LabelsToKeyString` is to generate hashKeys
 // for maps. We are benchmarking that here.
 func BenchmarkSeriesMap(b *testing.B) {

pkg/querier/batch/batch.go

@@ -162,6 +162,11 @@ func (a *iteratorAdapter) AtHistogram(h *histogram.Histogram) (int64, *histogram
 
 // AtFloatHistogram implements chunkenc.Iterator.
 func (a *iteratorAdapter) AtFloatHistogram(h *histogram.FloatHistogram) (int64, *histogram.FloatHistogram) {
+	// PromQL engine always selects float histogram in its implementation so might call AtFloatHistogram
+	// even if it is a histogram. https://github.com/prometheus/prometheus/blob/v2.53.0/promql/engine.go#L2276
+	if a.curr.ValType == chunkenc.ValHistogram {
+		return a.curr.Timestamps[a.curr.Index], a.curr.Histograms[a.curr.Index].ToFloat(h)
+	}
 	return a.curr.Timestamps[a.curr.Index], a.curr.FloatHistograms[a.curr.Index]
 }
 

pkg/querier/batch/batch_test.go

@@ -147,6 +147,11 @@ func TestSeekCorrectlyDealWithSinglePointChunks(t *testing.T) {
 			actual, val := sut.AtHistogram(nil)
 			require.Equal(t, histograms[0], val)
 			require.Equal(t, int64(1*time.Second/time.Millisecond), actual)
+
+			// Histogram chunk should support querying float histograms since it is what Query Engine does.
+			actualT, fh := sut.AtFloatHistogram(nil)
+			require.Equal(t, histograms[0].ToFloat(nil), fh)
+			require.Equal(t, int64(1*time.Second/time.Millisecond), actualT)
 		case promchunk.PrometheusFloatHistogramChunk:
 			actual, val := sut.AtFloatHistogram(nil)
 			require.Equal(t, histograms[0].ToFloat(nil), val)