refactor(intent_algebra): retire SketchAgg/WindowedAgg — legacy_expr is a pure L2 IR - #229
Merged
Merged
Conversation
…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>
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.
Follow-up to #228.
Summary
#228 folded the
legacy_lowersketch-fusion pass intoconvert_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 IR.convert'sAggregatearm now does the single-statistic sketchable fusion directly in canonical terms —Window-input →Window { Aggregate { by: [] } },GROUP BYkeys → wrappingPartition,StdDev/Variance→Mergeof sibling quantile aggregates — instead of emitting intermediate legacy nodes.convert_rootis now justBinder::bind+convert.legacy_expr::QueryExpr:SketchAggandWindowedAggvariants removed;WindowSpec/WindowKindremoved (onlyWindowedAggused them).legacy_expris now a pure Layer-2 relational IR.legacy_to_canonical: theSketchAgg/WindowedAggconvert arms,lower_to_sketch_algebra,lower_aggregate,map_window_kind, and thelower_testsmodule are deleted.binder.rs:collect_referenced_columnsdrops theSketchAgg/WindowedAggarm — the converter only ever resolves group-by keys positionally, neverAggItem.col.Behavior unchanged for every tree the parsers emit: the merged
Aggregatearm matches the rawWindowinput, which is exact because the parsers never nestAggregatedirectly overWindowdirectly over another sketchableAggregate— the only shape where raw vs. sketch-lowered input would diverge.window_fusion.rstests (which built legacyWindowedAggby hand) are rewritten to build the canonicalWindow { Aggregate }shape directly or via L2Aggregate { Window }+convert_root. design.md §3/§6 stalelegacy_lower/ fused-L3 references refreshed.Net: ~520 lines deleted.
Test plan
cargo build --workspacecleancargo test -p control_plane— 754 tests pass (727 + 27), 0 failureswindow_fusionequivalence harness green end-to-end against the canonical planner🤖 Generated with Claude Code