Skip to content

chore(plan-selection): delete the legacy physical-plan cost model - #622

Draft
zzylol wants to merge 1 commit into
stack/cleanup-2-dag-export-modulefrom
stack/cleanup-3-legacy-cost
Draft

zzylol wants to merge 1 commit into
stack/cleanup-2-dag-export-modulefrom
stack/cleanup-3-legacy-cost

Conversation

@zzylol

@zzylol zzylol commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Stack: #574 → #620 → #618 → #621 → #627 → #625 → #628 → #632 → #634 → #616 → #617 → #622 → #624 → #629 → #630 → #631 → #633 → #635 → #636 → #637

Problem

PhysicalPlanCostModel, PlannerPhysicalPlanProvider and PhysicalEvidenceSnapshot (plan-selection cost/physical_plan_cost_model.rs) priced plans for the legacy selection and dag_export. With dag_export gone (#616, #617), nothing calls them, and Stage 3 never did. Most of asap_types::cost (cost annotations, workload cost summaries) was the dag_export wire format.

Changes

  • Delete crates/plan-selection/src/cost/physical_plan_cost_model.rs and its cost/mod.rs entry.
  • asap_types::cost: keep only CostUnit (still used by plan-selection's CostModel). Delete CostSource, BaselineRef, CostInput, CostAnnotation, benefit_ratio, total_cost, UnitMismatch, sum_workload_costs, WorkloadCostSummary, workload_cost_summary and their tests.
  • analytical_cost::cache_hit_ratios: delete it. Its only callers were tests. The result/buffer hit-ratio fields of ResolvedCacheProfile, which only it read, go too. The fail-closed cache tests now call resolve_cache_profile directly. Two tests drop their hit-ratio assertions and keep the CPU / scan-byte effect assertions.

Not deleted, though no non-test code reaches them any more: query_physical_lowering::lower_query_physical_dag and the analytical whole-DAG comparison (estimate_physical_dag_comparison), together with the storage-I/O / hand-off evidence they consume. They are the natural starting point for #615 (evidence-driven Stage 3 cost), so I left them for that issue to keep or delete.

Stacked on #617.

Test plan

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace: 1615 passed, 0 failed, 13 ignored (after the rebase on main d4869a7; was 1612 passed, 13 ignored)

🤖 Generated with Claude Code

@zzylol
zzylol force-pushed the stack/cleanup-2-dag-export-module branch from 59d531f to 8d8621f Compare October 5, 2026 03:07
@zzylol
zzylol force-pushed the stack/cleanup-3-legacy-cost branch from 7e08c9d to 281858b Compare October 5, 2026 03:07
@zzylol
zzylol force-pushed the stack/cleanup-2-dag-export-module branch from 8d8621f to c64da44 Compare October 5, 2026 06:21
@zzylol
zzylol force-pushed the stack/cleanup-3-legacy-cost branch from 281858b to 84e9cfd Compare October 5, 2026 06:21
PhysicalPlanCostModel, PlannerPhysicalPlanProvider and
PhysicalEvidenceSnapshot had no caller once dag_export was gone, and
Stage 3 never used them. asap_types::cost keeps only CostUnit; the
annotation and workload-summary types were dag_export's. cache_hit_ratios
moves into analytical_cost's tests, its only caller.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the stack/cleanup-3-legacy-cost branch from 84e9cfd to cb02df4 Compare October 8, 2026 16:47
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