feat(asap-aware-mapping): add workload-aware fine-to-coarse rollups - #262
Conversation
ca60c0c to
66c772d
Compare
8cc86fe to
066dc98
Compare
…licability.rs "Site" was an informal synonym introduced when this module's docs were written — every occurrence names the exact same thing the type system already calls TargetSubDAG (or, once discovered, a MemoGroup's own `target`). Replaced every free-standing "site"/"a site's..." with TargetSubDAG (or "the TargetSubDAG" as a phrase) throughout the module doc and the OptimizationKind/collect_locations/test doc comments, so the prose names the real type instead of a parallel, undefined term. Left two references untouched: `crate::search::discover_sites` (a real, unrenamed function name in search.rs) and the quoted section title "Where `for site in plan.bindable_sites()` comes from" (a verbatim quote of search.rs's own doc heading) — both are accurate references to search.rs's own content, which this PR doesn't touch. Also fixed a handful of stale `boundary::`-module doc links found while in here (the module was renamed to `implementation` well before this branch's fork point — same staleness class as PR #263/#262/#261/ #260, just not load-bearing for compilation since these were doc-only intra-doc links). Verified: cargo build --workspace --all-targets, cargo test --workspace (0 failures), cargo fmt --all -- --check, cargo clippy --workspace --all-targets --all-features -- -D warnings — all clean.
066dc98 to
6a7f560
Compare
…rrent tip This branch had been forked at the very first draft of #251 (boundary::implementation_for_with, a ForceSketchKind CostModel-wrapping hack to steer bind::implement_tree_with) and never rebased — missing the "make binding literally a selector over ReplacementStrategy" refactor, and the deletion of bind::implement_tree/implement_tree_with. It would not compile against the current tip of feat/replacement-strategy-251. Its own copy of replacement.rs/lib.rs was just a frozen snapshot of that first draft, reintroducing already-fixed stale code. Rebuilt fresh: reset this branch onto the current #259 tip (which already carries the current, correct replacement.rs/bind.rs/lib.rs/ implementation.rs/cost_model.rs), then reapplied only the genuinely new content this branch adds on top of it — rollup.rs (issue #254, part of - rollup.rs itself needed no code changes at all: it never called bind::implement_tree/implement_tree_with or referenced boundary:: directly — RollupStrategy/is_legal_rollup_source/build_rollup are self-contained around crate::replacement's TargetSubDAG/ ReplacementSubDAG/Replacement/ReplacementStrategy vocabulary, which is unchanged in shape on the current #251 tip. Copied over verbatim. - lib.rs: added `pub mod rollup;` and a module-doc bullet under `## Status` describing RollupStrategy/is_legal_rollup_source (the old branch's own lib.rs diff added no such bullet and no public re-exports for rollup, so none were reintroduced here either — the module's own `pub struct`/`pub fn` items are reachable via `asap_aware_mapping::rollup::*` same as before). - Did not touch replacement.rs/bind.rs/cost_model.rs/implementation.rs: they already reflect #259's current, correct state. Verified: cargo build --workspace --all-targets, cargo test --workspace (0 failures, including all 16 rollup:: tests unchanged), cargo fmt --all -- --check, cargo clippy --workspace --all-targets --all-features -- -D warnings — all clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6a7f560 to
db61054
Compare
# Conflicts: # crates/asap-aware-mapping/src/lib.rs
|
Didn't understand the example in the PR. Is that the output of both queries? A single query? Is it just showing the strategy replacement sub dag? |
|
Addressed the roll-up review findings in
Local validation: 117 |
|
Opened #278 to track accuracy-aware approximate PR #262 now notes that its exact-only restriction is conservative: approximate coarse queries may become legal when finer error composition or mergeable sketch state can be proven to satisfy the coarse query's own |
Summary
Closes #254. Part of #33.
This PR adds workload-aware group-by-lattice roll-up reuse. When two aggregates compute a compatible measure over identical input IR and one grouping is strictly finer than the other, the planner can derive the coarse result by re-aggregating the fine result instead of independently aggregating the raw input again.
The implementation includes both pieces required for the feature to be reachable:
RollupStrategy: validates a fine/coarse pair and constructs the rewrite.search_workloadandsearch_workload_withbuild the strategy from all discovered aggregate siblings and include its candidates automatically.PR #259, which introduced
ReplacementStrategyand workload search, is already merged; this PR is no longer stacked on an unmerged dependency.Example workload
Both queries are considered together:
Names used below:
S: inputScan(metrics)F: fine aggregate grouped by(job, region)C: original independent coarse aggregate grouped byjobR: coarse roll-up that consumesFCore detection algorithm
For aggregates sharing one input, pairwise matching costs approximately
O(k² × g), wherekis the number of sibling aggregates andgis the number of grouping keys.Input identity and column safety
A fast
Rc::ptr_eqcheck handles children already shared by CSE. FullQueryExpr::PartialEqis the fallback for independently lowered but identical inputs, including identical schemas. This fallback is necessary because CSE conservatively does not pointer-share a scan without a declared unique key.ColumnIdvalues are compared only after child equality proves both aggregates index the same input shape. Structurally different inputs or schemas are rejected.Strict grouping relation
The comparison is set-based:
Equality is not a roll-up, unrelated key sets are rejected, and duplicate keys cannot fake a strict superset.
Supported combinators
Sum(x)Sum(F.sum_x)Min(x)Min(F.min_x)Max(x)Max(F.max_x)CountSum(F.count)CountAvg,Rate, and unsupported intentsExact count uses
Sum, not anotherCount: counting rows inFwould count fine groups, while summing their exact counts recovers the original coarse row count.Approximate coarse queries are not inherently ineligible. They require composing finer error bounds or merging compatible sketch state and proving that the result satisfies the coarse query's own
AccuracyTarget. Equal finer/coarse epsilon values alone are not sufficient. This follow-up is tracked by #278.Strategy candidates for both queries
Before this PR:
After this PR:
Combined candidate space:
RollupStrategyproposes the alternative; it does not choose it. Cost/search or a downstream planner decides whether to use the independent candidate or the roll-up candidate.Complete two-query workload DAG
Every arrow below represents data flow, from producer to consumer/result.
Before selecting the roll-up
flowchart LR S["S: Scan(metrics)"] --> F["F: fine aggregate<br/>GROUP BY job, region<br/>SUM(bytes)"] S --> C["C: independent coarse aggregate<br/>GROUP BY job<br/>SUM(bytes)"] F --> QF["Q_fine result"] C --> QC["Q_coarse result"] classDef result fill:#eef,stroke:#668; classDef aggregate fill:#efe,stroke:#484; classDef scan fill:#fee,stroke:#844; class QF,QC result; class F,C aggregate; class S scan;Sfeeds two independent aggregates.After selecting the roll-up
flowchart LR S["S: Scan(metrics)"] --> F["F: fine aggregate<br/>GROUP BY job, region<br/>SUM(bytes)"] F --> QF["Q_fine result"] F --> R["R: coarse roll-up<br/>GROUP BY job<br/>SUM(F.sum_bytes)"] R --> QC["Q_coarse result"] classDef result fill:#eef,stroke:#668; classDef aggregate fill:#efe,stroke:#484; classDef scan fill:#fee,stroke:#844; class QF,QC result; class F,R aggregate; class S scan;Fappears once and becomes the shared producer. The original independent coarse nodeCis removed; itsS -> Cflow is replaced byF -> R.This is the selected pre-ASAP
QueryExprDAG. Materializing a final post-ASAP or physical plan remains a downstream decision.Implementation details
RollupStrategyowns clonedRchandles for its workload sibling set.is_legal_rollup_sourceremains a standalone predicate for reuse by other strategies.by ++ measuresoutput schema.without(...),Reduction::PerEntity, multi-measure aggregates, and aggregates withHAVINGare rejected.Tests
without(...), intent mismatch, unsupported combinators, and missing fine unique key.Sum, exactCount -> Sum, column remapping, shared fine child, and output-name preservation.Countis rejected both directly and through workload search.F -> Rroll-up candidate.Validation
cargo test -p asap-aware-mapping: 117 passed.cargo check --workspace: passed.cargo clippy -p asap-aware-mapping --all-targets --all-features -- -D warnings: passed.format-and-lintandtestchecks: passed.