sparsehistogram: Address two TODOs

Signed-off-by: beorn7 <beorn@grafana.com>

model/histogram/float_histogram.go

@@ -470,7 +470,10 @@ type FloatBucket struct {
 	Lower, Upper                   float64
 	LowerInclusive, UpperInclusive bool
 	Count                          float64
-	Index                          int32 // Index within schema. To easily compare buckets that share the same schema.
+
+	// Index within schema. To easily compare buckets that share the same
+	// schema and sign (positive or negative). Irrelevant for the zero bucket.
+	Index int32
 }
 
 // String returns a string representation of a FloatBucket, using the usual
@@ -686,7 +689,7 @@ func (r *allFloatBucketIterator) Next() bool {
 				LowerInclusive: true,
 				UpperInclusive: true,
 				Count:          r.h.ZeroCount,
-				Index:          math.MinInt32, // TODO(codesome): What is the index for this?
+				// Index is irrelevant for the zero bucket.
 			}
 			return true
 		}

model/histogram/float_histogram_test.go

@@ -15,7 +15,6 @@ package histogram
 
 import (
 	"fmt"
-	"math"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -973,7 +972,6 @@ func TestAllFloatBucketIterator(t *testing.T) {
 					LowerInclusive: true,
 					UpperInclusive: true,
 					Count:          c.h.ZeroCount,
-					Index:          math.MinInt32,
 				})
 			}
 			if c.includePos {

promql/engine_test.go

@@ -3002,8 +3002,8 @@ func TestSparseHistogram_Sum_AddOperator(t *testing.T) {
 			ts := int64(i+1) * int64(10*time.Minute/time.Millisecond)
 			app := test.Storage().Appender(context.TODO())
 			for idx, h := range c.histograms {
-				// TODO(codesome): Without h.Copy(), it was working weird. TSDB is keeping reference of last histogram. AppendHistogram should not take pointers!
 				lbls := labels.FromStrings("__name__", seriesName, "idx", fmt.Sprintf("%d", idx))
+				// Since we mutate h later, we need to create a copy here.
 				_, err = app.AppendHistogram(0, lbls, ts, h.Copy())
 			}
 			require.NoError(t, err)

storage/interface.go

@@ -234,7 +234,11 @@ type HistogramAppender interface {
 	// histograms in the same or later transactions. Returned reference
 	// numbers are ephemeral and may be rejected in calls to Append() at any
 	// point. Adding the sample via Append() returns a new reference number.
-	// If the reference is 0 it must not be used for caching.
+	// If the reference is 0, it must not be used for caching.
+	//
+	// For efficiency reasons, the histogram is passed as a
+	// pointer. AppendHistogram won't mutate the histogram, but in turn
+	// depends on the caller to not mutate it either.
 	AppendHistogram(ref SeriesRef, l labels.Labels, t int64, h *histogram.Histogram) (SeriesRef, error)
 }