From cb18653c9fa6ef3366729ee36a3d221d9e158df4 Mon Sep 17 00:00:00 2001 From: discord9 <55937128+discord9@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:38:50 +0800 Subject: [PATCH] fix(promql): report dropped modulo histogram samples Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --- src/query/src/promql/planner.rs | 22 ++++++++++++++++++++-- src/query/src/promql/planner/test.rs | 3 +++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/query/src/promql/planner.rs b/src/query/src/promql/planner.rs index 8984ce9c205..32320144c02 100644 --- a/src/query/src/promql/planner.rs +++ b/src/query/src/promql/planner.rs @@ -1620,6 +1620,23 @@ impl PromPlanner { { float_pairs.extend(std::mem::take(&mut invalid_field_pairs)); } + // `%` over a mixed float/histogram vector and a native-histogram vector supports + // none of its pairs, so `align_binary_field_columns` falls back to a single lane + // that holds nothing but the discarded pairs. Record the dropped samples from + // that lane instead of a standalone annotation filter: `%` is not a comparison, + // so each lane expression is a `Float64(NULL)` drop UDF, and the outer non-null + // filter still discards those rows. + else if op.id() == token::T_MOD + && self.promql_annotations.is_some() + && !invalid_field_pairs.is_empty() + && field_groups.len() == 1 + && field_groups[0] + .1 + .iter() + .all(|pair| invalid_field_pairs.contains(pair)) + { + field_groups[0].1 = std::mem::take(&mut invalid_field_pairs); + } let mut field_groups = field_groups.into_iter(); // `vector()` uses EmptyMetric and keeps GreptimeDB's timestamp broadcast. let has_empty_metric_operand = left_is_empty_metric || right_is_empty_metric; @@ -1685,8 +1702,9 @@ impl PromPlanner { let promql_annotations = self.promql_annotations.clone(); // These predicates always pass; they only evaluate otherwise-discarded pairs // while collecting annotations. `+`/`-` over two alternative float/histogram - // operands already consumed its mixed pairs into the float lane above, where - // the physical plan cannot short-circuit them away. + // operands and `%` over a mixed vector and a native-histogram vector already + // consumed their mixed pairs into their only output lane above, where the + // physical plan cannot short-circuit them away. let invalid_pair_predicates = invalid_field_pairs .into_iter() .filter(|_| promql_annotations.is_some()) diff --git a/src/query/src/promql/planner/test.rs b/src/query/src/promql/planner/test.rs index 4f845450d6d..55964305689 100644 --- a/src/query/src/promql/planner/test.rs +++ b/src/query/src/promql/planner/test.rs @@ -8267,6 +8267,9 @@ async fn test_mixed_binary_operator_reports_only_dropped_samples() { 0, ), ("(lf or on(tag) lh) % on(tag) lh", 0, 1), + // A non-bool comparison drops its unsupported pairs through the standalone annotation + // filter, not through the lane, so the dropped samples are still reported. + ("(lf or on(tag) lh) > on(tag) lh", 0, 1), ] { let state = build_query_engine_state(); let annotations = PromqlAnnotationCollector::default();