Skip to content

refactor: extract the Stage 1 logical-optimizer crate (#572 B2) - #583

Draft
zzylol wants to merge 2 commits into
stack/509-d2-stage1-without-costfrom
stack/572-b2-logical-optimizer
Draft

zzylol wants to merge 2 commits into
stack/509-d2-stage1-without-costfrom
stack/572-b2-logical-optimizer

Conversation

@zzylol

@zzylol zzylol commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Why

#572 puts each #509 stage in its own crate so that Cargo enforces the one-way stage flow. This PR is step B2. Stage 1 (logical candidate generation) moves out of asap-aware-mapping into a new crate, asap-logical-optimizer. #582 left four blockers: Stage 1 imported plan_selection::candidate_selection, inherent impls on Stage 1 types lived in Stage 3, and the code relied on pub(crate) items. User decision Q42 (a) removes these first.

What

Before this PR

crates:  frontends ──► asap-aware-mapping (Stage 1 + 2 + 3 + facade) ──► asap-types
                       Stage 1 replacement ──► plan_selection::candidate_selection   (Stage 1 → Stage 3)

asap-aware-mapping/src/
  replacement rewrite rollup grouping exact_composition function_rules
  maintained_population explanation logical_candidates topk_reuse
  accuracy/{mod, composition, allocation, evidence, estimators/*, reconciliation}
  physical_candidates  plan_selection/  cost_model recurrence analytical_cost …  pass/

After this PR

crates:  frontends ──► asap-aware-mapping (Stage 2 + 3 + facade) ──► asap-logical-optimizer (Stage 1) ──► asap-types
         no path from asap-logical-optimizer back to asap-aware-mapping or asap-physical-operators

logical-optimizer/src/
  pass1/    replacement rewrite rollup grouping exact_composition function_rules
            maintained_population explanation logical_candidates
  pass2/    topk_reuse reconciliation
  accuracy/ mod composition allocation evidence estimators/*
asap-aware-mapping/src/
  physical_candidates  plan_selection/{mod, candidate_selection}  cost_model recurrence
  analytical_cost … storage_io  pass/

asap-logical-optimizer depends only on asap-types, asap_sketchlib, thiserror and serde_json. The PromQL front end is a dev-dependency used only by tests. cargo tree -p asap-logical-optimizer -e normal,dev,build finds neither asap-aware-mapping, asap-physical-operators nor asap-planner. An import of asap_aware_mapping from the new crate fails with E0433.

How

Commit 1: remove the blockers inside asap-aware-mapping (behavior unchanged)

  • GlobalSelection and TargetSubDAGSelection move back into replacement, together with assemble_*. Building a DAG from given choices is Stage 1 composition. Exact-composition plans are passed to GlobalSelection::new.
  • CostedGlobalSelection. global_selection* now returns it. It derefs to GlobalSelection and keeps the cost comparison behind each chosen exact composition in composition(target). This replaces the TargetSubDAGSelection::composition field, which carried cost types.
  • Free functions. The impl ReplacementSubDAG / impl CandidateLogicalASAPDAGs blocks in candidate_selection become free functions: cost_sorted, cost_sorted_with_recurrence, recurrence_profiles, global_selection, global_selection_with_recurrence and runtime_support_evidence. Callers are updated, for example space.global_selection(&m) becomes global_selection(&space, &m).
  • Newly public Stage 1 items:
    • read accessors groups(), order() and composition_plans();
    • PreparedComposition, a record read by Stage 3, with public fields;
    • cse_candidate_pair, direct_child_counts, realize_child (used by cost_model.rs) and ExactComposition::same_as.
  • Tests that exercised selection move with it.
    • Five tests move whole to candidate_selection: four reconciliation tests and global_selection_can_choose_nested_summaries.
    • Five mixed tests are split. Their Stage 1 assertions stay; their selection assertions become three new candidate_selection tests.
    • Two assembly tests move to replacement.
  • The candidate_selection module doc records that it is deleted under Pass 2 sharing and Stage 1 coverage parity with the retired MajorPass #580.

Commit 2: create the crate (pure move)

  • git mv. 26 files move with git mv, including tests/logical_candidates.rs, the Stage 1 guard and test_support. asap-aware-mapping keeps a trimmed test_support: lower_promql, agg, metric_scan and scan.
  • No compatibility re-exports. A script, followed by cargo check, rewrote every import across the workspace to asap_logical_optimizer::…: asap-aware-mapping, asap-physical-operators, devtools, integration-tests, planner, the PromQL and SQL frontend tests, and docs.
  • The D2 guard moves to the new crate. It scans every production file for cost_model:: / recurrence::. A new test checks that the manifest names neither asap-aware-mapping nor asap-physical-operators.
  • Crate-level docs are rewritten for both crates. Doc paths that cite the moved modules are updated.
  • Test fixtures. Two candidate_selection tests now build their fixtures through public Stage 1 API (a TargetSubDAGCandidates literal, search_workload) instead of crate-private constructors.
  • Rationale strings. Three rationale strings that named asap_aware_mapping::… now name the new path. They do not occur in any fixture.

Moved files

From asap-aware-mapping/ To logical-optimizer/
src/{replacement, rewrite, rollup, grouping, exact_composition, function_rules, maintained_population, explanation, logical_candidates}.rs src/pass1/*.rs
src/topk_reuse.rs src/pass2/topk_reuse.rs
src/accuracy/reconciliation.rs src/pass2/reconciliation.rs
src/accuracy/{mod, composition, allocation, evidence}.rs, src/accuracy/estimators/* (8 files) src/accuracy/…
src/test_support.rs src/test_support.rs (trimmed copy kept)
tests/logical_candidates.rs, tests/stage1_cost_independence.rs tests/…

Volume

Commit 1 Commit 2
Files changed 35 96, including 26 moved with git mv and 5 new: Cargo.toml, lib.rs, pass1/mod.rs, pass2/mod.rs and the trimmed test_support copy
Rewrites about 90 call sites from methods to free functions 111 use statements rewritten (131 after splitting mixed groups); 321 changed lines that cite old paths, including docs

Gate (at the tip)

  • cargo fmt --all --check: passes.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings: passes.
  • cargo test --workspace: 1,528 passed, 12 ignored. refactor(planner): Stage 1 no longer depends on the cost model #582 had 1,524. Commit 1 adds 3 tests by splitting mixed Stage 1 and selection tests (1,527). Commit 2 adds the manifest guard.
  • python3 -m unittest discover -s tools/dag-viewer -p test_render.py: 29 tests OK, 6 skipped.
  • stage_pipeline --example planner-layering-1 regenerates tools/dag-viewer/examples/planner-layering-example1.json byte for byte, at both commits.

Remaining items (B3–B5)

  • Q22: move AccuracyModel pluggability to plan-selection. The trait stays in logical-optimizer/src/accuracy/mod.rs:37 for this pure move. Stage 1's legacy search still takes it:

    • replacement.rs:1173, :1223, :1238 and :4943 (search_workload_with_targets);
    • grouping.rs:141;
    • exact_composition.rs:217.

    The stage pipeline's Stage 1 entry, enumerate_local_logical_candidates (logical_candidates.rs:100), does not take it. PlanningModels.accuracy (plan_selection/mod.rs:83) imports the trait from Stage 1, which is a legal direction.

  • Legacy selection (Pass 2 sharing and Stage 1 coverage parity with the retired MajorPass #580). candidate_selection uses the raw-pointer accessors and PreparedComposition. Deleting it under Pass 2 sharing and Stage 1 coverage parity with the retired MajorPass #580 lets B4 avoid moving it.

  • B3 (physical-optimizer). physical_candidates imports only asap-types and Stage 1. Its test uses the test_support copy (physical_candidates.rs:156), so B3 needs its own test_support copy and a frontend-promql dev-dependency. Stage 3 imports Stage 2 at plan_selection/mod.rs:45, which is a legal direction.

  • B4 (plan-selection). Its tests use the same test_support copy (plan_selection/mod.rs:1014, candidate_selection.rs:1359). pane_sharing, erp and empirical_comparison are still present, although Reorganize crates and modules by #509 stages and the #511 unified IR #572 marks them for deletion.

  • B5 (executor). The asap-physical-operators unit test at physical_planner/candidates.rs:367-370 and four integration tests call the legacy global_selection through dev-dependencies. They follow Pass 2 sharing and Stage 1 coverage parity with the retired MajorPass #580.

Part of #509 and #572 (B2). Follows #582. Related: #580.

🤖 Generated with Claude Code

zzylol and others added 2 commits October 4, 2026 03:18
…crate split

Stage 1 no longer imports `plan_selection::candidate_selection`, and the
legacy selection code no longer adds inherent methods to Stage 1 types, so
Stage 1 can move into its own crate (#572 B2, decision Q42 (a)).

- `GlobalSelection` and `TargetSubDAGSelection` move back into
  `replacement` with the DAG assembly (`assemble_*`): building a DAG from
  given choices is Stage 1 composition and needs no cost. Exact-composition
  plans are passed in by `GlobalSelection::new`.
- `global_selection*` returns `CostedGlobalSelection`, which derefs to
  `GlobalSelection` and keeps the cost comparison behind each exact
  composition (`composition(target)`), formerly the
  `TargetSubDAGSelection::composition` field.
- The `impl ReplacementSubDAG` / `impl CandidateLogicalASAPDAGs` blocks in
  `candidate_selection` become free functions (`cost_sorted`,
  `cost_sorted_with_recurrence`, `recurrence_profiles`, `global_selection`,
  `global_selection_with_recurrence`, `runtime_support_evidence`); callers
  across the workspace and docs are updated.
- Stage 1 exposes what they need: read accessors `groups()`, `order()` and
  `composition_plans()`, and public `PreparedComposition`,
  `cse_candidate_pair`, `direct_child_counts`, `realize_child` and
  `ExactComposition::same_as`.
- Selection assertions in Stage 1 test modules move to
  `candidate_selection` tests: four reconciliation tests and
  `global_selection_can_choose_nested_summaries` move whole; five mixed
  tests are split, adding three selection tests. Assembly tests move to
  `replacement`.

Behavior is unchanged; the Example 1 fixture regenerates byte for byte.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stage 1 of #509 (logical candidate generation) moves out of
asap-aware-mapping into the new crate `asap-logical-optimizer`
(`crates/logical-optimizer`), so Cargo enforces that Stage 1 depends on no
later stage (#572, step B2). This is a pure move plus import updates.

Layout:
- `pass1/`: replacement, rewrite, rollup, grouping, exact_composition,
  function_rules, maintained_population, explanation, logical_candidates.
- `pass2/`: topk_reuse, reconciliation (was `accuracy::reconciliation`).
- `accuracy/`: the analytical model (mod, composition, allocation, evidence,
  estimators). `AccuracyModel` stays here for now; moving its pluggability
  to plan selection (Q22) is a later step.

The new crate depends only on asap-types (plus asap_sketchlib, thiserror and
serde_json); asap-aware-mapping, which keeps Stage 2, Stage 3 and the facade,
depends on it. asap-aware-mapping no longer re-exports any Stage 1 item:
every import across the workspace and docs now names
`asap_logical_optimizer`. Files move with `git mv`, together with their tests
(`tests/logical_candidates.rs`, the Stage 1 guard) and `test_support`;
asap-aware-mapping keeps a trimmed copy of `test_support` for its own tests.

The Stage 1 guard now scans every production file of the new crate and also
checks that its manifest names neither asap-aware-mapping nor
asap-physical-operators. Two candidate_selection tests build their fixture
through public Stage 1 API instead of crate-private constructors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant