Skip to content

Commit c1becfb

Browse files
zzylolclaude
andcommitted
test: pin whole-query forwarding; name the query when binding fails
The non-mixed ratio test now requires forwarding with no maintained state, a binding failure names its query in the candidate error, and the metric name follows the left side of and/unless. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 4f3c436 commit c1becfb

2 files changed

Lines changed: 45 additions & 16 deletions

File tree

‎control_plane/src/physical/compiler.rs‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2427,7 +2427,10 @@ impl DeploymentPlanCompiler {
24272427
query_node_bindings.retain(|(index, _), _| *index != query_index);
24282428
Ok(exact_query_entry(query, canonical.clone(), instant))
24292429
}
2430-
other => other,
2430+
other => other.map_err(|error| CompileError::Query {
2431+
query_id: query.query_id.clone(),
2432+
reason: error.to_string(),
2433+
}),
24312434
}
24322435
}?;
24332436
if frontend == QueryFrontend::MetricsQl {
@@ -5135,18 +5138,11 @@ pub(crate) mod tests {
51355138
.compile_promql(workload, environment(10_000))
51365139
.unwrap();
51375140
let entry = plan.query_plan.lookup(query).unwrap();
5138-
if entry.materialization_bindings().is_empty() {
5139-
assert!(matches!(
5140-
&entry.nodes[&entry.root],
5141-
crate::query_plan::QueryPlanNode::ExactFallback { .. }
5142-
));
5143-
assert!(plan.precompute_plan.materializations.is_empty());
5144-
} else {
5145-
assert!(matches!(
5146-
&entry.nodes[&entry.root],
5147-
crate::query_plan::QueryPlanNode::PhysicalFragment { .. }
5148-
));
5149-
}
5141+
assert!(matches!(
5142+
&entry.nodes[&entry.root],
5143+
crate::query_plan::QueryPlanNode::ExactFallback { .. }
5144+
));
5145+
assert!(plan.precompute_plan.materializations.is_empty());
51505146
}
51515147

51525148
// avg_over_time divides two per-series readouts. Planner does not yet

‎control_plane/src/query_plan.rs‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -680,10 +680,12 @@ where
680680

681681
/// Does PromQL keep `__name__` on this query's result series? Only a series
682682
/// selector keeps it, through ordering, selection, filtering, relabeling,
683-
/// subqueries and `first_`/`last_over_time`; other functions, aggregations and
684-
/// arithmetic drop it.
683+
/// subqueries, `first_`/`last_over_time` and the left side of `and`/`unless`;
684+
/// other functions, aggregations and arithmetic drop it. A comparison keeps it
685+
/// unless it has `bool`, which the IR does not record; Planner compiles no
686+
/// comparison, so the rule treats them as dropping it.
685687
pub fn result_keeps_metric_name(expr: &planner_types::pre_asap::QueryExpr) -> bool {
686-
use planner_types::pre_asap::{AggIntent, QueryExpr};
688+
use planner_types::pre_asap::{AggIntent, BinaryOpKind, PromQLVectorSetOpKind, QueryExpr};
687689
match expr {
688690
QueryExpr::Scan { .. } => true,
689691
QueryExpr::TimeRange { child, .. }
@@ -694,6 +696,11 @@ pub fn result_keeps_metric_name(expr: &planner_types::pre_asap::QueryExpr) -> bo
694696
| QueryExpr::PromqlSeriesSample { child, .. }
695697
| QueryExpr::PromqlRelabel { child, .. }
696698
| QueryExpr::PromqlSubquery { child, .. } => result_keeps_metric_name(child),
699+
QueryExpr::BinaryOp {
700+
op: BinaryOpKind::Set(PromQLVectorSetOpKind::And | PromQLVectorSetOpKind::Unless),
701+
lhs,
702+
..
703+
} => result_keeps_metric_name(lhs),
697704
QueryExpr::Aggregate {
698705
child, measures, ..
699706
} if matches!(
@@ -1213,6 +1220,32 @@ mod tests {
12131220
compile_query_computation(&temporal).unwrap();
12141221
}
12151222

1223+
// PromQL keeps `__name__` only on selections of a series selector.
1224+
#[test]
1225+
fn metric_name_follows_promql_result_rules() {
1226+
for (query, keeps) in [
1227+
("m", true),
1228+
("m offset 5m", true),
1229+
("sort(m)", true),
1230+
("topk(2, m)", true),
1231+
("last_over_time(m[5m])", true),
1232+
("m and n", true),
1233+
("rate(m[5m])", false),
1234+
("quantile_over_time(0.5, m[5m])", false),
1235+
("m * 2", false),
1236+
("sum by (job) (m)", false),
1237+
("topk(2, rate(m[5m]))", false),
1238+
("max_over_time(rate(m[1m])[5m:1m])", false),
1239+
] {
1240+
let expr = crate::query_parser::parse_query_expr_canonical(
1241+
query,
1242+
planner_types::types::AccuracyTarget::Exact,
1243+
)
1244+
.unwrap();
1245+
assert_eq!(result_keeps_metric_name(&expr), keeps, "{query}");
1246+
}
1247+
}
1248+
12161249
#[test]
12171250
fn canonical_identity_ignores_formatting() {
12181251
assert_eq!(

0 commit comments

Comments
 (0)