feat(promql): lower unary negation as multiply-by-minus-one (#36)

crates/e2e/tests/binary_op.rs

@@ -226,3 +226,35 @@ fn q21_div_two_sum_by_job() {
         expected,
     );
 }
+
+// #36 — unary negation lowers as `expr * -1`: a Mul BinaryOp of the vector
+//   against Scalar(-1), no vector match. The vector side keeps its schema.
+#[test]
+fn q36_unary_negation_is_multiply_by_minus_one() {
+    let expected = QueryExpr::BinaryOp {
+        op: BinaryOpKind::Arith(ArithOp::Mul),
+        lhs: Box::new(scan("some_metric", &[])),
+        rhs: Box::new(QueryExpr::Scalar(-1.0)),
+        vector_match: None,
+    };
+    assert_eq!(lower("-some_metric"), expected);
+}
+
+// #36 — negation nested inside an aggregate argument (issue #27 nesting):
+//   `sum(-m)` → Aggregate{Sum} over the `m * -1` BinaryOp.
+#[test]
+fn q36_sum_of_negation_nests() {
+    let expected = QueryExpr::Aggregate {
+        by: vec![].into(),
+        aggs: vec![AggIntent::Sum { col: None }],
+        output_names: vec!["".into()],
+        having: None,
+        child: Box::new(QueryExpr::BinaryOp {
+            op: BinaryOpKind::Arith(ArithOp::Mul),
+            lhs: Box::new(scan("node_cpu_seconds_total", &[])),
+            rhs: Box::new(QueryExpr::Scalar(-1.0)),
+            vector_match: None,
+        }),
+    };
+    assert_eq!(lower("sum(-node_cpu_seconds_total)"), expected);
+}

crates/frontend-promql/src/error.rs

