feat(promql): lower limitk / limit_ratio as series sampling (#86) - #95
Merged
Merged
Conversation
`limitk`/`limit_ratio` parsed but were rejected (UnsupportedAggregateOp).
They are series-*sampling selection* operators — a subset of whole series kept
unchanged, deterministic-but-unordered — so they need a dedicated node, not
`topk`'s `Sort → Limit` (which would rank by value and change *which* series
survive).
L3 (asap-ir):
- `QueryExpr::Sample { by, kind, child }` + `SampleKind::{LimitK(k),
LimitRatio(r)}`. Schema derivation is pass-through (a subset of whole series
→ the child's schema and row-uniqueness are preserved), grouped with
Filter/Sort/Limit. Added to the `canonicalize` children walk.
L2 (asap-l2):
- `QueryExpr::Sample { keys, kind, input }` + walk/leaf_source arms and the
converter arm (resolves `keys` positionally, passes the child through).
- Binder seeds the `Sample.keys` grouping labels, like group-by keys.
Front end (asap-frontend-promql):
- `T_LIMITK` → `Sample{LimitK(k)}` (k via `count_param`); `T_LIMIT_RATIO` →
`Sample{LimitRatio(r)}` (`ratio_param` rejects non-finite and clamps to
[-1, 1] like Prometheus; a negative ratio's complement is preserved). Routed
through the generic base construction (label-preserving, no reducing
aggregate) so a bare selector is sampled directly and a range-vector arg
reduces per series first. `limitk by (group)` grouping works.
A dynamic k/ratio (`limitk(scalar(foo), …)`, `limit_ratio(time()%17/17, …)`)
isn't a compile-time constant, so `num_param` can't fold it — those stay
rejected, as does a NaN.
Tests: conformance §S — limitk/limit_ratio → Sample with the right kind and a
preserved schema; negative-ratio complement + out-of-range clamp; `limitk by`
grouping and preservation under `count(… and …)` (the corpus case that turns
on series identity); dynamic/NaN rejection. Flipped the
`limitk_and_limit_ratio_are_rejected__GAP` pin.
Co-Authored-By: Claude Opus 4.8 <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
1 participant
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 #86.
What
limitk(k, v)andlimit_ratio(r, v)parsed but were rejected asUnsupportedAggregateOp(33 corpus rejections). They are series-sampling selection operators — they keep a subset of whole series unchanged (deterministic but unordered), not a ranking or a reduction — so they get a dedicated IR node rather than being forced intotopk'sSort → Limit(which would rank by value and silently change which series survive).How
L3 (asap-ir)
QueryExpr::Sample { by, kind, child }withSampleKind::{ LimitK(usize), LimitRatio(f64) }. Schema derivation is pass-through — a subset of whole series preserves the child's schema and row-uniqueness — so it's grouped withFilter/Sort/Limit. Added to thecanonicalizechild walk.L2 (asap-l2)
Sample { keys, kind, input }node +walk/leaf_source/converter arms (keys resolve positionally; the child passes through). The binder seedsSample.keyslike group-by keys.Front end (asap-frontend-promql)
T_LIMITK→Sample{ LimitK(k) }(count_param);T_LIMIT_RATIO→Sample{ LimitRatio(r) }. Newratio_paramrejects non-finite ratios and clamps to[-1, 1](Prometheus semantics); a negative ratio (complementary fraction) is preserved, not normalised away. Routed through the generic base construction — a bare selector is sampled directly (label-preserving, no reducing aggregate), a range-vector argument reduces per series first.limitk by (group)grouping works.Honest partial support
A dynamic k/ratio (
limitk(scalar(foo), …),limit_ratio(time() % 17 / 17, …)) isn't a compile-time constant, sonum_paramcan't fold it — those stay rejected (as doeslimitk(NaN, …)), rather than being mislowered. Constant forms lower.Tests
Conformance §S:
limitk/limit_ratio→Samplewith the rightSampleKindand a preserved(ts, value)schema.[-1, 1].limitk by (group)carries its grouping;count(limitk(2, v) and v)still lowers — the exact corpus case that turns on the identity of the surviving series (whichtopkwould have broken).Flipped the
limitk_and_limit_ratio_are_rejected__GAPpin to positive. Full workspace suite green (corpus totality ratchet picks up the 33 newly-lowering queries); clippy clean.