Repository navigation
fix(planner): topk over a grouped sum of sum_over_time or count_over_time gets the fused heap - #836
Merged
Merged
Conversation
zzylol
force-pushed
the
fix/topk-grouped-sum-heap
branch
from
October 21, 2026 21:21
a456a52 to
972201f
Compare
…e fused heap `topk(k, sum by (G) (sum_over_time(m[W])))` and its `count_over_time` form lowered to an exact Sort + Limit, so Pass 1 had no top-k target and offered no heap. The PromQL frontend now lowers them to `TopK` over the grouped sum, as it lowers the counter forms. The keyed-additive rule reads the raw samples beneath a grouped sum of per-series sums (counts): weighted by the sample value (an open SamplesOfMetric requirement) or by one. `FusedComputation.absorbs` lists every target the fusion absorbs, so the heap absorbs the grouped and the per-series aggregate, and the grouped sum keeps its nested-sum merge for the exact top-k. Closes #832
FusedComputation.absorbs is a list now: the heap over the raw samples beneath a grouped sum of per-series sums or counts absorbs the grouped sum and the per-series target. The mapping architecture and contracts docs, the planner-layering proposal and the computation_for doc comment said it absorbs only the target directly beneath. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnjmPuXMpEWTxdSH31cvQi
… absorb When a fused computation is picked, no target beneath it is computed, including one that an absorbed target's own merge absorbs. Close each FusedComputation.absorbs over those after the nested-sum merges are added, so absorbed_choice_varies keeps only their first choice and combinations still counts exactly the choices enumeration keeps. No query reaches that shape today; a test builds it by hand. Also add a test that bottomk, `sum without` and `sum by (__name__)` get no heap over the raw samples, reuse the enumerated_choices test helper, rename the frontend's weighted_counter_ranking to weighted_group_ranking and fix the comments that still said a fused computation absorbs only the target directly beneath. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RqhsM5XsKhxjMP6J2XXXfB
Rebased onto the rename: the new code, tests and docs say raw data samples, WeightDomain::RawDataSamplesOfMetric and RawDataSample. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RqhsM5XsKhxjMP6J2XXXfB
zzylol
force-pushed
the
fix/topk-grouped-sum-heap
branch
from
October 21, 2026 22:58
793ddcf to
e41fe07
Compare
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.
Why
topk(k, sum by (G) (sum_over_time(m[W])))ranks groupsGby the sum of their raw data samples, and thecount_over_timeform ranks them by their sample count. A fused Count-Min or Count Sketch heap answers either one in a single pass over the samples: each sample adds its value (or 1) to its keyG. Pass 1 offered no heap candidate for these queries, only the exact sort and limit (#832). After #815 and #831, only the series-ranking formtopk by (G) (k, sum_over_time(m[W]))and the counter forms (sum by (G) (increase|rate(...))) got the fused heap.What
topk(k, sum by (G) (sum_over_time(m[W])))andtopk(k, sum by (G) (count_over_time(m[W])))aTopKtarget. The target keepsExactand gets a fusedCmsWithHeapandCountSketchWithHeap. This also works with an outertopk by (P), which partitions the heaps.Glabels other than the outerbylabels:RawDataSamplesOfMetricrequirement, so Count-Min is admitted only over a gauge declaredvalues_non_negative(fix(plan-selection): prove Count-Min weights over a gauge declared non-negative #815). Count Sketch takes any gauge.Glabels and the value, without__name__.Gover the samples). The selected exact plan is the same plan as before, plus one finalProject. Measured with the converter: 1.24077e-8 before, 1.24078e-8 after.increase,rate) were already covered and are unchanged.bottomkstill lowers toSort+Limit.How
build_over_sub_dagpromotes a descendingtopkoversumtoAggIntent::TopKwhen the summed child is a per-seriessumorcount, just as it does forrate,increaseandirate.summed_per_series_samplesrecognises a groupedsum(notwithout, not by__name__, with no filter and noHAVING) of the value of a per-seriessumorcountof raw rows.realize_keyed_additive_summary_inputthen builds the heap over those rows instead of over the per-series results: a grouped sum of per-series sums is the sum of the samples by group.FusedComputation.absorbsis nowVec<usize>. The heap absorbs the grouped sum and, when the grouped sum is the per-series target's only consumer, the per-series target too. Choices of the absorbed targets therefore do not multiply the heap candidates.combinationscountsn · Π m + frecursively, and it still equals the number of choices enumeration keeps.absorbed_choice_varieschecks every absorbed digit.close_absorptionsadds to each fused computation the targets its absorbed targets' own merges absorb. None of them is computed when the fused computation is picked, so the count stays exact for any nesting.TopKHeapOverOneRowPerItem) was not the cause, and it is unchanged. It runs after the fused heaps have moved out of the top-k's own candidates.Key code interfaces added / modified
crates/logical-asap-optimization/src/per_computation_candidates.rscrates/logical-asap-optimization/src/realization.rsUsage
This walks through
a_forced_heap_grouped_topk_sum_plan_compiles_and_answers_within_its_bound(crates/planner/tests/runtime_install.rs), run at the linked commit:The test also runs the other three combinations (Count Sketch over an undeclared gauge for the sum, and both heaps for
count_over_time, weighted by 1).Before this PR
topk(3, sum by (region) (sum_over_time(data[1m])))was lowered toLimit{3}(Sort{value desc}(...)), so it had no top-k target. The forced Count-Min plan above failed:After this PR
The query has a top-k target with
Exactand the two fused heaps over the raw data samples. Plan selection prices them beside the exact plan. Exhaustivestage_pipelineon the variant workload below (q07 rewritten astopk by (region, service) (4, sum by (region, service, job) (sum_over_time(data[20m]))), 10,000 series, guarantees ignored, no raw data) gave these costs in the built-in model:CmsWithHeapCountSketchWithHeapWith only 100 groups, the exact plan stays the cheapest. The heaps win only when there are many more groups.
Verification
whole_expression_topk_over_a_grouped_sum_of_range_sums_reads_the_raw_data_samplescovers sum, count, and partitioned sum. It checks that the target keepsExactand gets the two fused heaps, which absorb both aggregates. Each heap reads theTimeRangewith itemregion, the right weight and weight domain, and the outer partitions, andcombinationsequals the enumerated choices. It failed before the fix (no top-k target).grouped_range_sums_a_raw_data_samples_heap_cannot_rank_get_no_fused_heap(bottomkkeepsSort+Limit;sum withoutandsum by (__name__)get no heap over the raw data samples) anda_fused_computation_absorbs_what_its_absorbed_targets_merges_absorb(the closure keepscombinationsequal to the enumerated choices).a_forced_heap_grouped_topk_sum_plan_compiles_and_answers_within_its_boundcovers Count-Min and Count Sketch for sum, and both for count. In each case the forced heap plan compiles and recalls the true top 3 on skewed data. Its estimates are within the ε bound of the exact values and ofquery_time_reference, and its answers carryregiononly. It failed before the fix (no valid candidate).cargo test --workspace --locked,cargo test -p asap-planner --no-default-features --features promqlandcargo fmt --checkpass locally. I did not wait for CI.workload-01, local converterasapquery_random_workloadrebuilt on this branch,--no-raw --failure-probability 0.3 --check, three evaluations):The rows marked variant use q07 rewritten to the new shape,
topk by (region, service) (4, sum by (region, service, job) (sum_over_time(data[20m]))). Recall and max relative error are taken over the three evaluations.--ignore-accuracy-guaranteesCmsWithHeap(w 55, d 2, heap 20)--ignore-accuracy-guaranteesCmsWithHeap,CmsWithHeap--ignore-accuracy-guarantees--ignore-accuracy-guaranteesCmsWithHeap, q07 exactEvery check passes. The as-is q07 is the series-ranking form, which this PR does not change: its heap comes from #815 and #831. Without
--ignore-accuracy-guarantees, no heap is selected, because a top-k estimate has no guarantee. For the variant, the new heaps are offered and priced, but the exact nested-sum plan over 100 groups is cheaper (see the table above).Limitations:
bottomk, other range functions, and SQL nested aggregates are out of scope.Closes #832