@@ -16,8 +16,8 @@ pub enum PromqlError {
     UnsupportedFunction(String),
     /// A PromQL aggregation operator (`sum`, `topk`, …) not supported.
     UnsupportedAggregateOp(String),
-    /// A structural feature (offset / `@` / `without` / unary negation) not
-    /// supported in this version.
+    /// A structural feature (offset / `@` / `without`) not supported in this
+    /// version.
     UnsupportedFeature(String),
     /// A required function / aggregator argument was missing.
     MissingArgument(String),

crates/frontend-promql/src/promql.rs

@@ -210,14 +210,21 @@ fn walk(expr: &Expr) -> Result<L2> {
         Expr::Binary(bin) => walk_binary(bin),
         Expr::Paren(p) => walk(&p.expr),
         // `UnaryExpr` is built only by negation (`Neg`); unary `+` is folded to
-        // identity and `-<literal>` to a negated `NumberLiteral`, so this always
-        // wraps a vector expression whose samples must be sign-flipped. The L2
-        // PromQL path has no scalar/negate node to express that (there's no
-        // `-1 * x`, since `walk` rejects bare scalar operands), so reject it
-        // rather than silently dropping the sign and computing `+expr`.
-        Expr::Unary(_) => Err(LoweringError::UnsupportedFeature(
-            "unary negation (`-expr`): no negate/scalar node in the L2 PromQL path".into(),
-        )),
+        // identity and `-<literal>` to a negated `NumberLiteral`, so this wraps a
+        // sub-expression whose samples must be sign-flipped. Now that a scalar
+        // operand exists (#35), express it as `x * -1` — a constant-foldable
+        // operand (`-(10*1024)`) collapses to a negated `Scalar` leaf; anything
+        // else is a vector, sign-flipped by a `Mul` against `Scalar(-1)`. `Mul`
+        // is commutative, so operand order carries no hazard (#36).
+        Expr::Unary(u) => match num_expr(&u.expr) {
+            Ok(v) => Ok(L2::Scalar(-v)),
+            Err(_) => Ok(L2::BinaryOp {
+                op: BinaryOpKind::Arith(ArithOp::Mul),
+                lhs: Box::new(walk(&u.expr)?),
+                rhs: Box::new(L2::Scalar(-1.0)),
+                vector_match: None,
+            }),
+        },
         Expr::Subquery(sq) => Ok(L2::PromQLSubquery {
             range: sq.range,
             resolution: sq.step,
@@ -364,10 +371,10 @@ fn walk_aggregate(agg: &AggregateExpr) -> Result<L2> {
     // elsewhere (`sum(histogram_quantile(0.9, …))`). Lower it recursively with
     // the same `walk` used at the top level, then wrap it in the outer
     // aggregation. This is the path that lifts the old two-level limit to
-    // arbitrary function nesting (issue #27). If the inner expression is itself
-    // unsupported (e.g. unary negation), `walk` surfaces that error, so a
-    // genuinely unsupported query is still cleanly rejected rather than
-    // mislowered.
+    // arbitrary function nesting (issue #27) — a negated argument (`sum(-m)`)
+    // lowers here too (#36). If the inner expression is itself unsupported (e.g.
+    // an `offset` modifier), `walk` surfaces that error, so a genuinely
+    // unsupported query is still cleanly rejected rather than mislowered.
     let child = walk(&agg.expr)?;
     build_over_subtree(outer, keys, child)
 }

crates/frontend-promql/tests/promql_conformance.rs

@@ -129,6 +129,27 @@ fn has<F: Fn(&AggIntent) -> bool>(e: &QueryExpr, pred: F) -> bool {
     intents(e).iter().any(pred)
 }
 
+/// Whether the tree contains a `Mul`-by-`Scalar(-1)` anywhere — the shape unary
+/// negation lowers to (issue #36).
+fn negates_via_scalar(e: &QueryExpr) -> bool {
+    let is_neg_one = |q: &QueryExpr| matches!(q, QueryExpr::Scalar(v) if (*v + 1.0).abs() < 1e-12);
+    match e {
+        QueryExpr::BinaryOp { op, lhs, rhs, .. } => {
+            (*op == BinaryOpKind::Arith(ArithOp::Mul) && (is_neg_one(lhs) || is_neg_one(rhs)))
+                || negates_via_scalar(lhs)
+                || negates_via_scalar(rhs)
+        }
+        QueryExpr::Aggregate { child, .. }
+        | QueryExpr::Sort { child, .. }
+        | QueryExpr::Limit { child, .. }
+        | QueryExpr::TimeRange { child, .. }
+        | QueryExpr::Subquery { child, .. }
+        | QueryExpr::Filter { child, .. }
+        | QueryExpr::Project { child, .. } => negates_via_scalar(child),
+        _ => false,
+    }
+}
+
 // ─────────────────────────────────────────────────────────────────────────────
 // A. Selectors & label matchers           (basics §"Instant/Range Vector
 //    Selectors"; selectors.test)
@@ -509,19 +530,73 @@ fn vector_comparison_filters() {
 }
 
 #[test]
-fn unary_negation_is_rejected__GAP() {
-    // SEMANTICS (PromQL): `-expr` flips the sign of every sample (and `-rate(…)`
-    // negates the rate). With no negate/scalar node in the L2 PromQL path we
-    // can't model that, so it's rejected rather than silently lowered as `+expr`
-    // (which would compute the wrong result). `-<literal>` folds into the
-    // literal at parse time and is caught by the bare-scalar rejection instead.
-    let _ = rejected("-rate(http_errors_total[5m])");
-    let _ = rejected("-some_metric");
-    let _ = rejected("-metric_a or -metric_b");
-    // Negation nested inside a larger expression propagates the rejection,
-    // rather than lowering the rest with the inner sign silently dropped.
-    let _ = rejected("http_requests_total - -http_errors_total");
-    let _ = rejected("sum(-node_cpu_seconds_total)");
+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 `Scalar(-1)`. These are
+    // the five cases the old `__GAP` test pinned as rejected.
+    for q in [
+        "-rate(http_errors_total[5m])",
+        "-some_metric",
+        "-metric_a or -metric_b",
+        "http_requests_total - -http_errors_total",
+        "sum(-node_cpu_seconds_total)",
+    ] {
+        let qe = ok(q);
+        // A `Mul`-by-`-1` against a `Scalar(-1)` appears somewhere in every tree.
+        assert!(negates_via_scalar(&qe), "no `* -1` negation found in {q}: {qe:?}");
+    }
+
+    // `-some_metric` at the root: `Scan * Scalar(-1)`, schema follows the vector.
+    let QueryExpr::BinaryOp { op, lhs, rhs, vector_match } = &ok("-some_metric") else {
+        panic!("expected a BinaryOp for `-some_metric`");
+    };
+    assert_eq!(*op, BinaryOpKind::Arith(ArithOp::Mul));
+    assert!(matches!(lhs.as_ref(), QueryExpr::Scan { .. }), "vector on the left");
+    assert!(
+        matches!(rhs.as_ref(), QueryExpr::Scalar(v) if (*v + 1.0).abs() < 1e-12),
+        "negation multiplies by Scalar(-1), got {rhs:?}"
+    );
+    assert!(vector_match.is_none(), "scalar negation carries no vector match");
+    // Label-preserving: the schema is the vector operand's, unchanged.
+    let schema = ok("-some_metric").output_schema().unwrap();
+    assert_eq!(
+        schema.columns.iter().map(|c| c.name.as_str()).collect::<Vec<_>>(),
+        vec!["ts", "value"],
+    );
+
+    // `sum(-m)` — the negation lowers inside the aggregate argument (issue #27
+    // nesting), so the outer node is the `Sum` aggregate over the `Mul`.
+    let QueryExpr::Aggregate { aggs, child, .. } = &ok("sum(-node_cpu_seconds_total)") else {
+        panic!("expected an outer Aggregate for `sum(-m)`");
+    };
+    assert!(matches!(aggs.as_slice(), [AggIntent::Sum { .. }]));
+    assert!(matches!(child.as_ref(), QueryExpr::BinaryOp {
+        op: BinaryOpKind::Arith(ArithOp::Mul), ..
+    }));
+}
+
+#[test]
+fn unary_negation_of_constant_folds_to_scalar() {
+    // `-(10*1024*1024)` — the operand is constant-foldable, so negation collapses
+    // to a single negated `Scalar` leaf (no `BinaryOp`), just like a bare literal.
+    assert!(matches!(
+        ok("-(10*1024*1024)"),
+        QueryExpr::Scalar(v) if (v + 10_485_760.0).abs() < 1e-6
+    ));
+}
+
+#[test]
+fn double_unary_negation_nests() {
+    // `- -some_metric` — negation of a negation: `(m * -1) * -1`. Both levels
+    // lower; the value is unchanged but the structure is faithfully nested.
+    let QueryExpr::BinaryOp { op, lhs, .. } = &ok("- -some_metric") else {
+        panic!("expected outer BinaryOp for `- -some_metric`");
+    };
+    assert_eq!(*op, BinaryOpKind::Arith(ArithOp::Mul));
+    assert!(matches!(lhs.as_ref(), QueryExpr::BinaryOp {
+        op: BinaryOpKind::Arith(ArithOp::Mul), ..
+    }), "inner negation nests under the outer one");
 }
 
 #[test]

docs/promql-lowering.md

@@ -172,13 +172,13 @@ mislowered) — each row is pinned by a named test.
 | Range function over a sub-query (#42) | `max_over_time(rate(m[5m])[1h:])` | `over_time_of_subquery_reduces_per_series` |
 | Nested sub-queries (range fn over range fn over range fn; default resolution) | `max_over_time(deriv(rate(distance_covered_total[5s])[30s:5s])[10m:])` (Prometheus docs example) | `nested_subquery_from_prometheus_docs`; e2e `q27_nested_subquery_prometheus_docs_example` |
 | Outer group key absent from a nested aggregate's (closed) output (#53) | `sum(sum by (k)(m)) by (j)` — `j` provably absent → dropped per PromQL's absent-label grouping semantics | `outer_group_key_absent_from_nested_aggregate_is_dropped`; e2e `q53_outer_group_key_absent_from_nested_aggregate` |
+| Unary negation, anywhere in a nest (#36) | `sum(-m)` → `expr * -1` (`Mul` against `Scalar(-1)`); `-(const)` folds to a negated `Scalar` | `unary_negation_lowers_as_multiply_by_minus_one`; e2e `q36_sum_of_negation_nests` |
 | SQL derived tables / inline views (#29) | `SELECT … FROM (SELECT … GROUP BY …) t`, incl. aggregate-over-aggregate | `derived_table_aggregate_over_aggregate_nests` (`sql_lowering.rs`) |
 
 ### Rejected cleanly
 
 | Shape | Why | Pinned by |
 |---|---|---|
-| Unary negation anywhere in a nest — `sum(-m)` | no scalar-negate in the L2 PromQL path yet (issue #36) | `unary_negation_is_rejected__GAP` |
 | `without(...)` grouping | a usage-derived (open) schema can't enumerate the label complement (issue #39) | binder rejection (see "Why the Binder is its own pass") |
 | SQL subquery-valued **predicate** expressions — scalar `x > (SELECT …)`, `IN (SELECT …)`, `EXISTS` / `NOT EXISTS` / `NOT IN`, correlated or not | the v1 decision on #27's open question: these need a subquery node in the L2 expression IR + a correlated-vs-uncorrelated representation choice; rejected until that lands (derived tables in `FROM` are the supported nesting shape) | `scalar_subquery_in_predicate_is_rejected`, `semi_join_is_rejected_not_mislowered`, `exists_subquery_in_predicate_is_rejected` (`sql_lowering.rs`) |