Repository navigation
SQL per-group top-k (ROW_NUMBER … rn <= k) plans as PromQL topk by; exact plans compile; compile check covers stored-output builds - #912
Merged
Conversation
… plans compile; compile check covers stored-output builds
- canonicalize rewrites WHERE rn <= k over ROW_NUMBER() OVER (PARTITION BY p
ORDER BY o) under a SELECT list that drops rn to the per-partition
Limit{k}{Sort{o}}, which heavy-hitter promotion turns into
Aggregate(Reduce(p), [TopK{k}]) for a count or sum ranking.
- heavy-hitter promotion applies only where no consumer reads the node's
columns by position (the root or a Concat branch).
- SQL top-k answers keep the group keys naming each ranked item after
[keys.., topk_k], in exact plans and from heap sketches.
- PlannerExecutorCompileCheck reports a stored output the executor cannot
build, so the verdict no longer depends on where the output is built.
Closes #901
…der, generic-form and nested-ranking tests; mixed-language test table declares its time index non-null
zzylol
force-pushed
the
fix/sql-per-group-topk
branch
from
October 23, 2026 02:39
c2d0c31 to
5c480b0
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
#901: the SQL per-group top-k idiom
was planned as a count, then
SQLWindowFunc, thenFilter rn <= 5. Three problems followed: no heavy-hitter sketch (CMS+heap, CountSketch+heap) was offered for it; no exact plan compiled (the executor has no window function), so there was no exact answer to compare against; and Stage 3's compile check passed a plan whose count sketch was built at ingestion time while it rejected the same plan with the sketch built at query time. The selected plan (P42-m3) then failed to install:the executor cannot build stored output 2 ... unsupported weighted frequency family.What
topk by (city)lowers to:Aggregate(Reduce(by=[city]), [TopK{5}])over theCOUNT(*) GROUP BY city, hostaggregate. Pass 1 now offers the same choices for it as for the globalORDER BY c DESC LIMIT 5top-k (exact, CMS+heap, CountSketch+heap, Cms, UnivMon).TopKdoes not cover (e.g.ORDER BY AVG(x), a tie-breaking second key, a SELECT list without the partition column) becomes the generic per-partitionLimit{k}{Sort}, which the executor runs. So every such query has an exact plan that compiles.host) after[keys.., topk_k]. This holds in exact plans and from heap sketches, as PromQL top-k answers carry series labels. Before, a SQL top-k answered only the counts.Concatthat nothing reads by position either.SELECT host FROM (SELECT host, COUNT(*) AS c … ORDER BY c DESC LIMIT 5)used to return the counts under the namehost. It now stays an exactLimit{Sort}. This applies to PromQL too: a ranking that lowers toLimit{Sort}and sits under a reader, as incount(topk(5, sum by (job) (m))), is no longer promoted. That keeps it from losing its columns, at the cost of its heap-sketch choices. PromQL top-ks that the frontend lowers straight toTopK(over counts or rates) are unaffected.Ties.
ROW_NUMBERnumbers rows that tie on the ordering value in no particular order.rn <= ktherefore keeps an arbitrary subset of the rows tied at the k-th place, and so do PromQLtopk, the exactLimit{Sort}and a heap sketch. Those may each keep different tied rows. The tests use data whose counts are distinct at the k-th place, so they are deterministic.How
asap_types::ir::canonicalize: a new local rule,try_rewrite_row_number_top_k, matchesProject{P}{Filter{rn <= k | rn < k+1 | rn = 1}{[Project{Q}]{SQLWindowFunc{RowNumber, p, o}{X}}}}, wherePkeeps only columns ofXand dropsrn. It rewrites the chain toLimit{k,p}{Sort{o,p}{Project{P}{X}}}whenPkeepspando, and toProject{P}{Limit{k,p}{Sort{o,p}{X}}}otherwise. The existing heavy-hitter rule then promotes the first form, and now maps the partition through the projection.canonicalizecomputes once the set of nodes that a non-Concatconsumer reads by position (read_by_position). Heavy-hitter promotion is skipped for those nodes.implement_topk(exact) and Stage 1ranked_as_topk(heap sketch) keep the item columns for SQL as well as for PromQL.PlannerExecutorCompileCheck::compilesmapsInstallablePlanError::StoredOutputNotExecutabletoErr("stored output N: …"), the same formcheck_compilesuses. It used to treat that error as "not installable for another reason" and returnOk.Key code interfaces added / modified
crates/types/src/ir/canonicalize.rscrates/planner/src/installable.rsOutput format: a SQL top-k root outputs
[partition keys.., topk_k, item group keys..]. It used to output[partition keys.., topk_k]. Examples:["city", "topk_5", "host"]for the per-city query,["topk_5", "host"]for the global one.Usage (end to end, the issue's workload; see
crates/planner/tests/sql_per_group_topk.rs):stage_pipelineon the issue's workload with--exhaustive(the issue's command) selectsP7-m3:Q1 exact (Count acc) · Q2 exact (Count acc) · Count acc for Q1 in 30 × 10s panes, Count acc for Q2 in 30 × 10s panes · ingestion time: Count acc ×30 panes, Count acc ×30 panes. It reports nostored_outputs_error.Major algorithm changes
Canonicalization of SQL's ROW_NUMBER per-partition top-k
rnfilter were left in place. The plan therefore had noTopKtarget for Pass 1 to offer heap sketches for, and no exact plan the executor could run: all 22 all-exact plans were rejected withvalue operation has no native implementation.canonicalizeonly rewroteLimit{Sort{[Project]Aggregate([Count|Sum])}}intoAggregate([TopK]). A ROW_NUMBER rewrite had existed and was removed because dropping the visiblerncolumn broke outer projections.try_rewrite_row_number_top_k, thentry_promote_additive_top_ranking, both inapply_local_rules):Project{P}overFilter{Column(rn) <= k}(or< k+1, or= 1, with k ≥ 1). Optionally pass through the derived table'sProject{Q}. Under that isSQLWindowFunc{RowNumber, args: [], partition p, order o ≠ []}overX, wherernis the window's appended column.PthroughQto a column ofX. Give up ifPholdsrnor a computed item.Pkeeps every column ofpando, emitLimit{k,p'}{Sort{o',p'}{Project{P}{X}}}, withp'ando'remapped. This node hasP's schema. Otherwise, emitProject{P}{Limit{k,p}{Sort{o,p}{X}}}.columns_may_change, meaning the original node is not read by position (read_by_position). It now maps the partition through the projection.rn <= kfor every partition, up to which tied rows are kept, which ROW_NUMBER leaves unspecified anyway. The generic form keepsP's schema exactly. The promoted form is used only at a root or in aConcatbranch, as before for the global top-k. Tests:canonicalizeunit testsrownumber_*,a_visible_row_number_is_left_alone,filter_on_a_non_rownumber_column_is_left_aloneanddoes_not_promote_a_ranking_read_by_position, plus idempotence checks;cross_language::sql_rownumber_count_topk_is_promqls_topk_by;sql_per_group_topk(exact answers, and SQL equal to PromQL on the same data).read_by_position. The rule itself is O(|P| + |p| + |o|) perProject. On the issue's workload, the exhaustive search now enumerates 702 logical and 1980 physical candidates, up from 234 and 660, because Q2 has 9 Pass 1 choices instead of 3.stage_pipeline --exhaustivetakes 41.6 s in a debug build, up from 17.0 s. The default search takes 2.6 s.ROW_NUMBERis handled.RANK/DENSE_RANKwould keep ties.rn <= k,rn < korrn = 1works, with every SELECT item a plain column.k >= rn,BETWEENand a conjunction with another predicate keep the window.rnkeeps the window, which has no executor implementation, so no plan of such a query compiles.Limit{Sort}) and is not offered heap sketches.Executor compile check covers stored-output builds
PlannerExecutorCompileCheck::compilesbuilds the installable plan first. That step compiles each stored output's build and fails withStoredOutputNotExecutablewhen the executor refuses one. The check treated that failure as "not installable for another reason" and returnedOk. A Cms that the executor cannot build (unsupported weighted frequency family) therefore passed when it was kept at ingestion time (P42-m3, selected) and failed when it was built in the query run (P42,P42-m1).Err(_) => Ok(())becomesErr(StoredOutputNotExecutable { node_id, reason }) => Err("stored output {node_id}: {reason}"). Other installability errors (e.g. a scalar root) still pass toplan_installable, which reports them.sql_per_group_topk::a_plan_whose_stored_output_does_not_build_is_rejectedfails before this change, whenplan_installablereturnsInstallable(StoredOutputNotExecutable), and passes after, when every candidate is rejected asExecutorCannotCompile.stage_pipeline's mixed-language test plans a SQLCOUNT(*)over a table whosetstime index is nullable. The executor builds no count over a nullable time column (summary time column must be a timestamp). On main, the query-time plan was rejected, but the ingestion-time plan was selected even though it could not be installed. Now both are rejected. That test's table now declarestsnon-null, which is what a time index is. The executor limitation itself is unchanged.Before this PR
On the issue's command (
--exhaustive): 234 logical and 660 physical candidates. Q2's Pass 1 choices wereexact (Count acc),CmsandUnivMon. All 22 all-exact plans were rejected withexecutor_cannot_compile. The selectedP42-m3(Q1 Cms · Q2 Cms, Cms at ingestion time) could not be installed (stored_outputs_error). SQL top-k answers had no item column.After this PR
On the same command: 702 logical and 1980 physical candidates. Q2's choices match Q1's:
exact (Count acc),CMS+heap (Count acc),CMS+heap + Cms,CMS+heap + UnivMon,CountSketch+heap (Count acc),CountSketch+heap + Cms,CountSketch+heap + UnivMon,Cms,UnivMon. All 22 all-exact plans compile; 20 lost on cost, 1 lost on a cost tie to the enumeration order, and one was selected. The Cms plans are rejected asexecutor_cannot_compilewhether built at ingestion time (stored output 2: … unsupported weighted frequency family) or at query time. The selected plan isP7-m3, both queries exact from ingestion-time count accumulators in 30 × 10 s panes, and it installs. The sketch plans are still rejected (count_min_weights_not_proven_non_negative, compile errors) because of #886.#886 (sketch candidate placement for SQL top-k by
COUNT(*)) is not fixed here. Q2 now goes through the same top-k candidate generation as Q1, so #886's misplaced candidates (a per-host Cms, a heap over already exact counts) now apply to Q2 too, and the #886 fix will cover both. The compile-check fix means the per-host Cms/UnivMon candidates #886 asks about are now rejected at every materialization, never selected through an ingestion-time build.Verification
Run locally on the rebased head:
canonicalizeunit tests (asap-types): ROW_NUMBER shapes, thernbounds (= 1,<= 0,< 1,= 2), positional readers, Concat branches, idempotence.asap-devtools --test cross_language(including a nested PromQL ranking that staysLimit{Sort}) and--test stage_pipeline.cargo nextest run -p asap-planner --test sql_per_group_topk: 6 passed:SELECT host, c) run exactly;cargo nextest run -p asap-integration-tests --test per_computation_candidates_sql --test stage2_physical_asap_optimization_sql --test sql_to_post_asap --test sql_to_physical: 23 passed.asap-frontend-sql --test sql_lowering,asap-planner --test runtime_install --test e2e_plan,asap-physical-asap-optimization,asap-logical-asap-optimization, integrationpromql_to_post_asapandoperator_sharing: all passed.rn = 1, Concat branches seen through a reader that reads by position, the PromQL behavior change stated and tested, and end-to-end tests of the generic forms.cargo fmt --check, pluscargo clippy --all-targets --all-features --locked -- -D warningson the touched crates: clean.stage_pipelineon the issue's workload, numbers above.