Repository navigation
Description
Summary
For nested queries of the shape agg by (x) (fn(m[w])) (pattern OneTemporalOneSpatial), the planner
and the query engine disagree about who applies the outer aggregation — and for every combination
outside the small "collapsable" whitelist, nobody does. A planned query such as
sum by (host) (rate(http_requests[5m]))
is served from the precompute but returns per-series rate values with full label sets — i.e. the
result of rate(http_requests[5m]) — with the outer sum by (host) silently discarded. Values and
label shape both differ from Prometheus/VictoriaMetrics.
This affects every planned nested query whose (outer, inner) pair is not in the collapsable whitelist:
sum/avg/min/max/quantile/count over rate, increase, quantile_over_time, and all other
non-matching inner functions.
Root cause
The division of labor is decided by get_is_collapsable
(asap-common/dependencies/rs/promql_utilities/src/query_logics/logics.rs), which whitelists only:
sum(sum_over_time),sum(count_over_time),min(min_over_time),max(max_over_time)
Everything else — including sum(rate) / sum(increase) — is non-collapsable. (The exclusion is
intentional and correct for storage: a group-keyed increase accumulator would interleave counters
from different series under one key and manufacture spurious resets. The code notes "Increase and Rate
are commented out in the Python reference".)
For non-collapsable queries:
- Planner (
asap-planner-rs/src/planner/promql.rs,get_label_routing): routes the precompute
per-series —(rollup = ∅, subpopulation_labels = all_labels)— i.e. it stores per-series inner
values, implicitly deferring the outer aggregation to query time. It does not exclude these
queries (is_supportedis just a pattern match), so it emits configs andquery_configentries
for them. - Engine (
asap-query-engine/src/engines/simple_engine/promql.rs,
build_promql_execution_context_tail): computes the statistic from the inner function only, and
for the non-collapsable branch just setsquery_output_labels = all_labels. The execution pipeline
(collect_results_same_aggregation→format_final_resultsin
src/engines/simple_engine/mod.rs) emits one value per stored key and never applies any outer
reduce — the only execution-time use of the outer operator in the whole engine is the
collapsability check itself (promql.rs:253). - The planned query string gets bound to the per-series aggregation via
find_query_config_promql_structural, so the wrong result is served confidently instead of falling
back.
Net effect: planner stores per-series values expecting a query-time outer reduce; the engine has no
such step; the outer aggregation vanishes.
Why the test suite doesn't catch it
The only nested-query execution test is test_collapsable_sum_of_rate_old_vs_new
(src/tests/datafusion/plan_execution_temporal_tests.rs:606). Its rig is degenerate: metric labels are
exactly ["host"], with one series and one group, hand-built host-keyed accumulators. With one
series per group the outer sum is a no-op and all_labels == [host], so both the value bug and the
label bug are invisible — and the assertion only checks that the in-memory and DataFusion paths agree
with each other, not with Prometheus semantics. The hand-built config also isn't the shape the
planner would emit for this query.
Reproduction
- Metric
http_requests{host, path}with 2 hosts × 2 paths (4 series), monotonically increasing
counters. - Let the planner plan
sum by (host) (rate(http_requests[5m]))(emits a per-series
MultipleIncreaseconfig withgrouping = [],aggregated = [host, path], plus aquery_config
binding the query to it). - Ingest, then run the query against the engine.
Expected (Prometheus): 2 series, labels {host}, each the sum of its 2 per-series rates.
Actual: 4 series, labels {host, path}, each an individual per-series rate — identical to
rate(http_requests[5m]).
What works correctly today
- Collapsable whitelist queries (
sum(sum_over_time),sum(count_over_time),
min(min_over_time),max(max_over_time)): planner keys the precompute by thebylabels and the
engine's per-key read is the outer aggregation. Correct. - Unplanned nested queries: capability matching requires
grouping == by-labels(strict equality
inlabels_compatible), which the per-series config fails → the query correctly falls back to the
upstream backend.
Workaround
Remove/avoid query_config entries for non-collapsable nested queries so they fall back to the
upstream Prometheus/VM (correct results, no acceleration), or restrict planning to the collapsable
whitelist.
Proposed fix
Engine-side (the planner's per-series routing is the correct storage strategy):
- In the non-collapsable
OneTemporalOneSpatialpath, add the missing query-time outer reduce:
after computing per-series inner values, group them by theby/withoutoutput labels and apply
the outer operator —sum/min/max/countare trivial folds,avg= sum/count,quantile=
exact quantile over the per-series scalars. Setquery_output_labelsto the spatial output labels
(also fixing the label-name mismatch for grouped configs). - Keep
get_is_collapsableunchanged (rate/increase exclusion is required for counter continuity)
and keep capability matching strict. - Tests:
- Non-degenerate regression: 2 groups × 2 series per group,
sum by (host)(rate(...))and
max by (host)(rate(...))with hand-computed expected values, for both the in-memory and
DataFusion paths. - A planner-routing test pinning that the emitted config + engine execution together produce
Prometheus-shaped results (label names =bylabels, series count = group count).
- Non-degenerate regression: 2 groups × 2 series per group,
Acceptance criteria
- Planned
sum by (x)(rate(m[w]))returns one series perxwith the summed rate (matching upstream
within extrapolation tolerance), for instant and range queries, on both execution paths. - Non-collapsable nested queries with
avg/min/max/quantile/countouters likewise match
upstream on multi-series-per-group data. - Collapsable-whitelist behavior and unplanned-query fallback are unchanged.
- New tests fail on the current behavior and pass with the fix.
Activity