refactor(intent_algebra): fold legacy_lower into the legacy→canonical converter - #228
Merged
Merged
Conversation
… converter
The L2→L3 sketch lowering (`legacy_lower::lower_to_sketch_algebra`) and the
legacy→canonical conversion (`legacy_to_canonical::convert`) were two
separate passes the parse path chained: the lowering *fused* relational
`Aggregate { AggFunc }` shapes into `SketchAgg` / `WindowedAgg`, and the
converter immediately *un-fused* most of them while mapping onto the
canonical IR. The intermediate legacy Layer-3 fused IR existed only to be
created and instantly converted away.
Collapse the two: `lower_to_sketch_algebra` + `lower_aggregate` +
`agg_func_to_intents` move into `legacy_to_canonical.rs` as private items,
and `convert_root` runs them internally (lower-then-convert). The pass is
idempotent on input already at Layer 3, so `convert_root` still accepts the
hand-built `WindowedAgg` / `SketchAgg` trees the `window_fusion` harness
feeds it. `legacy_lower.rs` is deleted; the sketch-fused legacy Layer-3 IR
is no longer a surface anything outside the converter can construct.
`query_parser::parse_query_expr` becomes a pure language-dispatch front
door — it returns the raw legacy Layer-2 tree with no lowering — and drops
to `pub(crate)`. `parse_query_expr_canonical` is now the single public
algebra-IR entry; the legacy IR is never observable to callers.
Behavior is unchanged: `parse_query_expr_canonical` still computes
`convert ∘ lower ∘ parse`. The only test churn is the handful of tests that
asserted on the legacy Layer-3 shapes via `parse_query_expr` — rewritten to
pin the canonical output, which is the only algebra IR the parse path now
emits.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 14, 2026
…is now a pure L2 IR PR #228 folded the `legacy_lower` sketch-fusion pass into `convert_root`, but the sketch-fused legacy Layer-3 IR (`SketchAgg` / `WindowedAgg`) still existed as the intermediate between two internal walks. This collapses that to a single walk and deletes the intermediate. `convert`'s `Aggregate` arm now does the single-statistic sketchable fusion directly in canonical terms — `Window`-input → `Window { Aggregate { by: [] } }`, `GROUP BY` keys → wrapping `Partition`, `StdDev` / `Variance` → `Merge` of sibling quantile aggregates — instead of emitting intermediate `SketchAgg` / `WindowedAgg` nodes for the (now-deleted) second walk to un-fold. `convert_root` is just `Binder::bind` + `convert`. With the fusion done in canonical terms, the legacy IR has no remaining producer or consumer of the fused variants, so they're removed: * `legacy_expr::QueryExpr`: `SketchAgg` and `WindowedAgg` variants gone; `WindowSpec` / `WindowKind` (only `WindowedAgg` used them) gone. `legacy_expr` is now a pure Layer-2 *relational* IR. * `legacy_to_canonical`: the `SketchAgg` / `WindowedAgg` convert arms, `lower_to_sketch_algebra`, `lower_aggregate`, `map_window_kind`, and the `lower_tests` module are deleted. * `binder.rs`: `collect_referenced_columns` drops the `SketchAgg`/`WindowedAgg` arm — the converter never resolved `AggItem.col` positionally anyway, only group-by keys. Behavior is unchanged for every tree the parsers emit: the merged `Aggregate` arm matches the *raw* `Window` input, which is exact because the parsers never nest `Aggregate` directly over `Window` directly over another sketchable `Aggregate` — the only shape where raw vs. sketch-lowered input would diverge. `window_fusion.rs` tests, which built legacy `WindowedAgg` nodes by hand, are rewritten to build the canonical `Window { Aggregate }` shape directly (or L2 `Aggregate { Window }` + `convert_root`). design.md §3/§6 stale `legacy_lower` / fused-L3 references refreshed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
zzylol
added a commit
that referenced
this pull request
May 14, 2026
…is now a pure L2 IR (#229) PR #228 folded the `legacy_lower` sketch-fusion pass into `convert_root`, but the sketch-fused legacy Layer-3 IR (`SketchAgg` / `WindowedAgg`) still existed as the intermediate between two internal walks. This collapses that to a single walk and deletes the intermediate. `convert`'s `Aggregate` arm now does the single-statistic sketchable fusion directly in canonical terms — `Window`-input → `Window { Aggregate { by: [] } }`, `GROUP BY` keys → wrapping `Partition`, `StdDev` / `Variance` → `Merge` of sibling quantile aggregates — instead of emitting intermediate `SketchAgg` / `WindowedAgg` nodes for the (now-deleted) second walk to un-fold. `convert_root` is just `Binder::bind` + `convert`. With the fusion done in canonical terms, the legacy IR has no remaining producer or consumer of the fused variants, so they're removed: * `legacy_expr::QueryExpr`: `SketchAgg` and `WindowedAgg` variants gone; `WindowSpec` / `WindowKind` (only `WindowedAgg` used them) gone. `legacy_expr` is now a pure Layer-2 *relational* IR. * `legacy_to_canonical`: the `SketchAgg` / `WindowedAgg` convert arms, `lower_to_sketch_algebra`, `lower_aggregate`, `map_window_kind`, and the `lower_tests` module are deleted. * `binder.rs`: `collect_referenced_columns` drops the `SketchAgg`/`WindowedAgg` arm — the converter never resolved `AggItem.col` positionally anyway, only group-by keys. Behavior is unchanged for every tree the parsers emit: the merged `Aggregate` arm matches the *raw* `Window` input, which is exact because the parsers never nest `Aggregate` directly over `Window` directly over another sketchable `Aggregate` — the only shape where raw vs. sketch-lowered input would diverge. `window_fusion.rs` tests, which built legacy `WindowedAgg` nodes by hand, are rewritten to build the canonical `Window { Aggregate }` shape directly (or L2 `Aggregate { Window }` + `convert_root`). design.md §3/§6 stale `legacy_lower` / fused-L3 references refreshed. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 14, 2026
…canonical → lower_to_canonical (#231) The "legacy" naming was a misnomer. After #228–#230, `legacy_expr` is the real, current **L2 relational IR** every `query_parser` front end emits, and `legacy_to_canonical` is the genuine **L2→L3 lowering pass** — neither is removable debt; the 5-layer design mandates both layers exist. Pure rename, no behavior change: * `intent_algebra/legacy_expr.rs` → `intent_algebra/relational.rs` * `intent_algebra/legacy_to_canonical.rs`→ `intent_algebra/lower_to_canonical.rs` * the `convert_legacy` re-export alias → `convert` All ~25 consumer modules updated to the new paths; module-header docs and design.md §3/§6 references refreshed so they describe `relational` as the L2 IR rather than calling it legacy. The type is still spelled `QueryExpr` (consumers already disambiguate it from the canonical `query_expr::QueryExpr` via the established `LQueryExpr` alias) — a type rename is a separate, larger change if wanted. The word "legacy" no longer appears in any live module path or identifier in this area; remaining "legacy" mentions elsewhere refer to genuinely pre-canonical code (the untyped optimizer/physical path, `accuracy_sla`, etc.), not this IR. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
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
The parse path chained two separate L3-producing passes —
legacy_lower::lower_to_sketch_algebra(fuses relationalAggregate { AggFunc }intoSketchAgg/WindowedAgg) andlegacy_to_canonical::convert(immediately un-fuses most of it while mapping onto the canonical IR). The intermediate legacy Layer-3 fused IR existed only to be created and instantly converted away.This collapses the two:
lower_to_sketch_algebra+lower_aggregate+agg_func_to_intentsmove intolegacy_to_canonical.rsas private items;convert_rootruns them internally (lower-then-convert). The pass is idempotent on Layer-3 input, soconvert_rootstill accepts the hand-builtWindowedAgg/SketchAggtrees thewindow_fusionharness feeds it.legacy_lower.rsis deleted — the sketch-fused legacy Layer-3 IR is no longer a surface anything outside the converter can construct.query_parser::parse_query_exprbecomes a pure language-dispatch front door (returns the raw legacy Layer-2 tree, no lowering) and drops topub(crate).parse_query_expr_canonicalis now the single public algebra-IR entry.Behavior is unchanged:
parse_query_expr_canonicalstill computesconvert ∘ lower ∘ parse. The only test churn is the handful of tests that asserted legacy Layer-3 shapes viaparse_query_expr— rewritten to pin the canonical output.This is step 1 of retiring the legacy IR island; a follow-up slims
legacy_expr::QueryExprto a pure Layer-2 relational IR (the now-unusedSketchAgg/WindowedAgg/TopKvariants) and de-"legacy"s the naming.Test plan
cargo build --workspacecleancargo test -p control_plane— 774 tests pass (747 + 27), 0 failureswindow_fusionequivalence harness still green (convert_root idempotent on L3 input)🤖 Generated with Claude Code