Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/frontend-metricsql/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ path = "tests/main.rs"
[dev-dependencies]
asap-types = { path = "../types", features = ["test-support"] }
asap-frontend-metricsql = { path = ".", features = ["test-support"] }
asap-frontend-promql = { path = "../frontend-promql" }
14 changes: 8 additions & 6 deletions crates/frontend-metricsql/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,15 @@ impl Lowerer<'_> {
Expr::NumberLiteral(_) => {
Err(unsupported("scalar root requires lower_metricsql_query"))
}
// Vector negation is `x * -1` (as in the PromQL front end).
Expr::UnaryOperator(e) => Ok(U::PromqlScalarOp {
// Vector negation negates each sample and drops the metric name,
// lowered exactly as the PromQL front end does.
Expr::UnaryOperator(e) => Ok(U::PromqlMap {
child: Rc::new(self.lower(&e.expr)?),
scalar: UnresolvedScalar::Literal(ScalarValue::Float64(-1.0)),
op: BinaryOpKind::Arithmetic(ArithmeticOpKind::Mul),
scalar_left: false,
return_bool: false,
sample: UnresolvedScalar::Negative {
expr: Box::new(UnresolvedScalar::Column(ColumnRef::SampleValue)),
semantics: ExprSemantics::Promql,
},
drop_metric_name: true,
}),
Expr::BinaryOperator(e) => self.binary(e),
Expr::Parens(e) if e.expressions.len() == 1 => self.lower(&e.expressions[0]),
Expand Down
13 changes: 13 additions & 0 deletions crates/frontend-metricsql/tests/lowering_semantics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,16 @@ fn workload_lowering_uses_the_declared_sample_interval() {
});
lower_metricsql_query_workload(&at_rest).unwrap();
}

/// Vector negation lowers to the same IR as PromQL's: a sample-value
/// `Negative` that drops the metric name, not a multiplication by -1.
#[test]
fn vector_negation_lowers_as_in_promql() {
let query = "-rate(up[5m])";
let metricsql =
lower_metricsql_query_workload(&workload(QueryLanguage::MetricsQL, query)).unwrap();
let promql =
asap_frontend_promql::lower_promql_query_workload(&workload(QueryLanguage::PromQL, query))
.unwrap();
assert_eq!(metricsql, promql);
}
19 changes: 8 additions & 11 deletions crates/frontend-promql/tests/promql_conformance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ fn has<F: Fn(&AggIntent) -> bool>(e: &OperatorNode, pred: F) -> bool {
intents(e).iter().any(pred)
}

/// Whether the tree contains a `Mul`-by-`ScalarExpr(-1)` anywhere — the shape unary
/// Whether the tree contains a `ScalarExpr::Negative` anywhere — the shape unary
/// negation lowers to (issue #36).
fn negates_via_scalar(e: &OperatorNode) -> bool {
fn negative(expr: &ScalarExpr) -> bool {
Expand Down Expand Up @@ -538,11 +538,11 @@ fn comparison_bool_modifier_returns_zero_or_one() {
}

#[test]
fn unary_negation_lowers_as_multiply_by_minus_one() {
// SEMANTICS (PromQL, issue #36): `-expr` flips the sign of every sample.
// Now that a scalar operand exists (#35), it lowers as `expr * -1` — a `Mul`
// BinaryOp of the (label-preserving) vector against `ScalarExpr(-1)`. These are
// the five cases the old `__GAP` test pinned as rejected.
fn unary_negation_lowers_as_a_negative_projection() {
// SEMANTICS (PromQL, issue #36): `-expr` flips the sign of every sample and
// drops the metric name. It lowers as a projection whose value column is
// `ScalarExpr::Negative`. These are the five cases the old `__GAP` test
// pinned as rejected.
for q in [
"-rate(http_errors_total[5m])",
"-some_metric",
Expand All @@ -551,11 +551,8 @@ fn unary_negation_lowers_as_multiply_by_minus_one() {
"sum(-node_cpu_seconds_total)",
] {
let qe = ok(q);
// A `Mul`-by-`-1` against a `ScalarExpr(-1)` appears somewhere in every tree.
assert!(
negates_via_scalar(&qe),
"no `* -1` negation found in {q}: {qe:?}"
);
// A `Negative` appears somewhere in every tree.
assert!(negates_via_scalar(&qe), "no negation found in {q}: {qe:?}");
}

let negated = ok("-some_metric");
Expand Down
9 changes: 4 additions & 5 deletions crates/integration-tests/tests/binary_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,10 @@ fn q21_div_two_sum_by_job() {
);
}

// #36 — unary negation lowers as `expr * -1`: a Mul BinaryOp of the vector
// against a `ScalarExpr(-1)` leaf, no vector match. The vector side keeps
// its schema.
// #36 — unary negation lowers as a projection whose value is
// `ScalarExpr::Negative`, no vector match. The vector side keeps its schema.
#[test]
fn q36_unary_negation_is_multiply_by_minus_one() {
fn q36_unary_negation_is_a_negative_projection() {
let root = lower("-some_metric");
let NonASAPOp::Project { cols, child, .. } = root.expect_non_asap() else {
panic!()
Expand All @@ -282,7 +281,7 @@ fn q36_unary_negation_is_multiply_by_minus_one() {
}

// #36 — negation nested inside an aggregate argument (issue #27 nesting):
// `sum(-m)` → Aggregate{Sum} over the `m * -1` BinaryOp.
// `sum(-m)` → Aggregate{Sum} over the negating projection.
#[test]
fn q36_sum_of_negation_nests() {
let root = lower("sum(-node_cpu_seconds_total)");
Expand Down
Loading