migrate: bump ASAPPlanner pin to current main, adapt to IR restructuring - #443
Merged
Merged
Conversation
ASAPController was renamed to ASAPPlanner, and its main branch has moved
far past the rev this repo was pinned to: the flat asap-ir/asap-l2/
asap-sketch/asap-plan crate split was consolidated into asap-types
(pre_asap/post_asap modules) + asap-aware-mapping, and several IR shapes
changed underneath. This re-pins to current ASAPPlanner main and adapts
every downstream consumer so the workspace builds and passes tests
against it.
Dependency changes
- control_plane, crates/asap_types, data_plane Cargo.toml: repoint git
deps from ASAPController to ASAPPlanner at current main
(cb70086b4c4a7ba89baf2516be81d0b192137a3a); replace asap-ir/asap-l2/
asap-sketch/asap-plan with asap-types (aliased locally as
`planner-types` via Cargo's `package = "..."` to avoid a path
collision with this repo's own crates/asap_types) and
asap-aware-mapping.
Upstream IR changes adapted to
- SummaryKind/SummaryParams split into per-family ExactKind/ExactParams
(exact accumulators) and SketchKind/SketchParams (approximate
sketches); SummaryAgg's kind+params fields collapsed into one
`family: SummaryFamilyType` enum. Every call site across
sketch_algebra, optimizer, emit, physical, and data_plane's
query engine updated for the split/collapse.
- QueryExpr::Window removed upstream (never had a real producer);
TimeRange{range,child} was already the actual canonical shape, so
every Window match arm across window_fusion.rs, optimizer/engine.rs,
physical/{allocator,planner}.rs, colored_dag/*, asap_tier_*.rs,
and query_parser is rewritten against TimeRange. window_fusion.rs's
recognize_windowed_sketch rewrite is a genuine bug fix, not just a
rename: the old Window-matching code was dead against real traffic.
- QueryExpr::LetBinding/Ref removed (workload-level CSE representation
is gone upstream). control_plane's own dead CSE consumers
(optimizer/cse.rs, intent_algebra/lower.rs) are deleted to match --
both were confirmed to have zero live callers (PR #428 switched
intent_algebra to asap_frontend_promql::lower_promql directly).
CommonSubexprElim (R8) is changed to always return None, with the
removed representation documented inline; this is correctness-
preserving, it only gives up an optimization pass.
- L2Expr/L3Expr/Expr<C> folded directly into QueryExpr itself; L3Scalar
renamed ScalarValue; Predicate<C> changed from Predicate(pub L3Expr)
to Predicate<C>(pub Box<QueryExpr<C>>). expr_ir.rs now re-exports the
upstream scalar types plus type aliases (L2Expr = UnresolvedQueryExpr,
L3Expr = QueryExpr) instead of defining its own.
- implement_tree_in_with(expr, &BindingScope, cost_model) collapsed to
implement_tree_with(expr, cost_model) (BindingScope removed); all
call sites updated.
- Aggregate.aggs field renamed to Aggregate.measures.
Vendored locally (upstream deleted, this repo still needs them)
- summary_exec.rs (data_plane): SummaryExecutor trait + execute(),
deleted upstream (ASAPPlanner#190/#197). Ported from the pre-deletion
crates/sketch/src/exec.rs, adapted so find_candidates takes
`family: &SummaryFamilyType` instead of separate kind/params.
- WindowKind (crates/asap_types/src/enums.rs): deleted upstream
alongside QueryExpr::Window: ASAPPlanner's scope (batch workload
planning) has no use for tumbling/sliding/session flush semantics,
but this repo's streaming aggregation config still does. Same shape
as the old re-export, so its ~145 call sites needed no changes.
- Flat SummaryKind/SummaryParams (crates/asap_types/src/
accumulator_spec.rs): upstream's split into ExactKind/SketchKind has
no single type spanning both anymore, but this repo's
AccumulatorSpec dispatch (~25 call sites in
precompute_engine::accumulator_factory) never needed that
distinction -- it's purely "which concrete Rust accumulator struct
to build." Vendored as the same 14-variant shape the pre-split type
had, with From impls to convert from upstream's ExactKind/SketchKind
where this repo's other code needs to bridge between them
(Materialization.kind/params, BackendAggregation's backend-facing
fields -- both genuinely span exact+sketch).
Testing
- cargo check --workspace --all-targets: clean.
- control_plane lib: 706 passed, 1 failed (pre-existing -- see below).
- data_plane lib: 927 passed, 0 failed.
- asap_types lib: 42 passed, 0 failed.
- data_plane/control_plane integration suites: same pattern, all
passing except the same 1 pre-existing failure plus 2 already-
ignored tests tracked as ASAPQuery-backend#431.
Pre-existing failures (not regressions -- verified against unmodified
main via a detached git worktree before touching anything):
- optimizer::rules::tests::invalid_sketch_type_override_falls_back_to_default:
fails identically on unmodified main. bind_workload_typed_with_item_filter's
override-re-derivation logic re-derives `statistic` to match the
override's natural family BEFORE validity is checked, making the
"invalid override falls back to default" path unreachable for the 5
canonical sketch families -- a genuine pre-existing logic bug.
- controller_plan_to_query_full_roundtrip_{cms,count_sketch}_with_heap_topk:
fail identically on unmodified main; matches two adjacent #[ignore]d
tests in the same file already documented as a known gap tracked at
ASAPQuery-backend#431.
Not in scope
- A repo-wide sweep of ASAPController -> ASAPPlanner prose in comments/
docs was only done in files this migration otherwise touched; a full
sweep across untouched files is left as follow-up.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
What
ASAPController was renamed to ASAPPlanner, and its
mainbranch had moved far past the rev this repo was pinned to. This re-pinsasap-ir/asap-l2/asap-sketch/asap-plan/asap-frontend-promqlto current ASAPPlannermain(cb70086b4c4a7ba89baf2516be81d0b192137a3a) and adapts every downstream consumer in this workspace to the IR restructuring that happened upstream in the meantime. This is a full migration, not a partial one — the workspace builds clean and the test suite passes at the same rate as before the bump.Why this is bigger than a routine pin bump
What started as "bump the pin + do a comment sweep" turned out to require adapting to real upstream architecture changes:
asap-ir/asap-l2/asap-sketch/asap-planwere folded intoasap-types(pre_asap/post_asapmodules) + a newasap-aware-mappingcrate. Aliased locally asplanner-types(via Cargo'spackage = "...") to avoid a path collision with this repo's owncrates/asap_types.SummaryKind/SummaryParamssplit into per-familyExactKind/ExactParamsandSketchKind/SketchParams;SummaryAgg'skind+paramscollapsed into onefamily: SummaryFamilyTypeenum. Threaded throughsketch_algebra,optimizer,emit,physical, anddata_plane's query engine.QueryExpr::Windowremoved upstream (confirmed: it never had a real producer).TimeRange{range,child}was always the actual canonical shape, so everyWindowmatch arm is rewritten againstTimeRange.window_fusion.rs'srecognize_windowed_sketchrewrite is a genuine bug fix, not just a rename — the old code was dead against real traffic.QueryExpr::LetBinding/Refremoved (workload-level CSE representation gone upstream). This repo's own dead CSE consumers (optimizer/cse.rs,intent_algebra/lower.rs) are deleted to match — both confirmed to have zero live callers (PR control_plane: adopt asap-frontend-promql for L1 parsing (Part B) #428 already switchedintent_algebratoasap_frontend_promql::lower_promqldirectly).CommonSubexprElim(R8) now always returnsNone; correctness-preserving, just gives up an optimization pass.L2Expr/L3Expr/Expr<C>folded directly intoQueryExpr;L3ScalarrenamedScalarValue;Predicate<C>changed from wrappingL3Exprto wrappingBox<QueryExpr<C>>.implement_tree_in_with(expr, &BindingScope, cost_model)collapsed toimplement_tree_with(expr, cost_model)(BindingScoperemoved upstream).Aggregate.aggsrenamed toAggregate.measures.Vendored locally (upstream deleted, this repo still genuinely needs them)
data_plane/.../summary_exec.rs:SummaryExecutortrait +execute(), deleted upstream (ASAPPlanner#190/feat(sid): migrate PrecomputedOutput + sinks to PolicyFingerprint #197). Ported from the pre-deletioncrates/sketch/src/exec.rs, adapted sofind_candidatestakesfamily: &SummaryFamilyType.WindowKind(crates/asap_types/src/enums.rs): deleted upstream alongsideQueryExpr::Window— ASAPPlanner's scope (batch workload planning) has no use for tumbling/sliding/session flush semantics, but this repo's streaming aggregation config still does. Same shape as the old re-export, so ~145 call sites needed zero changes.SummaryKind/SummaryParams(crates/asap_types/src/accumulator_spec.rs): this repo'sAccumulatorSpecdispatch (~25 call sites inaccumulator_factory) never needed the exact/sketch distinction upstream now enforces — vendored as the same 14-variant flat shape, withFromimpls bridging from upstream's split types where this repo's other code genuinely needs to distinguish exact vs. sketch (Materialization,BackendAggregation).Testing
cargo check --workspace --all-targets: clean.control_planelib: 706 passed, 1 failed (pre-existing, see below).data_planelib: 927 passed, 0 failed.asap_typeslib: 42 passed, 0 failed.#[ignore]d and tracked as effective_is_cumulative misclassifies bare count()/instant Cardinality-Frequency queries as non-cumulative #431.All 3 observed failures were verified pre-existing by checking them out against unmodified
mainin a separategit worktree, before any of this migration's changes:optimizer::rules::tests::invalid_sketch_type_override_falls_back_to_default— fails identically on unmodifiedmain.bind_workload_typed_with_item_filter's override-re-derivation logic re-derivesstatisticto match the override's natural family before validity is checked, making the "invalid override falls back to default" path unreachable for the 5 canonical sketch families. Genuine pre-existing logic bug, untouched by this PR's renames (confirmed viagit diff main).controller_plan_to_query_full_roundtrip_{cms,count_sketch}_with_heap_topk— fail identically on unmodifiedmain; matches two adjacent#[ignore]d tests in the same file already documented as a known gap, tracked as effective_is_cumulative misclassifies bare count()/instant Cardinality-Frequency queries as non-cumulative #431.None of these are regressions introduced by this migration.
Not in scope
A repo-wide sweep of
ASAPController→ASAPPlannerprose in comments/docs was only done in files this migration otherwise touched. A full sweep across files untouched by the technical migration is left as follow-up.