feat(ir): shared L3 canonicalization pass for heavy-hitter topk (#34) - #91
Merged
Merged
Conversation
Semantically equivalent SQL and PromQL queries produced structurally
different L3, and the heavy-hitter gate was duplicated across both front
ends with divergent recognition logic. This adds a single post-lowering
canonicalization pass both languages run through.
- `asap_l2::canonicalize` — a bottom-up L3 rewrite, run at the end of the
shared `convert_root` so both front ends get it for free. It promotes a
count-ranked `Limit{Sort{[Project] Aggregate([Count])}}` into the canonical
heavy-hitter shape: an outer `Aggregate([TopK{k}])` (grouped by the sort's
partition) over the *explicit* inner `Aggregate([Count])` — the shape
PromQL's `topk(k, count_over_time(...))` already produced. The match is
positional (the DESC key must land on the Count's output column), so it is
oblivious to whether the count was aliased.
- Remove the SQL front-end heavy-hitter gate (`heavy_hitter_topk` /
`lower_as_topk` and their helpers). SQL now emits a plain Sort+Limit and
lets canonicalize recognise the count-ranked shape. This fixes the alias
blind spot (#20 — `ORDER BY cnt DESC` now promotes like `ORDER BY COUNT(*)`)
and closes the SQL/PromQL structural gap (#25 — SQL's implicit-count
`TopK{Scan}` is gone; both languages now emit `TopK` over an explicit
`Count`). The outer `TopK.by` is the ranking partition ([] for a global
`ORDER BY … LIMIT k`), with the grouping on the inner `Count`.
Tests:
- `asap-l2` unit tests for the pass: promotion (with/without a passthrough
projection), idempotency, and the negative cases (ascending, OFFSET,
ranking a group key, non-Count aggregate).
- `crates/lower/tests/cross_language.rs` — the executable spec: SQL S2 and
PromQL P1 reach the same canonical shape; aliased ≡ inline (#20);
non-count / OFFSET stay generic in both languages.
- Updated the SQL `count_ranked_topk_is_heavy_hitter` expectation to the
canonical two-level form.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 5, 2026
Closed
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.
Closes #34. Fixes #20 (SQL alias gate miss) and #25 (SQL/PromQL heavy-hitter tree divergence) as a side effect.
Problem
Semantically identical "top-k by count" queries produced different L3 depending on source language, and each front end had its own heavy-hitter recognition gate with subtly different logic:
ORDER BY cnt DESC(aliased) silently fell through toSort+LimitwhileORDER BY COUNT(*) DESChit the gate (SQL heavy-hitter gate misses count-ranked topk when COUNT is aliased #20).Aggregate([TopK]) { Scan }(count implicit); PromQL's wasAggregate([TopK]) { Aggregate([Count]) { … } }(count explicit) (L3 IR structural inconsistency: SQL and PromQL heavy-hitter topk produce different tree shapes #25).Fix: one shared pass
asap_l2::canonicalize— a bottom-up L3 rewrite run at the end of the sharedconvert_root, so both front ends (and any future language) get it for free. It promotes a count-rankedinto the canonical
The match is positional — the DESC key must resolve (through an optional passthrough projection) to the Count's output column — so it doesn't care whether the count was aliased. That kills #20 without any SQL-plan-specific alias handling.
The SQL front-end gate is removed (
heavy_hitter_topk/lower_as_topk+ helpers): SQL now emits a plainSort+Limitand lets canonicalize do the promotion — the same path PromQL's generictopktakes. PromQL'stopk(k, count_over_time(…))gate still emits the canonical shape directly (canonicalize is idempotent on it).Result: SQL and PromQL heavy-hitters are now structurally identical above the leaf —
TopKover an explicitCount— differing only in what they count (SQL rows vs. windowed samples), which is the genuinely intentional part of #25. The outerTopK.byis the ranking partition ([]for a globalORDER BY … LIMIT k); the grouping lives on the innerCount.Tests
asap-l2unit tests for the pass in isolation: promotion (with and without a passthrough projection), idempotency, and negatives — ascending sort,OFFSET, ranking a group key, non-Countaggregate.crates/lower/tests/cross_language.rs— the executable spec L3 IR standardization: cross-language correctness and canonicalization #34 asked for: SQL S2 and PromQL P1 reach the same canonical shape; SQL aliased ≡ inline (SQL heavy-hitter gate misses count-ranked topk when COUNT is aliased #20 regression); non-count andOFFSETstay generic in both languages.count_ranked_topk_is_heavy_hitterto the canonical two-level form (outer globalTopK, innerCountbyservice).Verified via the
topk_irexample: S1≡S2 now, S3 (OFFSET) / S4 (SUM) / S5 (AVG) staySort+Limit, P1/P10 unchanged.Full workspace suite green; clippy clean.
Not in scope
min_of/max_ofetc. are unrelated. Partitioned SQL topk (S8/S9, #24) now lowers on its own (derived-table support landed since #24 was filed) and is untouched here.