From f74c4b299f0c940c6e98acaff92d6944d158bfba Mon Sep 17 00:00:00 2001 From: zzylol Date: Wed, 30 Sep 2026 12:30:22 +0000 Subject: [PATCH 1/4] feat(ir): carry PromQL bool comparisons as BinaryOpKind::CompareBool The frontend dropped the bool modifier, so `a > bool 1` lowered like the filter `a > 1`. A separate variant keeps bool off non-comparisons and reaches both the query-level BinaryOp and the post-ASAP Binary payload. Co-Authored-By: Claude Opus 5.5 --- .../src/query_physical_lowering.rs | 2 +- .../src/expressions/arithmetic.rs | 33 ++++++++---- .../src/expressions/mod.rs | 8 +++ .../src/operators/vector_binary.rs | 8 ++- .../tests/promql_binary.rs | 52 +++++++++++++++---- crates/frontend-promql/src/promql.rs | 5 +- .../frontend-promql/tests/promql_lowering.rs | 19 +++++++ crates/types/src/pre_asap/query_expr.rs | 8 ++- 8 files changed, 112 insertions(+), 23 deletions(-) diff --git a/crates/asap-aware-mapping/src/query_physical_lowering.rs b/crates/asap-aware-mapping/src/query_physical_lowering.rs index bd2aa675..1cc2b1ca 100644 --- a/crates/asap-aware-mapping/src/query_physical_lowering.rs +++ b/crates/asap-aware-mapping/src/query_physical_lowering.rs @@ -1029,7 +1029,7 @@ fn promql_binary_operation( BinaryOpKind::Set(PromQLVectorSetOpKind::And) => PromqlBinaryOperation::And, BinaryOpKind::Set(PromQLVectorSetOpKind::Or) => PromqlBinaryOperation::Or, BinaryOpKind::Set(PromQLVectorSetOpKind::Unless) => PromqlBinaryOperation::Unless, - BinaryOpKind::Arithmetic(_) | BinaryOpKind::Compare(_) => { + BinaryOpKind::Arithmetic(_) | BinaryOpKind::Compare(_) | BinaryOpKind::CompareBool(_) => { PromqlBinaryOperation::ArithmeticOrComparison } } diff --git a/crates/asap-physical-operators/src/expressions/arithmetic.rs b/crates/asap-physical-operators/src/expressions/arithmetic.rs index 30e277d4..e0766763 100644 --- a/crates/asap-physical-operators/src/expressions/arithmetic.rs +++ b/crates/asap-physical-operators/src/expressions/arithmetic.rs @@ -25,7 +25,7 @@ pub fn evaluate_binary( right: f64, ) -> Result { use crate::{values::Value, Error}; - use planner_types::pre_asap::{ArithmeticOpKind, BinaryOpKind, CompareOpKind}; + use planner_types::pre_asap::{ArithmeticOpKind, BinaryOpKind}; let invalid = || Error::Invalid("unsupported binary operation or invalid checked-division domain".into()); if operator.vector_match.is_some() { @@ -49,15 +49,28 @@ pub fn evaluate_binary( BinaryOpKind::Arithmetic(ref op) => { Value::Float64(evaluate_float64_arithmetic(op, left, right)) } - BinaryOpKind::Compare(ref op) => Value::Bool(match op { - CompareOpKind::Eq => left == right, - CompareOpKind::Ne => left != right, - CompareOpKind::Lt => left < right, - CompareOpKind::Le => left <= right, - CompareOpKind::Gt => left > right, - CompareOpKind::Ge => left >= right, - _ => return Err(invalid()), - }), + BinaryOpKind::Compare(ref op) => Value::Bool(compare(op, left, right).ok_or_else(invalid)?), + BinaryOpKind::CompareBool(ref op) => { + Value::Float64(if compare(op, left, right).ok_or_else(invalid)? { + 1. + } else { + 0. + }) + } _ => return Err(invalid()), }) } + +/// IEEE comparison, as Go's: NaN is unequal to everything, itself included. +fn compare(op: &planner_types::pre_asap::CompareOpKind, left: f64, right: f64) -> Option { + use planner_types::pre_asap::CompareOpKind; + Some(match op { + CompareOpKind::Eq => left == right, + CompareOpKind::Ne => left != right, + CompareOpKind::Lt => left < right, + CompareOpKind::Le => left <= right, + CompareOpKind::Gt => left > right, + CompareOpKind::Ge => left >= right, + _ => return None, + }) +} diff --git a/crates/asap-physical-operators/src/expressions/mod.rs b/crates/asap-physical-operators/src/expressions/mod.rs index a16a3bde..627fd931 100644 --- a/crates/asap-physical-operators/src/expressions/mod.rs +++ b/crates/asap-physical-operators/src/expressions/mod.rs @@ -87,6 +87,14 @@ impl Expression { | CompareOpKind::Gt | CompareOpKind::Ge, ) => DataType::Bool, + BinaryOpKind::CompareBool( + CompareOpKind::Eq + | CompareOpKind::Ne + | CompareOpKind::Lt + | CompareOpKind::Le + | CompareOpKind::Gt + | CompareOpKind::Ge, + ) => DataType::Float64, _ => return Err(invalid("unsupported binary operation")), }; Ok((dtype, n || m)) diff --git a/crates/asap-physical-operators/src/operators/vector_binary.rs b/crates/asap-physical-operators/src/operators/vector_binary.rs index cc60ff4c..88f019c2 100644 --- a/crates/asap-physical-operators/src/operators/vector_binary.rs +++ b/crates/asap-physical-operators/src/operators/vector_binary.rs @@ -46,8 +46,14 @@ impl Operator { left: Schema, right: Schema, operator: BinaryOperator, - return_bool: bool, + mut return_bool: bool, ) -> Result { + let mut operator = operator; + // The IR's `bool` comparison is this operator's `return_bool` mode. + if let BinaryOpKind::CompareBool(op) = &operator.kind { + operator.kind = BinaryOpKind::Compare(op.clone()); + return_bool = true; + } let scalar = is_scalar(&left)? && is_scalar(&right)?; is_scalar(&right)?; let expression = Expression::Binary { diff --git a/crates/asap-physical-operators/tests/promql_binary.rs b/crates/asap-physical-operators/tests/promql_binary.rs index 24844b5e..0a2ac35c 100644 --- a/crates/asap-physical-operators/tests/promql_binary.rs +++ b/crates/asap-physical-operators/tests/promql_binary.rs @@ -49,17 +49,18 @@ fn row(name: &str, job: &str, value: f64) -> Vec { ] } fn program() -> CompiledPhysicalDag { + program_for(BinaryOperator { + kind: BinaryOpKind::Arithmetic(ArithmeticOpKind::Div), + vector_match: None, + checked_relative_division: true, + checked_finite_division: false, + }) +} +fn program_for(operator: BinaryOperator) -> CompiledPhysicalDag { let schema = schema(); let node = PostAsapDagNode { id: PostAsapNodeId(2), - payload: PostAsapOperatorPayload::Binary { - operator: BinaryOperator { - kind: BinaryOpKind::Arithmetic(ArithmeticOpKind::Div), - vector_match: None, - checked_relative_division: true, - checked_finite_division: false, - }, - }, + payload: PostAsapOperatorPayload::Binary { operator }, output_state: ExecutionDataState::QUERY_ROWS, output_schema: (*schema).clone(), guarantee: None, @@ -80,7 +81,13 @@ fn evaluate( left: Vec>, right: Vec>, ) -> Result>, asap_physical_operators::Error> { - let graph = program(); + evaluate_with(program(), left, right) +} +fn evaluate_with( + graph: CompiledPhysicalDag, + left: Vec>, + right: Vec>, +) -> Result>, asap_physical_operators::Error> { let sources = [left, right] .into_iter() .enumerate() @@ -268,3 +275,30 @@ fn binary_obeys_memory_and_cancellation() { )); } } + +// A `bool` comparison over label-map vectors yields 1 or 0 and drops the name. +#[test] +fn label_map_bool_comparison_drops_the_name() { + let program = program_for(BinaryOperator { + kind: BinaryOpKind::CompareBool(planner_types::pre_asap::CompareOpKind::Gt), + vector_match: None, + checked_relative_division: false, + checked_finite_division: false, + }); + let rows = evaluate_with( + program, + vec![row("a", "api", 6.)], + vec![row("b", "api", 2.)], + ) + .unwrap(); + let [row] = rows.as_slice() else { + panic!("expected one row, got {}", rows.len()); + }; + let Value::Map(labels) = &row[0] else { + panic!("expected labels"); + }; + assert!(labels + .iter() + .all(|(k, _)| !matches!(k, Value::Utf8(k) if &**k == "__name__"))); + assert!(matches!(row[1], Value::Float64(v) if v == 1.)); +} diff --git a/crates/frontend-promql/src/promql.rs b/crates/frontend-promql/src/promql.rs index b71a8c1e..6564003d 100644 --- a/crates/frontend-promql/src/promql.rs +++ b/crates/frontend-promql/src/promql.rs @@ -1274,7 +1274,10 @@ fn selector_is_bucket(vs: &VectorSelector) -> bool { fn walk_binary(bin: &BinaryExpr) -> Result { let lhs = scalar_or_vector(&bin.lhs)?; let rhs = scalar_or_vector(&bin.rhs)?; - let op = binop(bin.op.id())?; + let op = match (binop(bin.op.id())?, bin.return_bool()) { + (BinaryOpKind::Compare(op), true) => BinaryOpKind::CompareBool(op), + (op, _) => op, + }; let vector_match = bin.modifier.as_ref().map(|m| { let (kind, labels) = match &m.matching { Some(LabelModifier::Include(ls)) => (VectorMatchKind::On, ls.labels.clone()), diff --git a/crates/frontend-promql/tests/promql_lowering.rs b/crates/frontend-promql/tests/promql_lowering.rs index 3c92b7ba..a71cb5ba 100644 --- a/crates/frontend-promql/tests/promql_lowering.rs +++ b/crates/frontend-promql/tests/promql_lowering.rs @@ -705,6 +705,25 @@ fn binary_op_with_on_grouping() { assert_eq!(vm.labels, vec!["host".to_string()]); } +// `bool` changes a comparison from a filter to a 0/1 result, so the IR must +// carry it. +#[test] +fn bool_comparisons_are_distinct() { + let op = |q: &str| match lower(q) { + QueryExpr::BinaryOp { op, .. } => op, + other => panic!("expected BinaryOp, got {other:?}"), + }; + assert_eq!(op("a > 1"), BinaryOpKind::Compare(CompareOpKind::Gt)); + assert_eq!( + op("a > bool 1"), + BinaryOpKind::CompareBool(CompareOpKind::Gt) + ); + assert_eq!( + op("a == bool on(job) b"), + BinaryOpKind::CompareBool(CompareOpKind::Eq) + ); +} + #[test] fn binary_op_binds_each_branch_against_its_own_schema() { // Each side scans a different metric and groups by a different label. With a diff --git a/crates/types/src/pre_asap/query_expr.rs b/crates/types/src/pre_asap/query_expr.rs index 5b63e5a2..c5e7f40a 100644 --- a/crates/types/src/pre_asap/query_expr.rs +++ b/crates/types/src/pre_asap/query_expr.rs @@ -254,8 +254,13 @@ pub enum BinaryOpKind { /// Arithmetic — `Add/Sub/Mul/Div/Mod` (shared with `QueryExpr::Arithmetic`). Arithmetic(ArithmeticOpKind), /// Comparison — `Eq/Ne/Lt/Le/Gt/Ge` + `Like/ILike/Regex` family (shared - /// with `QueryExpr::Compare`). + /// with `QueryExpr::Compare`). PromQL keeps the matched series whose + /// comparison holds. Compare(CompareOpKind), + /// PromQL comparison with the `bool` modifier: every matched series + /// yields 1 or 0 and loses its metric name. A separate variant, not a + /// flag, because only comparisons take `bool`. + CompareBool(CompareOpKind), /// PromQL vector-set operation. Set(PromQLVectorSetOpKind), } @@ -265,6 +270,7 @@ impl std::fmt::Display for BinaryOpKind { match self { BinaryOpKind::Arithmetic(op) => write!(f, "{op}"), BinaryOpKind::Compare(op) => write!(f, "{op}"), + BinaryOpKind::CompareBool(op) => write!(f, "{op} bool"), BinaryOpKind::Set(PromQLVectorSetOpKind::And) => f.write_str("AND"), BinaryOpKind::Set(PromQLVectorSetOpKind::Or) => f.write_str("OR"), BinaryOpKind::Set(PromQLVectorSetOpKind::Unless) => f.write_str("unless"), From 13baddb0cdf5985ce9c26ca1c0af643aee0ba7aa Mon Sep 17 00:00:00 2001 From: zzylol Date: Wed, 30 Sep 2026 12:30:22 +0000 Subject: [PATCH 2/4] feat(physical): compile PromQL comparisons, set operators, and group modifiers series_binary now implements Prometheus' VectorBinop, VectorAnd/Or/Unless and vector-scalar semantics for Fallback subtrees and query-time Binary nodes, including scalar() operands. Range functions drop __name__ in the Fallback and reject equal label sets. Conflicts with earlier stack changes resolved to the integration tree: - crates/asap-physical-operators/src/physical_planner/promql_rows.rs: a9b8fdd Merge #492 comparison and set coverage with shared series identity typing Co-Authored-By: Claude Opus 5.5 --- .../src/operators/mod.rs | 1 + .../src/operators/series_labels.rs | 406 +++++++++++++++--- .../src/operators/unchecked.rs | 4 +- .../src/physical_planner/mod.rs | 89 ++-- .../src/physical_planner/promql_fallback.rs | 146 ++----- .../src/physical_planner/row_values.rs | 235 +--------- .../tests/deployment_computation.rs | 58 ++- .../tests/promql_fallback.rs | 330 +++++++++++++- 8 files changed, 822 insertions(+), 447 deletions(-) diff --git a/crates/asap-physical-operators/src/operators/mod.rs b/crates/asap-physical-operators/src/operators/mod.rs index 8c189fd4..d34314c0 100644 --- a/crates/asap-physical-operators/src/operators/mod.rs +++ b/crates/asap-physical-operators/src/operators/mod.rs @@ -85,6 +85,7 @@ enum Kind { }, SeriesBinary { operator: planner_types::post_asap::BinaryOperator, + scalars: [bool; 2], }, Project(Vec), Filter(Expression), diff --git a/crates/asap-physical-operators/src/operators/series_labels.rs b/crates/asap-physical-operators/src/operators/series_labels.rs index 5032f435..e6f4a43d 100644 --- a/crates/asap-physical-operators/src/operators/series_labels.rs +++ b/crates/asap-physical-operators/src/operators/series_labels.rs @@ -1,5 +1,5 @@ -//! PromQL label-set rewriting and one-to-one vector matching over rows that -//! carry a series identity or plain label columns. +//! PromQL label-set rewriting and binary operators over rows that carry a +//! series identity or plain label columns. use super::*; use planner_types::{ post_asap::BinaryOperator, @@ -113,34 +113,344 @@ impl Operator { }) } - /// PromQL one-to-one arithmetic between rows with equal label sets. The - /// result keeps the left row, without the metric name. + /// A PromQL binary operator with Prometheus' matching, metric-name, and + /// duplicate rules. `scalars` marks the operands that are one-row PromQL + /// scalars, such as a literal or `scalar(x)`, rather than vectors. + /// Vectors match by `operator.vector_match`; `None` matches all labels + /// but the name. The result has the vector operand's schema, the left one + /// between vectors; label columns without a series identity must hold + /// every label the result can take from the right side. pub fn series_binary( left: Schema, right: Schema, operator: BinaryOperator, + scalars: [bool; 2], ) -> Result { - layout(&left)?; - layout(&right)?; - if !matches!(operator.kind, BinaryOpKind::Arithmetic(_)) || operator.vector_match.is_some() + use planner_types::pre_asap::{CompareOpKind::*, GroupSide, PromQLVectorSetOpKind}; + let vectors = scalars == [false, false]; + let valid = match &operator.kind { + BinaryOpKind::Arithmetic(_) => true, + BinaryOpKind::CompareBool(op) => matches!(op, Eq | Ne | Lt | Le | Gt | Ge), + // Prometheus requires `bool` between two scalars. + BinaryOpKind::Compare(op) => { + matches!(op, Eq | Ne | Lt | Le | Gt | Ge) && scalars != [true, true] + } + BinaryOpKind::Set(_) => vectors, + }; + let grouping = operator + .vector_match + .as_ref() + .and_then(|m| m.grouping.as_ref()); + // The frontend records a `bool` modifier as default matching. + let default_match = operator.vector_match.as_ref().is_none_or(|m| { + m.kind == VectorMatchKind::Ignoring && m.labels.is_empty() && m.grouping.is_none() + }); + if !valid + || (!vectors && !default_match) + || (grouping.is_some() && matches!(operator.kind, BinaryOpKind::Set(_))) { - return Err(invalid("series binary requires unmatched arithmetic")); + return Err(invalid("unsupported PromQL binary operation")); + } + for (schema, scalar) in [(&left, scalars[0]), (&right, scalars[1])] { + if scalar { + if schema.fields.len() != 1 || plain(schema, 0)? != (&DataType::Float64, false) { + return Err(invalid("PromQL scalar operand must be one Float64 value")); + } + } else { + layout(schema)?; + } + } + if vectors { + let (l, r) = (layout(&left)?, layout(&right)?); + let names = |layout: &Layout, schema: &Schema| { + layout + .labels + .iter() + .map(|&i| schema.fields[i].name.clone()) + .collect::>() + }; + let right_rows = matches!(operator.kind, BinaryOpKind::Set(PromQLVectorSetOpKind::Or)) + || matches!(grouping, Some(g) if g.side == GroupSide::Right); + let fits = l.identity.is_some() + || match grouping { + _ if right_rows => { + r.identity.is_none() && names(&r, &right).is_subset(&names(&l, &left)) + } + Some(g) => g + .labels + .iter() + .all(|label| names(&l, &left).contains(label)), + None => true, + }; + let or = matches!(operator.kind, BinaryOpKind::Set(PromQLVectorSetOpKind::Or)); + // A series identity holds any label set, since `write` re-encodes it. + // A right row needs a time only if the left layout has one. + if !fits || (or && left.time_index.is_some() && right.time_index.is_none()) { + return Err(invalid( + "PromQL binary result labels do not fit the left schema", + )); + } } + let output = if scalars == [true, false] { + right.clone() + } else { + left.clone() + }; Ok(Self { - kind: Kind::SeriesBinary { operator }, - inputs: vec![left.clone(), right], - output: left, + kind: Kind::SeriesBinary { operator, scalars }, + inputs: vec![left, right], + output, }) } } +/// PromQL's matching signature: `on` keeps only the listed labels; `ignoring` +/// drops them and the metric name. +fn signature(kind: &VectorMatchKind, names: &[String], mut labels: Labels) -> Labels { + match kind { + VectorMatchKind::On => labels.retain(|k, _| names.contains(k)), + VectorMatchKind::Ignoring => labels.retain(|k, _| k != "__name__" && !names.contains(k)), + } + labels +} + +fn label_bytes(labels: &Labels) -> usize { + labels.iter().map(|(k, v)| 64 + k.len() + v.len()).sum() +} + +fn float(layout: &Layout, row: &[Value]) -> Result { + match row[layout.value] { + Value::Float64(value) => Ok(value), + _ => Err(invalid("vector value must be Float64")), + } +} + +/// The value of a matched pair, or `None` when a comparison filters it out. +/// A filter keeps the left value. +fn apply(operator: &BinaryOperator, left: f64, right: f64) -> Result, Error> { + let unmatched = BinaryOperator { + vector_match: None, + ..operator.clone() + }; + match crate::expressions::arithmetic::evaluate_binary(&unmatched, left, right)? { + Value::Float64(value) => Ok(Some(value)), + Value::Bool(keep) => Ok(keep.then_some(left)), + _ => Err(invalid("PromQL binary result must be a number")), + } +} + +/// Evaluate `Kind::SeriesBinary` over the collected operand rows. +async fn series_binary( + operator: &Operator, + binary: &BinaryOperator, + scalars: [bool; 2], + rows: [Vec>; 2], + work: &mut Cooperative, + workspace: &mut Workspace, +) -> Result>, Error> { + use planner_types::pre_asap::{GroupSide, PromQLVectorSetOpKind}; + let drops_name = matches!( + binary.kind, + BinaryOpKind::Arithmetic(_) | BinaryOpKind::CompareBool(_) + ); + let scalar = |rows: &[Vec]| match rows { + [row] => match row[0] { + Value::Float64(value) => Ok(value), + _ => Err(invalid("PromQL scalar must be Float64")), + }, + _ => Err(invalid("PromQL scalar operand must have one row")), + }; + let [left, right] = rows; + let mut result = Vec::new(); + if scalars == [true, true] { + let value = apply(binary, scalar(&left)?, scalar(&right)?)? + .ok_or_else(|| invalid("scalar comparison requires bool"))?; + return Ok(vec![vec![Value::Float64(value)]]); + } + if scalars[0] || scalars[1] { + let (constant, vector, schema) = if scalars[0] { + (scalar(&left)?, right, &operator.inputs[1]) + } else { + (scalar(&right)?, left, &operator.inputs[0]) + }; + let layout = layout(schema)?; + let mut seen = std::collections::BTreeSet::new(); + for mut row in vector { + work.checkpoint().await?; + let value = float(&layout, &row)?; + let (l, r) = if scalars[0] { + (constant, value) + } else { + (value, constant) + }; + let Some(mut computed) = apply(binary, l, r)? else { + continue; + }; + // A filter keeps the vector's value, even on the right. + if matches!(binary.kind, BinaryOpKind::Compare(_)) { + computed = value; + } + let mut set = layout.read(schema, &row)?; + if drops_name { + set.remove("__name__"); + layout.write(schema, &mut row, &set)?; + } + row[layout.value] = Value::Float64(computed); + workspace.grow(row_bytes(&row) + label_bytes(&set))?; + if !seen.insert(set) { + return Err(invalid( + "vector cannot contain metrics with the same labelset", + )); + } + result.push(row); + } + return Ok(result); + } + let schemas = [&operator.inputs[0], &operator.inputs[1]]; + let layouts = [layout(schemas[0])?, layout(schemas[1])?]; + let (kind, names, grouping) = match &binary.vector_match { + None => (VectorMatchKind::Ignoring, &[][..], None), + Some(m) => (m.kind.clone(), m.labels.as_slice(), m.grouping.as_ref()), + }; + let read = |side: usize, row: &[Value]| layouts[side].read(schemas[side], row); + if let BinaryOpKind::Set(set) = &binary.kind { + // `and`/`unless` look up the right side; `or` adds unmatched right rows. + let lookup = if *set == PromQLVectorSetOpKind::Or { + &left + } else { + &right + }; + let side = usize::from(*set != PromQLVectorSetOpKind::Or); + let mut signatures = std::collections::BTreeSet::new(); + for row in lookup { + work.checkpoint().await?; + let labels = signature(&kind, names, read(side, row)?); + workspace.grow(label_bytes(&labels))?; + signatures.insert(labels); + } + let and = *set == PromQLVectorSetOpKind::And; + for row in &left { + work.checkpoint().await?; + let found = signatures.contains(&signature(&kind, names, read(0, row)?)); + if *set == PromQLVectorSetOpKind::Or || found == and { + workspace.grow(row_bytes(row))?; + result.push(row.clone()); + } + } + if *set == PromQLVectorSetOpKind::Or { + for row in &right { + work.checkpoint().await?; + let labels = read(1, row)?; + if signatures.contains(&signature(&kind, names, labels.clone())) { + continue; + } + let mut out = vec![Value::Null; schemas[0].fields.len()]; + if let (Some(to), Some(from)) = (schemas[0].time_index, schemas[1].time_index) { + out[to] = row[from].clone(); + } + out[layouts[0].value] = Value::Float64(float(&layouts[1], row)?); + layouts[0].write(schemas[0], &mut out, &labels)?; + workspace.grow(row_bytes(&out))?; + result.push(out); + } + } + return Ok(result); + } + // Prometheus returns before matching when either side is empty. + if left.is_empty() || right.is_empty() { + return Ok(result); + } + // `group_right` makes the left side the "one" side. + let swapped = matches!(grouping, Some(g) if g.side == GroupSide::Right); + let (one, many) = if swapped { (0, 1) } else { (1, 0) }; + let sides = [&left, &right]; + let mut ones = BTreeMap::new(); + for (index, row) in sides[one].iter().enumerate() { + work.checkpoint().await?; + let labels = read(one, row)?; + let key = signature(&kind, names, labels.clone()); + workspace.grow(2 * label_bytes(&labels))?; + if ones + .insert(key, (labels, float(&layouts[one], row)?, index)) + .is_some() + { + return Err(invalid(&format!( + "found duplicate series for the match group on the {} hand-side of the operation", + if swapped { "left" } else { "right" } + ))); + } + } + let mut matched = BTreeMap::>::new(); + for row in sides[many].iter() { + work.checkpoint().await?; + let labels = read(many, row)?; + let key = signature(&kind, names, labels.clone()); + let Some((one_labels, one_value, one_index)) = ones.get(&key) else { + continue; + }; + let value = float(&layouts[many], row)?; + let (l, r) = if swapped { + (*one_value, value) + } else { + (value, *one_value) + }; + let Some(computed) = apply(binary, l, r)? else { + continue; + }; + let mut metric = labels; + if matches!(binary.kind, BinaryOpKind::Arithmetic(_)) { + metric.remove("__name__"); + } + match grouping { + None => match kind { + VectorMatchKind::On => metric.retain(|k, _| names.contains(k)), + VectorMatchKind::Ignoring => metric.retain(|k, _| !names.contains(k)), + }, + // Included labels come from the "one" side. + Some(g) => { + for label in &g.labels { + match one_labels.get(label) { + Some(value) => metric.insert(label.clone(), value.clone()), + None => metric.remove(label), + }; + } + } + } + if matches!(binary.kind, BinaryOpKind::CompareBool(_)) { + metric.remove("__name__"); + } + workspace.grow(label_bytes(&metric))?; + let results = matched.entry(key).or_default(); + if grouping.is_none() && !results.is_empty() { + return Err(invalid( + "many-to-one matching must be explicit (group_left/group_right)", + )); + } + if !results.insert(metric.clone()) { + return Err(invalid( + "multiple matches for labels: grouping labels must ensure unique matches", + )); + } + // The output row is in the left layout. + let mut out = if swapped { + left[*one_index].clone() + } else { + row.clone() + }; + layouts[0].write(schemas[0], &mut out, &metric)?; + out[layouts[0].value] = Value::Float64(computed); + workspace.grow(row_bytes(&out))?; + result.push(out); + } + Ok(result) +} + pub(super) fn execute<'a>( operator: &'a Operator, mut inputs: Vec>, context: RunContext, ) -> Result, Error> { let output = operator.output.clone(); - let left_layout = layout(&operator.inputs[0])?; let right = match operator.kind { Kind::SeriesBinary { .. } => { Some(inputs.pop().ok_or_else(|| invalid("missing right input"))?) @@ -162,22 +472,17 @@ pub(super) fn execute<'a>( }, None, ) => { + let left_layout = layout(&operator.inputs[0])?; let mut seen = std::collections::BTreeSet::new(); for mut row in rows { work.checkpoint().await?; - let mut set = left_layout.read(&output, &row)?; - match kind { - VectorMatchKind::On => set.retain(|k, _| labels.contains(k)), - VectorMatchKind::Ignoring => { - set.retain(|k, _| k != "__name__" && !labels.contains(k)) - } - } + let set = signature(kind, labels, left_layout.read(&output, &row)?); left_layout.write(&output, &mut row, &set)?; workspace.grow(row_bytes(&row))?; // Matching and grouping sides may repeat a label set; // their consumers decide whether that is an error. if *unique { - workspace.grow(set.iter().map(|(k, v)| 64 + k.len() + v.len()).sum())?; + workspace.grow(label_bytes(&set))?; if !seen.insert(set) { return Err(invalid( "vector cannot contain metrics with the same labelset", @@ -187,52 +492,23 @@ pub(super) fn execute<'a>( result.push(row); } } - (Kind::SeriesBinary { operator: binary }, Some(right)) => { - let right_schema = &operator.inputs[1]; - let right_layout = layout(right_schema)?; + ( + Kind::SeriesBinary { + operator: binary, + scalars, + }, + Some(right), + ) => { let (right, _right_memory) = collect_rows(right, &context).await?; - // Prometheus returns before matching when either side is empty. - if rows.is_empty() || right.is_empty() { - return Batch::try_new(output.clone(), vec![]); - } - let mut matches = BTreeMap::new(); - for row in &right { - work.checkpoint().await?; - let set = right_layout.read(right_schema, row)?; - workspace.grow(set.iter().map(|(k, v)| 64 + k.len() + v.len()).sum())?; - let Value::Float64(value) = row[right_layout.value] else { - return Err(invalid("vector value must be Float64")); - }; - // Prometheus rejects a duplicate on the one side. - if matches.insert(set, (value, false)).is_some() { - return Err(invalid( - "duplicate series for a match group on the right-hand side", - )); - } - } - for mut row in rows { - work.checkpoint().await?; - let mut set = left_layout.read(&output, &row)?; - let Some((value, matched)) = matches.get_mut(&set) else { - continue; - }; - // Only a left duplicate that finds a match is ambiguous. - if std::mem::replace(matched, true) { - return Err(invalid( - "many-to-one matching must be explicit (group_left/group_right)", - )); - } - let Value::Float64(left_value) = row[left_layout.value] else { - return Err(invalid("vector value must be Float64")); - }; - row[left_layout.value] = crate::expressions::arithmetic::evaluate_binary( - binary, left_value, *value, - )?; - set.remove("__name__"); - left_layout.write(&output, &mut row, &set)?; - workspace.grow(row_bytes(&row))?; - result.push(row); - } + result = series_binary( + operator, + binary, + *scalars, + [rows, right], + &mut work, + &mut workspace, + ) + .await?; } _ => return Err(invalid("series label operator inputs mismatch")), } diff --git a/crates/asap-physical-operators/src/operators/unchecked.rs b/crates/asap-physical-operators/src/operators/unchecked.rs index fdafb630..20874c9a 100644 --- a/crates/asap-physical-operators/src/operators/unchecked.rs +++ b/crates/asap-physical-operators/src/operators/unchecked.rs @@ -71,8 +71,8 @@ impl TryFrom for Operator { Kind::SeriesLabels { kind, labels, .. } => { Operator::series_labels(input(0)?, kind, labels)? } - Kind::SeriesBinary { operator } => { - Operator::series_binary(input(0)?, input(1)?, operator)? + Kind::SeriesBinary { operator, scalars } => { + Operator::series_binary(input(0)?, input(1)?, operator, scalars)? } Kind::Project(expressions) => { if expressions.len() != output.fields.len() { diff --git a/crates/asap-physical-operators/src/physical_planner/mod.rs b/crates/asap-physical-operators/src/physical_planner/mod.rs index 2e244692..b8312ef1 100644 --- a/crates/asap-physical-operators/src/physical_planner/mod.rs +++ b/crates/asap-physical-operators/src/physical_planner/mod.rs @@ -464,6 +464,25 @@ fn compile_internal( if let Payload::Binary { operator } = &node.payload { let query_time = node.output_state.timing == planner_types::post_asap::ExecutionTiming::QueryTime; + // Per-series readouts keep `__name__` even where the range + // function drops it; only name-dropping operators ignore that. + let per_series = schemas.iter().any(|schema| { + schema + .fields + .iter() + .any(|f| f.name == promql_rows::SERIES_IDENTITY_COLUMN) + }); + if per_series + && matches!( + operator.kind, + planner_types::pre_asap::BinaryOpKind::Compare(_) + | planner_types::pre_asap::BinaryOpKind::Set(_) + ) + { + return Err(invalid(format!( + "node {id}: per-series readouts do not apply the range function's __name__ rule" + ))); + } if let Some(&(value, left)) = literals.get(&id) { let [input] = schemas.as_slice() else { return Err(invalid("scalar binary requires one row input")); @@ -471,23 +490,27 @@ fn compile_internal( if !query_time { return Err(invalid("scalar literal binary must run at query time")); } - if row_values::per_series(input) { - let mut chain = - row_values::series_scalar_binary(input, operator, value, left) - .map_err(|error| invalid(format!("node {id}: {error}")))?; - let last = chain.pop().expect("nonempty chain"); - let mut inputs = inputs; - for operator in chain { - graph.add(auxiliary, inputs, operator)?; - inputs = vec![auxiliary]; - auxiliary -= 1; - } - graph.add(id, inputs, last.with_output_schema(output)?)?; - continue; - } - let project = row_values::scalar_binary(input, operator, value, left) + let scalar = + Operator::scalar(crate::values::Value::Float64(value), DataType::Float64)?; + let (sides, scalars, operands) = if left { + ( + [scalar.schema(), input.clone()], + [true, false], + vec![auxiliary, inputs[0]], + ) + } else { + ( + [input.clone(), scalar.schema()], + [false, true], + vec![inputs[0], auxiliary], + ) + }; + let [l, r] = sides; + let binary = Operator::series_binary(l, r, operator.clone(), scalars) .map_err(|error| invalid(format!("node {id}: {error}")))?; - graph.add(id, inputs, project.with_output_schema(output)?)?; + graph.add(auxiliary, vec![], scalar)?; + graph.add(id, operands, binary.with_output_schema(output)?)?; + auxiliary -= 1; continue; } let label_map = |schema: &Schema| { @@ -496,24 +519,26 @@ fn compile_internal( .iter() .any(|f| matches!(f.dtype, SummaryFamilyType::Plain(DataType::Map { .. }))) }; + // Grouped rows carry their labels as columns; per-series rows + // carry the series identity. if let (true, [left, right]) = (query_time, schemas.as_slice()) { - if row_values::per_series(left) || row_values::per_series(right) { - let [left, right, binary] = - row_values::series_vector_binary(left, right, operator) - .map_err(|error| invalid(format!("node {id}: {error}")))?; - let sides = [auxiliary, auxiliary - 1]; - graph.add(sides[0], vec![inputs[0]], left)?; - graph.add(sides[1], vec![inputs[1]], right)?; - graph.add(id, sides.to_vec(), binary.with_output_schema(output)?)?; - auxiliary -= 2; - continue; - } if !label_map(left) && !label_map(right) { - let (join, project) = row_values::grouped_binary(left, right, operator) - .map_err(|error| invalid(format!("node {id}: {error}")))?; - graph.add(auxiliary, inputs, join)?; - graph.add(id, vec![auxiliary], project.with_output_schema(output)?)?; - auxiliary -= 1; + // A scalar-valued Fallback operand, such as `scalar(x)`, has no labels. + let scalar = |input: &NodeId| { + matches!( + nodes.get(input).map(|node| &node.payload), + Some(Payload::Fallback { expression }) + if promql_fallback::scalar(expression) + ) + }; + let binary = Operator::series_binary( + left.clone(), + right.clone(), + operator.clone(), + [scalar(&inputs[0]), scalar(&inputs[1])], + ) + .map_err(|error| invalid(format!("node {id}: {error}")))?; + graph.add(id, inputs, binary.with_output_schema(output)?)?; continue; } } diff --git a/crates/asap-physical-operators/src/physical_planner/promql_fallback.rs b/crates/asap-physical-operators/src/physical_planner/promql_fallback.rs index b0234acc..7e45f9f5 100644 --- a/crates/asap-physical-operators/src/physical_planner/promql_fallback.rs +++ b/crates/asap-physical-operators/src/physical_planner/promql_fallback.rs @@ -4,7 +4,7 @@ use super::*; use crate::operators::SubquerySteps; use planner_types::post_asap::execution_data_state::lift_plain; -use planner_types::pre_asap::{AtModifier, BinaryOpKind, VectorMatch, VectorMatchKind}; +use planner_types::pre_asap::{AtModifier, VectorMatchKind}; /// Input slot for the raw series read by the `selector`th selector (in /// [`raw_series`] order) of Fallback node `node`. The node's own ID names its @@ -84,14 +84,16 @@ fn selector(expression: &QueryExpr) -> Result<(i64, i64, Option), Error> { Ok((millis(range)?, offset, at)) } -/// PromQL scalar-valued expressions have no labels to match. -fn scalar(expression: &QueryExpr) -> bool { - matches!( - expression, +/// PromQL scalar-valued expressions have no labels to match. A binary +/// operator is scalar-valued when both operands are. +pub(super) fn scalar(expression: &QueryExpr) -> bool { + match expression { QueryExpr::PromqlScalarBridge(_) - | QueryExpr::PromqlScalarFromVector(_) - | QueryExpr::EvalTimestamp - ) + | QueryExpr::PromqlScalarFromVector(_) + | QueryExpr::EvalTimestamp => true, + QueryExpr::BinaryOp { lhs, rhs, .. } => scalar(lhs) && scalar(rhs), + _ => false, + } } impl Lowering { @@ -155,7 +157,13 @@ impl Lowering { let [function] = measures.as_slice() else { return Err(invalid("range function requires one measure")); }; - self.range_function(function, child, expression) + let step = self.range_function(function, child, expression)?; + if matches!(function, AggIntent::LastOverTime) { + return Ok(step); + } + // Other range functions drop the name; equal label sets then error. + let input = self.schema(&step); + Ok(self.add(Operator::series_without_name(input)?, vec![step])) } QueryExpr::Aggregate { reduction: planner_types::pre_asap::Reduction::Reduce(keys), @@ -206,57 +214,25 @@ impl Lowering { ) } QueryExpr::BinaryOp { - op: BinaryOpKind::Arithmetic(op), + op, lhs, rhs, vector_match, } => { - let (vector, literal, literal_left) = match ( - row_values::scalar_literal(lhs), - row_values::scalar_literal(rhs), - ) { - (None, Some(value)) => (lhs, value, false), - (Some(value), None) => (rhs, value, true), - (None, None) => return self.match_vectors(expression, vector_match), - _ => return Err(invalid("PromQL arithmetic between two literals")), - }; - let step = self.value(vector)?; - let input = self.schema(&step); - let step = self.add(Operator::series_without_name(input.clone())?, vec![step]); - let value = named_column(&input, &ColumnRef::SampleValue)?; - let literal = Expression::Literal { - value: crate::values::Value::Float64(literal), - dtype: DataType::Float64, - }; + let sides = vec![self.value(lhs)?, self.value(rhs)?]; let operator = planner_types::post_asap::BinaryOperator { - kind: planner_types::pre_asap::BinaryOpKind::Arithmetic(op.clone()), - vector_match: None, + kind: op.clone(), + vector_match: vector_match.clone(), checked_relative_division: false, checked_finite_division: false, }; - let (left, right) = if literal_left { - (literal, Expression::Column(value)) - } else { - (Expression::Column(value), literal) - }; - let columns = input - .fields - .iter() - .enumerate() - .map(|(i, field)| { - let expression = if i == value { - Expression::Binary { - operator: operator.clone(), - left: Box::new(left.clone()), - right: Box::new(right.clone()), - } - } else { - Expression::Column(i) - }; - (field.name.clone(), expression) - }) - .collect(); - self.push(Operator::project(input, columns)?, vec![step], expression) + let binary = Operator::series_binary( + self.schema(&sides[0]), + self.schema(&sides[1]), + operator, + [scalar(lhs), scalar(rhs)], + )?; + self.push(binary, sides, expression) } QueryExpr::PromqlScalarFromVector(child) => { let step = self.value(child)?; @@ -289,54 +265,6 @@ impl Lowering { } } - /// One-to-one vector arithmetic: both sides reduce to their matching - /// labels, which are also the result's labels. - fn match_vectors( - &mut self, - logical: &QueryExpr, - vector_match: &Option, - ) -> Result { - let QueryExpr::BinaryOp { - op: kind, lhs, rhs, .. - } = logical - else { - unreachable!() - }; - if scalar(lhs) || scalar(rhs) { - return Err(invalid("PromQL arithmetic with a non-literal scalar")); - } - let (matching, labels) = match vector_match { - None => (VectorMatchKind::Ignoring, vec![]), - Some(VectorMatch { - kind, - labels, - grouping: None, - }) => (kind.clone(), labels.clone()), - Some(_) => return Err(invalid("group_left/group_right matching is unsupported")), - }; - let mut sides = Vec::new(); - for side in [lhs, rhs] { - let step = self.value(side)?; - let input = self.schema(&step); - sides.push(self.add( - Operator::series_labels(input, matching.clone(), labels.clone())?, - vec![step], - )); - } - let (left, right) = (self.schema(&sides[0]), self.schema(&sides[1])); - let operator = planner_types::post_asap::BinaryOperator { - kind: kind.clone(), - vector_match: None, - checked_relative_division: false, - checked_finite_division: false, - }; - self.push( - Operator::series_binary(left, right, operator)?, - sides, - logical, - ) - } - /// `function(matrix)`, where the matrix is a range selector or a subquery. fn range_function( &mut self, @@ -390,11 +318,25 @@ impl Lowering { let (range, inner_offset, inner_at) = selector(selected)?; let raw = self.read(selected)?; let schema = self.schema(&raw); - let step = self.push( - Operator::series_window(schema, inner, range, inner_offset, inner_at, Some(steps))?, + let mut step = self.push( + Operator::series_window( + schema, + inner.clone(), + range, + inner_offset, + inner_at, + Some(steps), + )?, vec![raw], child, )?; + // The inner function drops the name too. A series repeats across + // steps, so this rewrite does not check for equal label sets. + if inner.is_some() && !matches!(inner, Some(AggIntent::LastOverTime)) { + let input = self.schema(&step); + let relabel = Operator::series_labels(input, VectorMatchKind::Ignoring, vec![])?; + step = self.add(relabel, vec![step]); + } let input = self.schema(&step); self.push( Operator::series_window(input, Some(function), steps.range_ms, offset, at_ms, None)?, diff --git a/crates/asap-physical-operators/src/physical_planner/row_values.rs b/crates/asap-physical-operators/src/physical_planner/row_values.rs index 7e2c7c5f..8763437b 100644 --- a/crates/asap-physical-operators/src/physical_planner/row_values.rs +++ b/crates/asap-physical-operators/src/physical_planner/row_values.rs @@ -1,10 +1,7 @@ //! Query-time PromQL value computation over logical row schemas. use super::*; -use planner_types::post_asap::{maintained_population::PopulationReadout, BinaryOperator}; -use planner_types::pre_asap::{ - BinaryOpKind, DataType, Predicate, ScalarValue, VectorMatch, VectorMatchKind, -}; -use std::rc::Rc; +use planner_types::post_asap::maintained_population::PopulationReadout; +use planner_types::pre_asap::{DataType, ScalarValue}; /// A PromQL number literal has no row schema; its consumer folds it in. pub(super) fn scalar_literal(expression: &QueryExpr) -> Option { @@ -15,234 +12,6 @@ pub(super) fn scalar_literal(expression: &QueryExpr) -> Option { } } -/// Rows without a time column, label map, or series identity carry only -/// their group labels, so those labels are the complete PromQL identity. -fn grouped_value(input: &Schema) -> Result<(usize, Vec), Error> { - if input.time_index.is_some() - || input - .fields - .iter() - .any(|field| field.name == promql_rows::SERIES_IDENTITY_COLUMN) - { - return Err(invalid( - "row binary requires grouped rows or rows with a series identity", - )); - } - let mut value = None; - let mut labels = Vec::new(); - for (i, field) in input.fields.iter().enumerate() { - match &field.dtype { - SummaryFamilyType::Plain(DataType::Float64) if value.is_none() => value = Some(i), - SummaryFamilyType::Plain(DataType::Utf8) => labels.push(i), - _ => { - return Err(invalid( - "row binary requires Utf8 labels and one Float64 value", - )) - } - } - } - Ok(( - value.ok_or_else(|| invalid("row binary requires a Float64 value"))?, - labels, - )) -} - -fn arithmetic(operator: &BinaryOperator) -> Result<(), Error> { - if !matches!(operator.kind, BinaryOpKind::Arithmetic(_)) { - return Err(invalid( - "row comparison requires filter or bool semantics, which Binary does not carry", - )); - } - Ok(()) -} - -/// Rows whose labels are the encoded PromQL series identity, such as -/// per-series readouts of stored state. -pub(super) fn per_series(input: &Schema) -> bool { - input - .fields - .iter() - .any(|field| field.name == promql_rows::SERIES_IDENTITY_COLUMN) -} - -/// Apply `vector op scalar` (or `scalar op vector`) to each row's value. -pub(super) fn scalar_binary( - input: &Schema, - operator: &BinaryOperator, - literal: f64, - literal_left: bool, -) -> Result { - arithmetic(operator)?; - let (value, _) = grouped_value(input)?; - literal_projection(input, value, operator, literal, literal_left) -} - -/// `scalar_binary` over per-series rows: PromQL arithmetic also drops the -/// metric name from the series identity. Returns a chain. -pub(super) fn series_scalar_binary( - input: &Schema, - operator: &BinaryOperator, - literal: f64, - literal_left: bool, -) -> Result, Error> { - arithmetic(operator)?; - let relabel = Operator::series_without_name(input.clone())?; - let value = input - .fields - .iter() - .position(|field| { - field.dtype == SummaryFamilyType::Plain(DataType::Float64) && !field.nullable - }) - .ok_or_else(|| invalid("per-series rows require a Float64 value"))?; - let project = literal_projection(input, value, operator, literal, literal_left)?; - Ok(vec![relabel, project]) -} - -/// PromQL one-to-one arithmetic where either side carries a series identity. -/// Returns the left and right relabelings to the matching labels, and the -/// binary over their outputs. -pub(super) fn series_vector_binary( - left: &Schema, - right: &Schema, - operator: &BinaryOperator, -) -> Result<[Operator; 3], Error> { - arithmetic(operator)?; - let (kind, labels) = match &operator.vector_match { - None => (VectorMatchKind::Ignoring, vec![]), - Some(VectorMatch { - kind, - labels, - grouping: None, - }) => (kind.clone(), labels.clone()), - Some(_) => return Err(invalid("group_left/group_right matching is unsupported")), - }; - let left_labels = Operator::series_labels(left.clone(), kind.clone(), labels.clone())?; - let right_labels = Operator::series_labels(right.clone(), kind, labels)?; - let binary = Operator::series_binary( - left_labels.schema(), - right_labels.schema(), - BinaryOperator { - vector_match: None, - ..operator.clone() - }, - )?; - Ok([left_labels, right_labels, binary]) -} - -fn literal_projection( - input: &Schema, - value: usize, - operator: &BinaryOperator, - literal: f64, - literal_left: bool, -) -> Result { - let literal = Expression::Literal { - value: crate::values::Value::Float64(literal), - dtype: DataType::Float64, - }; - let columns = input - .fields - .iter() - .enumerate() - .map(|(i, field)| { - let expression = if i != value { - Expression::Column(i) - } else if literal_left { - binary(operator, literal.clone(), Expression::Column(i)) - } else { - binary(operator, Expression::Column(i), literal.clone()) - }; - (field.name.clone(), expression) - }) - .collect(); - Operator::project(input.clone(), columns) -} - -/// One-to-one PromQL matching of grouped rows on equal label sets. Returns -/// the inner equi-join and the projection that applies the operator. -pub(super) fn grouped_binary( - left: &Schema, - right: &Schema, - operator: &BinaryOperator, -) -> Result<(Operator, Operator), Error> { - arithmetic(operator)?; - let (left_value, left_labels) = grouped_value(left)?; - let (right_value, right_labels) = grouped_value(right)?; - if left_labels.len() != right_labels.len() { - return Err(invalid("row binary inputs have different label sets")); - } - let width = left.fields.len(); - let keys = left_labels - .iter() - .map(|&l| { - let name = &left.fields[l].name; - let r = right_labels - .iter() - .copied() - .find(|&r| &right.fields[r].name == name) - .ok_or_else(|| invalid("row binary inputs have different label sets"))?; - let (a, b) = ( - Rc::new(QueryExpr::Column(l)), - Rc::new(QueryExpr::Column(width + r)), - ); - let equal = QueryExpr::Compare { - left: a.clone(), - op: CompareOpKind::Eq, - right: b.clone(), - }; - // A nullable label compares like PromQL's empty label: absent on both sides matches. - Ok(if left.fields[l].nullable || right.fields[r].nullable { - QueryExpr::BoolOr(vec![ - equal, - QueryExpr::BoolAnd(vec![QueryExpr::IsNull(a), QueryExpr::IsNull(b)]), - ]) - } else { - equal - }) - }) - .collect::, Error>>()?; - let predicate = Predicate(Rc::new(QueryExpr::BoolAnd(keys))); - let mut joined = left.fields.clone(); - joined.extend(right.fields.iter().cloned()); - let join = Operator::relational_join( - left.clone(), - right.clone(), - planner_types::pre_asap::JoinKind::Inner, - &predicate, - Arc::new(planner_types::post_asap::SummarySchema { - fields: joined, - time_index: None, - }), - )?; - let columns = left - .fields - .iter() - .enumerate() - .map(|(i, field)| { - let expression = if i == left_value { - binary( - operator, - Expression::Column(i), - Expression::Column(width + right_value), - ) - } else { - Expression::Column(i) - }; - (field.name.clone(), expression) - }) - .collect(); - let project = Operator::project(join.schema(), columns)?; - Ok((join, project)) -} - -fn binary(operator: &BinaryOperator, left: Expression, right: Expression) -> Expression { - Expression::Binary { - operator: operator.clone(), - left: Box::new(left), - right: Box::new(right), - } -} - /// Aggregate readouts of a maintained current-series population, as a chain. pub(super) fn population_aggregate( input: &Schema, diff --git a/crates/asap-physical-operators/tests/deployment_computation.rs b/crates/asap-physical-operators/tests/deployment_computation.rs index 532cd331..edef3121 100644 --- a/crates/asap-physical-operators/tests/deployment_computation.rs +++ b/crates/asap-physical-operators/tests/deployment_computation.rs @@ -348,20 +348,58 @@ fn exact_count_finalizes_to_declared_float_value() { ); } -// Comparisons need filter/bool semantics that `Binary` does not carry, so -// they fail at compile time instead of emitting 0/1 values. -#[test] -fn row_comparison_fails_closed() { - let mut dag = exact_dag("sum by (job) (sum_over_time(m[5m])) * 2"); +/// `dag` with its Binary operator replaced by `kind`. +fn with_kind(mut dag: PostAsapDag, kind: planner_types::pre_asap::BinaryOpKind) -> PostAsapDag { for node in &mut dag.nodes { if let PostAsapOperatorPayload::Binary { operator } = &mut node.payload { - operator.kind = planner_types::pre_asap::BinaryOpKind::Compare( - planner_types::pre_asap::CompareOpKind::Gt, - ); + operator.kind = kind.clone(); } } - let error = run(&dag, SAMPLES, 60_000).unwrap_err(); - assert!(error.contains("comparison"), "{error}"); + dag +} + +// A comparison Binary over grouped values keeps the groups whose comparison +// holds, with their value, on either side of the literal; `bool` yields 1 or 0. +#[test] +fn grouped_comparisons_filter_or_return_bool() { + use planner_types::pre_asap::{BinaryOpKind::*, CompareOpKind::Gt}; + // sum_over_time over 5m per job: api = 14, db = 5. + let right = exact_dag("sum by (job) (sum_over_time(m[5m])) * 10"); + let left = exact_dag("10 - sum by (job) (sum_over_time(m[5m]))"); + for (dag, expected) in [ + ( + with_kind(right.clone(), Compare(Gt)), + reference(&[("api", 14.)]), + ), + ( + with_kind(right, CompareBool(Gt)), + reference(&[("api", 1.), ("db", 0.)]), + ), + (with_kind(left, Compare(Gt)), reference(&[("db", 5.)])), + ] { + assert_eq!(run(&dag, SAMPLES, 60_000).unwrap(), expected); + } +} + +// A `bool` comparison Binary over per-series readouts matches one-to-one and +// drops the metric name; a filter fails closed. +#[test] +fn per_series_comparisons_filter_or_return_bool() { + use planner_types::pre_asap::{BinaryOpKind::*, CompareOpKind::*}; + let samples = counter("a", "api", 10., 10.) + .chain(counter("a", "db", 10., 10.)) + .chain(counter("b", "api", 5., 5.)) + .chain(counter("b", "db", 20., 20.)) + .collect::>(); + // rate: a{api} = a{db} = 50/300, b{api} = 25/300, b{db} = 100/300. + let dag = exact_dag("rate(a[5m]) / rate(b[5m])"); + // The readout keeps `__name__`, which rate drops, so a filter would keep it. + let error = run_series(&with_kind(dag.clone(), Compare(Gt)), &samples, 300_000).unwrap_err(); + assert!(error.contains("__name__"), "{error}"); + assert_eq!( + run_series(&with_kind(dag, CompareBool(Lt)), &samples, 300_000).unwrap(), + series(&[("api", "x", 0.), ("db", "x", 1.)]) + ); } /// [`execute`], returning per-series `(identity, value)` rows of the root, diff --git a/crates/asap-physical-operators/tests/promql_fallback.rs b/crates/asap-physical-operators/tests/promql_fallback.rs index c609d28f..f7625ab8 100644 --- a/crates/asap-physical-operators/tests/promql_fallback.rs +++ b/crates/asap-physical-operators/tests/promql_fallback.rs @@ -580,7 +580,6 @@ fn on_and_ignoring_select_the_matching_labels() { assert!(evaluate("a + on(job) b", &[("a", a), ("b", pair)], 60).is_err()); assert!(evaluate("a + on(job) b", &[("a", pair), ("b", b)], 60).is_err()); assert!(labeled("a + on(job) b", &[("a", pair), ("b", other)], 60).is_empty()); - assert!(promql_rows::with_series_identity(&parse("a + on(job) group_left b")).is_err()); } // without() groups by every label except the listed ones and the metric name. @@ -646,8 +645,8 @@ fn empty_labels_and_empty_sides_match_prometheus() { let pair: &[Sample] = &[("job=x,inst=1", 50, 1.), ("job=x,inst=2", 50, 2.)]; assert!(labeled("a + on(job) b", &[("b", pair)], 60).is_empty()); assert!(labeled("b + on(job) a", &[("b", pair)], 60).is_empty()); - // A non-literal scalar operand has no identity realization yet. - assert!(promql_rows::with_series_identity(&parse("a + scalar(b)")).is_err()); + // `time()` has no row realization yet. + assert!(promql_rows::with_series_identity(&parse("a - time()")).is_err()); } // Sums and averages use Prometheus' Kahan-Neumaier compensation, and an @@ -672,3 +671,328 @@ fn sums_and_averages_are_compensated_like_prometheus() { let huge = &[("a", 50, 1.7e308), ("b", 50, 1.7e308)]; assert_eq!(one("avg(m)", huge, 60), 1.7e308); } + +/// `(k=v,... sorted, value)` rows for readable expectations. +fn rows(pairs: &[(&str, f64)]) -> Vec<(String, f64)> { + let mut rows = pairs + .iter() + .map(|(spec, value)| (spec.to_string(), *value)) + .collect::>(); + rows.sort_by(|a, b| a.0.cmp(&b.0)); + rows +} + +/// `labeled`, with NaN values rendered comparable. +fn labeled_nan(query: &str, metrics: &[(&str, &[Sample])], at: i64) -> Vec<(String, String)> { + labeled(query, metrics, at) + .into_iter() + .map(|(labels, value)| (labels, format!("{value:?}"))) + .collect() +} + +const C: &[Sample] = &[ + ("job=x", 50, 10.), + ("job=y", 50, 20.), + ("job=w", 50, 0.), + ("job=n", 50, f64::NAN), +]; + +// A comparison with a scalar keeps the matching series with their value and +// metric name, whichever side the scalar is on; `bool` yields 1 or 0 for every +// series and drops the name. NaN compares unequal to everything. +#[test] +fn scalar_comparisons_filter_or_return_bool() { + let metrics = &[("a", C)]; + let kept = rows(&[("__name__=a,job=x", 10.), ("__name__=a,job=y", 20.)]); + assert_eq!(labeled("a > 5", metrics, 60), kept); + assert_eq!(labeled("5 < a", metrics, 60), kept); + assert_eq!( + labeled("a <= 10", metrics, 60), + rows(&[("__name__=a,job=w", 0.), ("__name__=a,job=x", 10.)]) + ); + assert_eq!( + labeled("a > bool 5", metrics, 60), + rows(&[("job=n", 0.), ("job=w", 0.), ("job=x", 1.), ("job=y", 1.)]) + ); + assert_eq!( + labeled("10 == bool a", metrics, 60), + rows(&[("job=n", 0.), ("job=w", 0.), ("job=x", 1.), ("job=y", 0.)]) + ); + // scalar() of no series is NaN. + assert_eq!(labeled("a != scalar(b)", metrics, 60).len(), 4); + assert!(labeled("a == scalar(b)", metrics, 60).is_empty()); + assert!(labeled("a > 5", &[], 60).is_empty()); + // Only `bool` drops the name, so only it can make label sets collide. + let equal: &[Sample] = &[("job=x", 50, 1.), ("__name__=b,job=x", 50, 2.)]; + assert_eq!(labeled("a > 0", &[("a", equal)], 60).len(), 2); + let error = evaluate("a > bool 0", &[("a", equal)], 60).unwrap_err(); + assert!(error.contains("same labelset"), "{error}"); +} + +// Vector comparisons match one-to-one like arithmetic. A filter keeps the +// left series, name included, unless `on` reduces its labels; `bool` drops the +// name. A left duplicate is an error only if more than one of it is kept. +#[test] +fn vector_comparisons_match_one_to_one() { + let metrics = &[("a", A), ("b", B)]; + assert_eq!( + labeled("a > b", metrics, 60), + rows(&[("__name__=a,job=x", 10.)]) + ); + assert_eq!( + labeled("a >= b", metrics, 60), + rows(&[("__name__=a,job=w", 0.), ("__name__=a,job=x", 10.)]) + ); + assert_eq!( + labeled("a > bool b", metrics, 60), + rows(&[("job=w", 0.), ("job=x", 1.)]) + ); + assert!(labeled("a < b", metrics, 60).is_empty()); + let a: &[Sample] = &[("job=x,inst=1", 50, 10.)]; + let b: &[Sample] = &[("job=x,inst=2", 50, 4.)]; + let metrics = &[("a", a), ("b", b)]; + assert_eq!( + labeled("a > on(job) b", metrics, 60), + rows(&[("job=x", 10.)]) + ); + assert_eq!( + labeled("a > ignoring(inst) b", metrics, 60), + rows(&[("__name__=a,job=x", 10.)]) + ); + let pair: &[Sample] = &[("job=x,inst=1", 50, 1.), ("job=x,inst=2", 50, 5.)]; + let metrics = &[("a", pair), ("b", b)]; + assert_eq!( + labeled("a > on(job) b", metrics, 60), + rows(&[("job=x", 5.)]) + ); + let error = evaluate("a > bool on(job) b", metrics, 60).unwrap_err(); + assert!(error.contains("many-to-one"), "{error}"); + let nan: &[Sample] = &[("job=x", 50, f64::NAN)]; + let metrics = &[("a", nan), ("b", nan)]; + assert_eq!(labeled("a == bool b", metrics, 60), rows(&[("job=x", 0.)])); + assert_eq!( + labeled_nan("a != b", metrics, 60), + vec![("__name__=a,job=x".into(), "NaN".into())] + ); +} + +const S: &[Sample] = &[ + ("job=x", 50, 1.), + ("job=y", 50, 2.), + ("job=z,inst=1", 50, 3.), +]; +const T: &[Sample] = &[ + ("job=x", 50, 10.), + ("job=w", 50, 20.), + ("job=z,inst=2", 50, 30.), +]; + +// Set operators match label sets many-to-many, ignoring the name by default, +// and return the original series unchanged. +#[test] +fn set_operators_match_label_sets() { + let metrics = &[("a", S), ("b", T)]; + assert_eq!( + labeled("a and b", metrics, 60), + rows(&[("__name__=a,job=x", 1.)]) + ); + assert_eq!( + labeled("a and on(job) b", metrics, 60), + rows(&[("__name__=a,job=x", 1.), ("__name__=a,inst=1,job=z", 3.)]) + ); + assert_eq!( + labeled("a and ignoring(inst) b", metrics, 60), + labeled("a and on(job) b", metrics, 60) + ); + assert_eq!( + labeled("a or b", metrics, 60), + rows(&[ + ("__name__=a,job=x", 1.), + ("__name__=a,job=y", 2.), + ("__name__=a,inst=1,job=z", 3.), + ("__name__=b,job=w", 20.), + ("__name__=b,inst=2,job=z", 30.), + ]) + ); + assert_eq!( + labeled("a or on(job) b", metrics, 60), + rows(&[ + ("__name__=a,job=x", 1.), + ("__name__=a,job=y", 2.), + ("__name__=a,inst=1,job=z", 3.), + ("__name__=b,job=w", 20.), + ]) + ); + assert_eq!( + labeled("a unless b", metrics, 60), + rows(&[("__name__=a,job=y", 2.), ("__name__=a,inst=1,job=z", 3.)]) + ); + assert_eq!( + labeled("a unless on(job) b", metrics, 60), + rows(&[("__name__=a,job=y", 2.)]) + ); + assert_eq!(labeled("a and on() b", metrics, 60).len(), 3); + // Empty sides, and duplicates on either side, which set operators allow. + let a_only = &[("a", S)]; + assert!(labeled("a and b", a_only, 60).is_empty()); + assert_eq!(labeled("a unless b", a_only, 60).len(), 3); + assert_eq!(labeled("b or a", a_only, 60).len(), 3); + let pair: &[Sample] = &[("job=x,inst=1", 50, 1.), ("job=x,inst=2", 50, f64::NAN)]; + assert_eq!( + labeled_nan("a and on(job) b", &[("a", pair), ("b", pair)], 60), + vec![ + ("__name__=a,inst=1,job=x".into(), "1.0".into()), + ("__name__=a,inst=2,job=x".into(), "NaN".into()), + ] + ); +} + +const MANY: &[Sample] = &[ + ("job=x,inst=1", 50, 2.), + ("job=x,inst=2", 50, 3.), + ("job=y,inst=1", 50, 4.), +]; +const ONE: &[Sample] = &[("job=x,team=t1", 50, 10.), ("job=y", 50, 100.)]; + +// group_left/group_right match many series to one; the result keeps the many +// side's labels plus the listed labels of the one side, which a missing label +// removes. A filter keeps the left value. +#[test] +fn group_modifiers_match_many_to_one() { + let metrics = &[("a", MANY), ("info", ONE)]; + assert_eq!( + labeled("a * on(job) group_left(team) info", metrics, 60), + rows(&[ + ("inst=1,job=x,team=t1", 20.), + ("inst=2,job=x,team=t1", 30.), + ("inst=1,job=y", 400.), + ]) + ); + assert_eq!( + labeled("info - on(job) group_right a", metrics, 60), + rows(&[ + ("inst=1,job=x", 8.), + ("inst=2,job=x", 7.), + ("inst=1,job=y", 96.) + ]) + ); + assert_eq!( + labeled("info > on(job) group_right a", metrics, 60), + rows(&[ + ("__name__=a,inst=1,job=x", 10.), + ("__name__=a,inst=2,job=x", 10.), + ("__name__=a,inst=1,job=y", 100.), + ]) + ); + assert_eq!( + labeled("a > bool ignoring(inst, team) group_left info", metrics, 60), + rows(&[ + ("inst=1,job=x", 0.), + ("inst=2,job=x", 0.), + ("inst=1,job=y", 0.) + ]) + ); + // Two "one" series for a match group, or two results with equal labels. + let two: &[Sample] = &[("job=x,team=t1", 50, 1.), ("job=x,team=t2", 50, 2.)]; + let error = evaluate( + "a * on(job) group_left info", + &[("a", MANY), ("info", two)], + 60, + ) + .unwrap_err(); + assert!(error.contains("duplicate series"), "{error}"); + let error = evaluate( + "info * on(job) group_right a", + &[("a", two), ("info", MANY)], + 60, + ) + .unwrap_err(); + assert!(error.contains("left hand-side"), "{error}"); + let named: &[Sample] = &[("job=x", 50, 1.), ("__name__=c,job=x", 50, 2.)]; + let error = evaluate( + "a * on(job) group_left info", + &[("a", named), ("info", ONE)], + 60, + ) + .unwrap_err(); + assert!(error.contains("unique matches"), "{error}"); + assert!(labeled("a * on(job) group_left info", &[("a", MANY)], 60).is_empty()); +} + +// A non-literal scalar applies like a literal; scalar-scalar arithmetic yields +// a scalar; and a literal applies to aggregated rows whose value has another name. +#[test] +fn scalar_operands_and_aggregates() { + let three: &[Sample] = &[("job=b", 50, 3.)]; + let metrics = &[("a", A), ("b", three)]; + assert_eq!( + labeled("a * scalar(b)", metrics, 60), + rows(&[("job=w", 0.), ("job=x", 30.), ("job=y", 60.)]) + ); + assert_eq!( + labeled("a > scalar(b)", metrics, 60), + rows(&[("__name__=a,job=x", 10.), ("__name__=a,job=y", 20.)]) + ); + assert_eq!(labeled("scalar(b) * 2", metrics, 60), rows(&[("", 6.)])); + assert_eq!( + labeled("scalar(b) > bool 2", metrics, 60), + rows(&[("", 1.)]) + ); + // scalar() of several series is NaN. + assert!(labeled("scalar(a) - 1", metrics, 60)[0].1.is_nan()); + assert_eq!( + labeled("sum by (job) (a) * 2", metrics, 60), + rows(&[("job=w", 0.), ("job=x", 20.), ("job=y", 40.)]) + ); + assert_eq!( + labeled("sum by (job) (a) > bool 5", metrics, 60), + rows(&[("job=w", 0.), ("job=x", 1.), ("job=y", 1.)]) + ); +} + +// Range functions other than last_over_time drop the metric name, so series +// that then share a label set are an error, as in Prometheus. +#[test] +fn range_functions_drop_the_name_and_reject_equal_label_sets() { + let equal: &[Sample] = &[ + ("job=x", 10, 1.), + ("job=x", 50, 2.), + ("__name__=b,job=x", 10, 1.), + ("__name__=b,job=x", 50, 4.), + ]; + let error = evaluate("rate(a[1m])", &[("a", equal)], 60).unwrap_err(); + assert!(error.contains("same labelset"), "{error}"); + assert_eq!( + labeled("last_over_time(a[1m])", &[("a", equal)], 60), + rows(&[("__name__=a,job=x", 2.), ("__name__=b,job=x", 4.)]) + ); + assert_eq!( + labeled("max_over_time(a[1m])", &[("a", &equal[..2])], 60), + rows(&[("job=x", 2.)]) + ); +} + +// Scalar-valued expressions are scalars too; `or vector(0)` fills an empty +// aggregate; a range function inside a subquery drops the name. +#[test] +fn scalar_expressions_or_vector_and_subquery_names() { + let three: &[Sample] = &[("job=b", 50, 3.)]; + let metrics = &[("a", A), ("b", three)]; + assert_eq!( + labeled("a + (scalar(b) * 2)", metrics, 60), + rows(&[("job=w", 6.), ("job=x", 16.), ("job=y", 26.)]) + ); + assert_eq!( + labeled("a + -scalar(b)", metrics, 60), + rows(&[("job=w", -3.), ("job=x", 7.), ("job=y", 17.)]) + ); + assert_eq!( + labeled("sum(a) or vector(0)", metrics, 60), + rows(&[("", 30.)]) + ); + assert_eq!(labeled("sum(a) or vector(0)", &[], 60), rows(&[("", 0.)])); + let counter: &[Sample] = &[("job=x", 0, 0.), ("job=x", 30, 3.), ("job=x", 60, 6.)]; + let result = labeled("last_over_time(rate(a[1m])[2m:1m])", &[("a", counter)], 60); + assert_eq!(result.len(), 1); + assert_eq!(result[0].0, "job=x"); +} From a6b7337d1bc147c0ec333daa30ce88ecf6cf5e6e Mon Sep 17 00:00:00 2001 From: zzylol Date: Wed, 30 Sep 2026 12:30:23 +0000 Subject: [PATCH 3/4] docs: record comparison and set operator coverage Co-Authored-By: Claude Opus 5.5 --- .../develop_docs/physical-compile-coverage.md | 63 +++++++++++++++++-- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/docs/develop_docs/physical-compile-coverage.md b/docs/develop_docs/physical-compile-coverage.md index 9a090ee2..4d6dc44c 100644 --- a/docs/develop_docs/physical-compile-coverage.md +++ b/docs/develop_docs/physical-compile-coverage.md @@ -116,7 +116,7 @@ Totals after this change: 19 Supported, 5 Partial, 5 Missing, 2 Backend. query's evaluation time. The raw rows must cover the window at that instant. Vector matching and `without` rewrite the series identity, so their results already lack `__name__`. Other results keep it; the query adapter still drops -it. +it. (Range functions drop it too since the comparison change below.) Totals are unchanged: 19 Supported, 5 Partial, 5 Missing, 2 Backend. @@ -138,6 +138,49 @@ compensation term would change the stored state layout. Its checked Totals after this change: 20 Supported, 4 Partial, 5 Missing, 2 Backend. +## Covered by comparisons and set operators + +The IR now distinguishes `bool` comparisons: `BinaryOpKind::CompareBool(op)` +beside the filtering `BinaryOpKind::Compare(op)`. The PromQL frontend emits it +for `bool`; it previously dropped the modifier. + +`Operator::series_binary` evaluates every PromQL binary operator, in the +Fallback and in query-time `Binary` nodes, following Prometheus' +`VectorBinop`, `VectorAnd`, `VectorOr`, and `VectorUnless`: + +- Arithmetic drops `__name__`. A comparison filter keeps the matched left + series with its value and name; with a scalar on the left it keeps the + vector's value. `bool` yields 1 or 0 and drops the name. NaN compares + unequal to everything. +- Operands may be vectors, literals, `scalar()`, or scalar-valued binary + expressions such as `-scalar(x)`. Two scalars yield a scalar. The + label-map `vector_binary` also treats `CompareBool` as its `bool` mode. +- One-to-one matching and `group_left`/`group_right` with included labels. + The "one" side must not repeat a match group; many-to-one results must be + unique. A left duplicate is an error only if more than one match is kept. +- `and`, `or`, and `unless` match label sets many-to-many, with `on` or + `ignoring`, and return the original series. +- Range functions other than `last_over_time` now drop `__name__` in the + Fallback. Series whose label sets become equal are an error, as in + Prometheus. Vector-scalar results are checked the same way. Inside a + subquery the inner function also drops the name, without that check, + because each series repeats across steps. + +A result is written in the left operand's schema. Without a series identity, +its label columns must hold every label the right side can contribute +(`or`, `group_right`, and `group_left` labels); otherwise `compile` rejects it. +So `sum(a) or vector(0)` compiles, but +`sum by (job) (a) * on(job) group_left(team) info` is rejected: the +aggregate's schema has no `team` column. + +| Row | Change | +|---|---| +| 1 | Comparisons, `bool`, set operators, `group_left`/`group_right`, `scalar()` operands, and literals over aggregates whose value has another name, such as `sum by (job) (a) * 2`. Still Partial. | +| 5 | Grouped `Binary` rows use the same operator instead of a relational join. A duplicate match group is now an error instead of a cross product. | +| 7 | Fallback, grouped `Binary`, and per-series `bool` comparisons. Per-series filter comparisons and set operators on `Binary` nodes fail closed: stored readouts keep `__name__` even where the range function drops it. Now Partial. | + +Totals after this change: 20 Supported, 5 Partial, 4 Missing, 2 Backend. + ## Remaining In order of backend usage: @@ -162,9 +205,7 @@ In order of backend usage: it returns NaN. It forces cumulative counts to be monotonic and returns NaN for fewer than two buckets. For q < 0 it returns -Inf; for q > 1, +Inf. - - Comparisons and set operators (`and`, `or`, `unless`); `group_left` and - `group_right`; arithmetic with a non-literal scalar, such as - `scalar(x)` or `time()`. + - `time()` and other scalar functions as operands. - Subquery operands other than one per-series function; implicit subquery resolution, which is a deployment default. - `@ start()` and `@ end()`, which need the range query's bounds in the @@ -172,8 +213,9 @@ In order of backend usage: - Other functions, such as `deriv`, `predict_linear`, `stddev_over_time`, `absent`, `label_replace`, and math functions. After these shapes are covered, the backend can delete rows 28 and 30. -2. Row 7: comparison filters and `bool` comparisons. This needs `return_bool` - in the `Binary` payload. `compile` currently rejects comparisons. +2. Row 7: per-series readouts must drop `__name__` where their range + function does, and check for equal label sets. Then filter comparisons + and set operators over them can compile. 3. Rows 25 and 27: constant weights and `EntityIdentity` items for precompute `SummaryAgg`. 4. Row 16: a label-map sketch-state readout, the counterpart of @@ -181,3 +223,12 @@ In order of backend usage: 5. Row 20: summary join, subtract, and delete. 6. Compensated stored exact `Sum` state, a state-layout change shared with the backend's stored-state decoding. +7. Non-finite literals (`NaN`, `Inf`) in compiled operators do not survive a + JSON round trip of the program. +8. `group_left` labels and `or`/`group_right` right-side labels onto + aggregated (label-column) rows. The logical output schema, which is the left + side's, has no column for them. +9. An equal-label-set check for inner subquery functions, per step. +10. `fill`, `fill_left`, and `fill_right` matching modifiers. The frontend + still ignores them. Rejecting them drops 27 corpus queries below the + lowering floor, so that change needs its own decision. From 50f4aef956947e435eea5a43e604a1fb4f895ff4 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:12:08 +0000 Subject: [PATCH 4/4] integrate: widen shared series identity to every PromQL binary operator The comparisons, set operators, and group modifiers compiled here share series identity through asap-types (moved there by #477), while promql_rows.rs no longer keeps its own copy. Accept every BinaryOp kind and group modifier in `with_promql_series_identity`, excluding only operands that read the evaluation timestamp. Taken from integration merge a9b8fdd. Co-Authored-By: Claude Opus 5.5 --- crates/types/src/pre_asap/schema.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/crates/types/src/pre_asap/schema.rs b/crates/types/src/pre_asap/schema.rs index fdac93de..959e37b5 100644 --- a/crates/types/src/pre_asap/schema.rs +++ b/crates/types/src/pre_asap/schema.rs @@ -209,18 +209,10 @@ pub fn with_promql_series_identity(root: &super::QueryExpr) -> Result Ok(()), QueryExpr::PromqlVectorFromScalar(child) if scalar_literal(child).is_some() => Ok(()), - QueryExpr::BinaryOp { - op: super::BinaryOpKind::Arithmetic(_), - lhs, - rhs, - vector_match, - } if vector_match.as_ref().is_none_or(|m| m.grouping.is_none()) - && ![&*lhs, &*rhs].into_iter().any(|side| { - matches!( - side.as_ref(), - QueryExpr::PromqlScalarFromVector(_) | QueryExpr::EvalTimestamp - ) - }) => + QueryExpr::BinaryOp { lhs, rhs, .. } + if ![&*lhs, &*rhs] + .into_iter() + .any(|side| matches!(side.as_ref(), QueryExpr::EvalTimestamp)) => { visit(Rc::make_mut(lhs))?; visit(Rc::make_mut(rhs))