feat(query): count_over_time binds to FrequencyEstimate (CMS / CountSketch) - #265
Merged
Merged
Conversation
…ketch)
PromQL's `count_over_time(metric[range])` is the per-series sample-
count idiom — exactly what a heap-less CMS or CountSketch estimates.
Pre-PR, the analyzer mapped it to `AggIntent::Count{Exact}` (via the
un-grouped Count carve-out in `lower::convert`), got `None` back
from `capability_for`, and surfaced as
`UnsupportedAggIntent("count")` — routing the query to the cold
tier even when a CMS / CountSketch policy was registered for the
metric.
The fix adds a dedicated `AggFunc::Frequency` variant that bypasses
the un-grouped Count carve-out:
- `walk_call_to_op` in `query_parser::promql` maps
`"count_over_time"` to `AggFunc::Frequency` when there's no
outer `count(...)` (still `CountDistinct`) and no outer
`topk(...)` (still `Count` — the topk wrapper expects a
count-shaped inner).
- `agg_func_to_intents` and `to_sketch_op` route the new variant
through `default_frequency()` → `AggIntent::Frequency{Epsilon}`.
- The reducer's `function_to_family` recognizes
`"count_over_time"` as `QueryFamily::FrequencyEstimate`.
Refreshed tests:
- `is_asap_tier_answerable_true_for_count_over_time` (was
`_false_for_unsupported` — invariant flipped).
- `count_over_time_binds_to_frequency_estimate` (was
`_is_unsupported_at_exact_accuracy` — now asserts
`Capability::FrequencyEstimate(Any)` directly).
- `analyzer_parity_18_query_corpus` GOLDEN block — Q08 ctrl
flipped from `MISS(UnsupportedAggIntent("count"))` to
`OK [cap=FrequencyEstimate(Any) fn=count_over_time]`.
The existing `count(metric)` distinct-counting and
`count by (...) (count_over_time(...))` paths are unaffected — both
still route through `AggFunc::CountDistinct` → HLL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 16, 2026
…266) PR #265 wired `count_over_time(metric[range])` to `Capability::FrequencyEstimate(Any)`. This PR consumes that surface: Tests 6 + 7 promote from soft-check (assert `status` field exists) to strict-success (assert `status == "success"` + non-empty result vector). Two underlying fixes surfaced when wiring this up: 1. **`build_count_min_export` was hardcoding wire `rows`/`cols` to 0** — the policy_fp content match keys on `parameters.{d, w}` against the DP's wire dimensions, so heap-less CMS sids were always registering with `policy_fp = UNSET` (unreachable through `sids_for_policy`). Same bug as the heap-bearing helpers had before PR #258 fixed them. Added `wire_rows: i32, wire_cols: i32` parameters mirroring `build_cms_with_heap_msgpack_export`'s shape. 2. **`asap_tier_result_to_query_result(.., is_range_query=true)` returned Matrix for instant queries with range-bound inners** — `execute(&str)` is the trait surface for `/api/v1/query` only, and PromQL instant queries with a `metric[10s]` inner always return Vector (one value per series computed over `[t-range, t]`). The `any_range_candidate` signal captured the inner range, not the outer eval shape, and produced a Matrix that the Prometheus adapter's `format_success_response` rejects with a 500 / empty body. Hard-wired the `execute(&str)` site to pass `false`; left the parameter on the helper for the future `handle_range_query_promql` wire-up. Test 6 also pivoted: `top_endpoint_qps` is name-classified as TopK by the planner (always heap-bearing), so the original heap-less wire setup couldn't match. Switched to heap-bearing msgpack wire (same shape as Test 9) and a `count_over_time(...)` query — cross- coverage with Test 9's `topk(...)` on the same underlying sid. Co-authored-by: Claude Opus 4.7 (1M context) <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.
Summary
Per user:
count_over_time(metric[range])IS frequency estimation — it counts samples per series in the window, which is exactly what heap-less CMS / CountSketch estimates. Pre-PR, the analyzer rejected it asUnsupportedAggIntent(\"count\")and routed every such query to the cold tier even when a CMS / CountSketch policy was registered.Root cause
The PromQL parser mapped
count_over_timetoAggFunc::Count, andlower::convert's un-grouped Count carve-out at line 254 pinned that toAggIntent::Count{Exact}(the SQLCOUNT(*)semantics — wrong for PromQL).Fix
AggFunc::Frequencyvariant that bypasses the un-grouped Count carve-out (onlyAggFunc::Counttriggers it).walk_call_to_opnow picks one of three mappings forcount_over_time:count(...)→CountDistinct(HLL) — unchangedtopk(...)→Count— unchanged (topk wrapper expects a count-shaped inner)Frequency— NEWagg_func_to_intents+to_sketch_oprouteFrequencythroughdefault_frequency()→AggIntent::Frequency{Epsilon}.function_to_familyrecognizes\"count_over_time\"asQueryFamily::FrequencyEstimate.Refreshed tests
is_asap_tier_answerable_true_for_count_over_time(invariant flipped from_false_for_unsupported).count_over_time_binds_to_frequency_estimate(was_is_unsupported_at_exact_accuracy).analyzer_parity_18_query_corpusGOLDEN — Q08 ctrlMISS → OK [cap=FrequencyEstimate(Any) fn=count_over_time].Unchanged paths
count(metric)distinct counting → HLL (CardinalityApprox) ✓count by (...) (count_over_time(...))→ HLL (CountDistinct over count) ✓topk(N, count_over_time(...))→ FrequencyTopk via the count-shaped inner ✓Test plan
cargo test --workspace --lib— 1534 pass, 0 failcargo test --test e2e_controller_plans_and_backend_serves— all 9 pass🤖 Generated with Claude Code