From 0692d92765a852005a681c18ee66489cb6da6a7d Mon Sep 17 00:00:00 2001 From: discord9 <55937128+discord9@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:59:30 +0800 Subject: [PATCH] fix(promql): keep feature-specific annotations with metric-name changes Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --- src/query/src/promql/planner.rs | 57 +--------------------------- src/query/src/promql/planner/test.rs | 13 ------- 2 files changed, 2 insertions(+), 68 deletions(-) diff --git a/src/query/src/promql/planner.rs b/src/query/src/promql/planner.rs index 64e193718d1..5867301f972 100644 --- a/src/query/src/promql/planner.rs +++ b/src/query/src/promql/planner.rs @@ -1407,25 +1407,6 @@ impl PromPlanner { &right_field_columns, ); - // `+`/`-` over two alternative float/histogram operands records its discarded - // mixed pairs from the float lane's `coalesce` fallbacks instead of a standalone - // annotation filter. `align_binary_field_columns` emits that lane first, named - // after the left operand's float lane. - let arithmetic_float_lane_output = if matches!(op.id(), token::T_ADD | token::T_SUB) - && Self::field_columns_are_alternative_samples( - left_input.schema(), - &left_field_columns, - ) - && Self::field_columns_are_alternative_samples( - right_input.schema(), - &right_field_columns, - ) { - Self::alternative_sample_columns(left_input.schema(), &left_field_columns) - .map(|(float, _)| float.to_string()) - } else { - None - }; - // normal join if left_table_ref == right_table_ref { // rename table references to avoid ambiguity @@ -1460,7 +1441,7 @@ impl PromPlanner { // Computed scalars reach this join path instead of the literal projection paths. // Broadcast them for arithmetic in the same way as literal scalars. let broadcast_scalar = !is_comparison_op; - let (mut field_groups, mut invalid_field_pairs) = Self::align_binary_field_columns( + let (field_groups, invalid_field_pairs) = Self::align_binary_field_columns( left_input.schema(), right_input.schema(), &left_field_columns, @@ -1491,37 +1472,6 @@ impl PromPlanner { .iter() .map(|(output, _)| output.clone()) .collect(); - // `+`/`-` over two alternative float/histogram operands records the discarded - // mixed pairs from the float lane: the valid float expression stays first and - // each fallback records its pair while returning NULL. A standalone always-true - // annotation filter cannot be used for them, because the outer non-null filter - // can be pushed into the same physical filter and short-circuit those UDFs - // before they run. - if self.promql_annotations.is_some() - && let Some(float_output) = arithmetic_float_lane_output.as_deref() - && let Some((_, float_pairs)) = field_groups - .iter_mut() - .find(|(output, _)| output.as_str() == float_output) - { - 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; @@ -1573,10 +1523,7 @@ 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 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. + // while collecting annotations. 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 3f9ebb3586c..19de1098288 100644 --- a/src/query/src/promql/planner/test.rs +++ b/src/query/src/promql/planner/test.rs @@ -7667,22 +7667,9 @@ async fn test_mixed_binary_operator_aligns_both_alternative_inputs() { #[tokio::test] async fn test_mixed_binary_operator_reports_only_dropped_samples() { for (query, expected_rows, expected_infos) in [ - // `+`/`-` report the discarded mixed pairs from the float lane's coalesce fallbacks. ("(lf or on(tag) lh) + on(tag) (rf or on(tag) rh)", 0, 1), - ("(lf or on(tag) lh) - on(tag) (rf or on(tag) rh)", 0, 1), - // Compatible float/float and histogram/histogram pairs must not invent an annotation. ("(lf or on(tag) lh) + on(tag) (lf or on(tag) lh)", 2, 0), - ("(lf or on(tag) lh) - on(tag) (lf or on(tag) lh)", 2, 0), - // An unmatched operand has no dropped pair to report. - ( - "(lf or on(tag) lh) + on(tag) (fallback or on(tag) bad_native)", - 0, - 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();