From 20371118b6333312991833dc832eeb09fc37fa33 Mon Sep 17 00:00:00 2001
From: Neeraj Gartia <80708727+NeerajGartia21@users.noreply.github.com>
Date: Thu, 6 Feb 2025 20:32:03 +0530
Subject: [PATCH] [FIX] PromQL: Ignore histograms in `scalar`, `sort` and
 `sort_desc` functions (#15964)

PromQL: Ignore histograms in scalar, sort and sort_desc functions

---------

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
---
 promql/functions.go                       | 55 +++++++++++++----------
 promql/promqltest/testdata/functions.test | 20 +++++++++
 2 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/promql/functions.go b/promql/functions.go
index 7ac17167f4..1cb8b2af2d 100644
--- a/promql/functions.go
+++ b/promql/functions.go
@@ -486,11 +486,22 @@ func funcDoubleExponentialSmoothing(vals []parser.Value, args parser.Expressions
 	return append(enh.Out, Sample{F: s1}), nil
 }
 
+// filterFloats filters out histogram samples from the vector in-place.
+func filterFloats(v Vector) Vector {
+	floats := v[:0]
+	for _, s := range v {
+		if s.H == nil {
+			floats = append(floats, s)
+		}
+	}
+	return floats
+}
+
 // === sort(node parser.ValueTypeVector) (Vector, Annotations) ===
 func funcSort(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) {
 	// NaN should sort to the bottom, so take descending sort with NaN first and
 	// reverse it.
-	byValueSorter := vectorByReverseValueHeap(vals[0].(Vector))
+	byValueSorter := vectorByReverseValueHeap(filterFloats(vals[0].(Vector)))
 	sort.Sort(sort.Reverse(byValueSorter))
 	return Vector(byValueSorter), nil
 }
@@ -499,7 +510,7 @@ func funcSort(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper)
 func funcSortDesc(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) {
 	// NaN should sort to the bottom, so take ascending sort with NaN first and
 	// reverse it.
-	byValueSorter := vectorByValueHeap(vals[0].(Vector))
+	byValueSorter := vectorByValueHeap(filterFloats(vals[0].(Vector)))
 	sort.Sort(sort.Reverse(byValueSorter))
 	return Vector(byValueSorter), nil
 }
@@ -631,11 +642,27 @@ func funcRound(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper
 
 // === Scalar(node parser.ValueTypeVector) Scalar ===
 func funcScalar(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) {
-	v := vals[0].(Vector)
-	if len(v) != 1 {
+	var (
+		v     = vals[0].(Vector)
+		value float64
+		found bool
+	)
+
+	for _, s := range v {
+		if s.H == nil {
+			if found {
+				// More than one float found, return NaN.
+				return append(enh.Out, Sample{F: math.NaN()}), nil
+			}
+			found = true
+			value = s.F
+		}
+	}
+	// Return the single float if found, otherwise return NaN.
+	if !found {
 		return append(enh.Out, Sample{F: math.NaN()}), nil
 	}
-	return append(enh.Out, Sample{F: v[0].F}), nil
+	return append(enh.Out, Sample{F: value}), nil
 }
 
 func aggrOverTime(vals []parser.Value, enh *EvalNodeHelper, aggrFn func(Series) float64) Vector {
@@ -1896,16 +1923,7 @@ func (s vectorByValueHeap) Len() int {
 }
 
 func (s vectorByValueHeap) Less(i, j int) bool {
-	// We compare histograms based on their sum of observations.
-	// TODO(beorn7): Is that what we want?
 	vi, vj := s[i].F, s[j].F
-	if s[i].H != nil {
-		vi = s[i].H.Sum
-	}
-	if s[j].H != nil {
-		vj = s[j].H.Sum
-	}
-
 	if math.IsNaN(vi) {
 		return true
 	}
@@ -1935,16 +1953,7 @@ func (s vectorByReverseValueHeap) Len() int {
 }
 
 func (s vectorByReverseValueHeap) Less(i, j int) bool {
-	// We compare histograms based on their sum of observations.
-	// TODO(beorn7): Is that what we want?
 	vi, vj := s[i].F, s[j].F
-	if s[i].H != nil {
-		vi = s[i].H.Sum
-	}
-	if s[j].H != nil {
-		vj = s[j].H.Sum
-	}
-
 	if math.IsNaN(vi) {
 		return true
 	}
diff --git a/promql/promqltest/testdata/functions.test b/promql/promqltest/testdata/functions.test
index 23aaa1fe59..49ab6c50ff 100644
--- a/promql/promqltest/testdata/functions.test
+++ b/promql/promqltest/testdata/functions.test
@@ -642,6 +642,7 @@ load 5m
 	http_requests{job="app-server", instance="1", group="production"}	0+60x10
 	http_requests{job="app-server", instance="0", group="canary"}		0+70x10
 	http_requests{job="app-server", instance="1", group="canary"}		0+80x10
+	http_requests{job="app-server", instance="2", group="canary"}		{{schema:0 sum:1 count:1}}x15
 
 eval_ordered instant at 50m sort(http_requests)
 	http_requests{group="production", instance="0", job="api-server"} 100
@@ -1652,3 +1653,22 @@ load 1m
 
 eval range from 0 to 5m step 1m round(mixed_metric)
 	{} _ 1 2 3
+
+# Test scalar() with histograms.
+load 1m
+	metric{type="float", l="x"} 1
+	metric{type="float", l="y"} 2
+	metric{type="histogram", l="x"} {{schema:0 sum:1 count:1}}
+	metric{type="histogram", l="x"} {{schema:0 sum:1 count:1}}
+
+# Two floats in the vector.
+eval instant at 0m scalar(metric)
+	NaN
+
+# No floats in the vector.
+eval instant at 0m scalar({type="histogram"})
+	NaN
+
+# One float in the vector.
+eval instant at 0m scalar({l="x"})
+	1