feat(promql): support without(...) grouping via a GroupKeys exclusion mode (#39) - #105
Merged
Merged
Conversation
…GroupKeys (#39) `sum without (instance) (...)` was rejected outright: `without` groups by every label EXCEPT those listed, and the complement can't be enumerated under an open usage-derived schema. But it doesn't need to be — the excluded labels are named (so the Binder seeds and resolves them), and the kept (complement) set is resolvable at runtime. So the honest representation stores the excluded positions and defers the complement. - L3 `GroupKeys` gains a `by`/`without` mode (was a bare `Vec<ColumnId>` newtype). Constructors `by`/`without`, accessors `is_without`/`keys`, and the Deref/From/IntoIterator/PartialEq impls preserve the existing slice-like API so the ~150 `by: vec![..].into()` call sites are untouched. Custom serde keeps the `by` wire format a bare array; `without` serialises as `{"without": [...]}`. - Schema derivation branches: a `without` aggregate keeps every input label except the excluded ones (and ts/value), appends the agg column, and stays OPEN (the kept set is runtime-only) — unlike `by`, which freezes to closed. `without()` (empty exclusion) is cross-series, never the per-series-global path. - L2 `Aggregate` gains a `without` flag; the PromQL front end resolves the excluded labels and `mark_without` flips the outer aggregate to the exclusion form. `without` on topk/bottomk/limitk (a ranking/sampling, not a reduction) is rejected rather than silently treated as `by`. - SQL is unaffected (GROUP BY is always inclusion → `without: false`). Tests: GroupKeys by/without semantics + serde; the without schema derivation (kept labels minus excluded, stays open); conformance (the grouping is the exclusion form, schema open) + the topk-without rejection; an exact-tree e2e pin; and the three former rejection tests flipped to passing. Docs updated (Binder section + nesting contract). Closes #39 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 #39.
The idea
sum without (instance) (…)was rejected:without(labels)groups by every label except those listed, and the complement can't be enumerated under an open usage-derived schema (we don't know a metric's full label set). But it doesn't need to be enumerated at lowering time:bylabels;So the honest representation stores the excluded positions and defers the complement — the output schema of a
withoutaggregate stays open, whereas abyaggregate freezes it to closed.Changes
L3
GroupKeysgains aby/withoutmode (it was a bareVec<ColumnId>newtype). The stored keys are kept labels forby, excluded labels forwithout. TheDeref/From/FromIterator/IntoIterator/PartialEq<Vec>impls are preserved, so the ~150 existingby: vec![..].into()construction sites compile unchanged — only the 7 explicitGroupKeys(..)sites becameGroupKeys::by(..). Custom serde keeps thebywire format a bare array (backward-compatible);withoutserialises as{"without": [...]}.Schema derivation branches on the mode. A
without(excluded)aggregate emits every input label column except the excluded positions (and ts/value), then the aggregate column, and stays open with no unique key. A subtle correctness point:without ()(empty exclusion = group by all labels) is cross-series, so it must not take the per-series-global path thatby.is_empty()selects — the guard isby.is_empty() && !by.is_without().Threading. L2
Aggregategains awithoutflag. The PromQL front end'sresolve_groupnow returns(keys, without)and no longer rejects the exclusion form;mark_withoutflips the outer aggregate's flag as a post-step (avoiding a param through ~23 builder call sites). SQL is untouched —GROUP BYis always inclusion (without: false).Rejections preserved where the model doesn't extend.
withoutontopk/bottomk(→Sort.partition_by) andlimitk/limit_ratio(→Sample) would need without-partitioning; those are rejected cleanly rather than silently lowered as abygrouping.Tests
GroupKeysby-vs-without semantics + serde (bare array forby, tagged object forwithout, round-trip).sum without (instance)over[ts, value, instance, job]→ keepsjob, dropsinstance, stays open, no unique key.sum_without_groups_by_the_complement(exclusion form + open schema) andwithout_on_topk_is_rejected.without_grouping_lowers_to_the_exclusion_form(over a per-series rate).q39_sum_without_instance_over_rate.promql_conformance,promql_lowering, the awesome-prometheus-alerts corpus GAP) are flipped to passing.Notes
asap-ir/asap-l2/ both front ends. Independent of other open work.cargo test --workspace— 0 failures;cargo clippy --all-targetsclean; production source (query_expr.rs,column_resolution.rs,relational.rs,lower.rs,promql.rs) carries zero net-newcargo fmtdrift vs main (two test files keep their pervasive compact-destructure convention).🤖 Generated with Claude Code