feat(promql): lower unary negation as multiply-by-minus-one (#36) - #102
Merged
Merged
Conversation
`-expr` was rejected outright — the L2 PromQL path had no way to express "sign-flip every sample". Now that a scalar operand exists (#35), negation lowers as `expr * -1`: a `Mul` BinaryOp of the vector against `Scalar(-1)`, with no vector match. A constant-foldable operand (`-(10*1024*1024)`) collapses to a negated `Scalar` leaf instead. `Mul` is commutative, so operand order carries no hazard, and the L3 BinaryOp schema rule already takes the vector side — negation is label-preserving for free. Negation composes with everything: it lowers inside aggregate arguments (`sum(-m)`), binary ops (`a - -b`), set ops (`-a or -b`), and nests (`- -m`), because the aggregate/binary walkers already recurse through the shared `walk`. The five cases the old `unary_negation_is_rejected__GAP` test pinned as rejected now lower correctly; that test is replaced by `unary_negation_lowers_as_multiply_by_minus_one` plus constant-fold and double-negation coverage, two exact-tree e2e pins, and a row in the nesting-contract table in docs/promql-lowering.md. Closes #36 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #36.
Problem
PromQL unary negation (
-expr) was rejected outright:-rate(m[5m]),-some_metric,sum(-m),a - -ball errored. The root cause was that the L2 PromQL path had no scalar/negate node to express "multiply this vector by -1" — negation is effectivelyx * -1, which needs a scalar operand.Fix
That scalar operand landed in #35 (
QueryExpr::Scalar+ scalarBinaryOpoperands), so this is now a small, localized change to the one rejection site in the PromQL walker:expr * -1: aMulBinaryOpof the (label-preserving) vector againstScalar(-1), no vector match.Mulis commutative, so operand order carries no hazard, and the existing L3BinaryOpschema rule already follows the non-scalar side — so negation is label-preserving with no extra handling.-(10*1024*1024)) → a negatedScalarleaf, matching how bare literals already fold.Because the aggregate and binary walkers already recurse through the shared
walk(from the #27 nesting work), negation composes everywhere for free: inside aggregate arguments (sum(-m)), binary/set ops (a - -b,-a or -b), and nested (- -m).Tests
The five cases the old
unary_negation_is_rejected__GAPtest pinned as rejected now lower correctly. That test is replaced by:unary_negation_lowers_as_multiply_by_minus_one— all five original cases lower; the root-some_metricis checked structurally (Scan * Scalar(-1), no vector match,[ts, value]schema preserved), andsum(-m)is verified to nest the negation under the aggregate.unary_negation_of_constant_folds_to_scalaranddouble_unary_negation_nests— the fold and nesting edge cases.binary_op.rs(q36_unary_negation_is_multiply_by_minus_one,q36_sum_of_negation_nests).docs/promql-lowering.mdnesting-contract table; scrubbed the two stale "negation rejected" references inpromql.rs/error.rs.The full PromQL corpus suite (awesome-prometheus-alerts + promql corpora) still passes — no rejection-count assertion shifted.
Notes
cargo test --workspace— 340 passing, 0 failures;cargo clippy --all-targetsclean; production source (promql.rs,error.rs) has zero net-newcargo fmtdrift vs main.🤖 Generated with Claude Code