From 329fed910baa28840b8fb86905bccbcb43f4d40a Mon Sep 17 00:00:00 2001 From: Dennis Zhuang Date: Sat, 17 Oct 2026 23:21:36 +0800 Subject: [PATCH] fix(promql): keep the first of equal classic bucket bounds like Prometheus Prometheus compares bounds with < and >, so +0 and -0 are equal, keep input order, and coalescing keeps the first; ordering -0 first returned the other zero and flipped the sign of an infinite reciprocal. Signed-off-by: Dennis Zhuang --- src/promql/src/histogram_eval.rs | 10 ++++++-- src/promql/src/histogram_eval/tests.rs | 33 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/promql/src/histogram_eval.rs b/src/promql/src/histogram_eval.rs index 5aa9843774e..c6d9a44e8a3 100644 --- a/src/promql/src/histogram_eval.rs +++ b/src/promql/src/histogram_eval.rs @@ -90,8 +90,14 @@ pub fn evaluate_histograms( let mut native_buckets = Vec::new(); let mut values = vec![None; schema.fields.len()]; for mut group in groups.into_values() { - // Pre-sorting by bound keeps the buckets of every point sorted. - group.classic.sort_by(|a, b| a.0.total_cmp(&b.0)); + // Pre-sorting by bound keeps the buckets of every point sorted. Like Go's comparison, + // +0 and -0 are equal and keep input order, and coalescing keeps the first bound. + group + .classic + .sort_by(|a, b| match (a.0.is_nan(), b.0.is_nan()) { + (false, false) => a.0.partial_cmp(&b.0).expect("neither is NaN"), + (nan_a, nan_b) => nan_a.cmp(&nan_b), + }); let ranges = group .classic .iter() diff --git a/src/promql/src/histogram_eval/tests.rs b/src/promql/src/histogram_eval/tests.rs index 8755a81fd04..4cda9c71bbf 100644 --- a/src/promql/src/histogram_eval/tests.rs +++ b/src/promql/src/histogram_eval/tests.rs @@ -368,3 +368,36 @@ fn close(a: f64, b: f64, epsilon: f64) -> bool { }; equal || (a.abs() < 1e-10 && b.abs() < 1e-10) } + +/// Bounds +0 and -0 compare equal and keep input order, so coalescing keeps the first one: +/// `1 / histogram_quantile(0, sort_desc(h))` over `le` "+Inf", "0", "-0" is +Inf. +#[test] +fn classic_bounds_keep_the_first_signed_zero() { + let grid = Arc::new( + datatypes::series::time::EvalGrid::try_new( + datatypes::series::time::GridSpec::try_new(0, 1_000, 1).unwrap(), + ) + .unwrap(), + ); + let schema = Arc::new(SeriesSchema { + fields: vec![FieldSpec { + name: Arc::from("value"), + }] + .into(), + eval_grid: Some(grid), + }); + let buckets: Vec<_> = [("+Inf", 3.0), ("0", 2.0), ("-0", 1.0)] + .iter() + .map(|(le, count)| { + let labels = Labels::from([ + ("__name__".to_string(), "h".to_string()), + ("le".to_string(), le.to_string()), + ]); + let points = BTreeMap::from([(0, f64::to_bits(*count))]); + mixed_series(&schema, &labels, &points, &BTreeMap::new()).unwrap() + }) + .collect(); + let result = evaluate_histograms(HistogramFunction::Quantile(0.0), &schema, &buckets).unwrap(); + let points = points_of(&result[0]); + assert_eq!(1.0 / f64::from_bits(points[&0]), f64::INFINITY); +}