Skip to content

Remove dead Pass 2 / choice candidate metadata and labels #997

Closed

Description

@milindsrivastava1997

Production planning (memo/joint search) always builds LogicalCandidate with choice: Vec::new() and pass2: None (plan-selection/src/search/pipeline.rs:281, route.rs:939,952). Only the test helper explicit_assignment_plans (integration-tests/src/lib.rs) populates them, and its sole caller (memo_covers_whole_workload_pass2.rs) never reads them. Everything below is therefore dead or test-only. (Found by grep at b318df5.)

Dead structs / fields

  • LogicalCandidate.choice, LogicalCandidate.pass2 (plan-selection/src/search/pipeline.rs)
  • Pass2Candidate and all its fields (logical-asap-optimization/src/workload_candidates.rs)
  • Stage1Candidate.choice, Stage1Candidate.pass2 — only roots is read (query_pruning.rs:325)
  • pub use ...::Pass2Candidate re-export (devtools/src/planning_stages.rs:7)

Dead code paths

  • choice_candidates(.., from, ..): from is always 0; fold into stage1_candidates_of_choice
  • devtools/src/candidate_labels.rs:
    • candidate_labels: the choice_label(...) branch and the if let Some(pass2) block
    • choice_label, pass2_label
    • helpers likely only used by those two: for_queries, readers_in, readers, same_computation, is_pane_chain_of, same_pane_chain, state_name, duration_name (verify each when deleting)

Not dead (keep)

  • Pass2 / Pass2List / pass2_candidates: still used via alone_violation in query_pruning.rs
  • *Pass2Candidate structs in asap_aware_cse.rs: consumed by pass2_candidates

Proposed cleanup

  • Make stage1_candidates_of_choice return the roots only.
  • Remove the fields and functions listed above.
  • Update the LogicalCandidate constructors: count_planning.rs, promql_numeric_regressions.rs, precompute_candidates.rs, pipeline.rs, route.rs, and explicit_assignment_plans.

Activity

  1. milindsrivastava1997 commented on Oct 28, 2026

    @milindsrivastava1997
    CollaboratorAuthor

    @zzylol can you check if this makes sense?

  2. zzylol commented on Oct 28, 2026

    @zzylol
    Contributor

    Checked against main at 93c90aca. The finding holds: production search builds every LogicalCandidate with choice: Vec::new() and pass2: None (now pipeline.rs:301, route.rs:1002,1015), and only explicit_assignment_plans fills them. The "Not dead (keep)" section is right too: alone_violation (query_pruning.rs:325) reads only roots.

    Corrections before deleting:

    1. Keep for_queries, duration_name and query_names. physical_label calls them (candidate_labels.rs:559,563), and production still uses it. The helpers that are only used by choice_label / pass2_label are readers_in, readers, same_computation, is_pane_chain_of, same_pane_chain and state_name.
    2. More tests to update: the unit tests in candidate_labels.rs (#[cfg(test)] from line 659) and workload_candidates.rs (line 507 asserts candidate.pass2), plus the from_pass1_position assignment in integration-tests/src/lib.rs:597.
    3. What remains afterwards. The whole-workload Pass 2 (pass2_candidates / Pass2List) will then have one production user, the alone_violation pruning check, alongside the memo Pass 2. That is the "Pass 2 exists twice" item in Stage 1 Pass 2 review: ASAP-aware CSE rules over memo groups and summary-state groups #1013. This issue can stay a pure deletion, and moving alone_violation onto the memo Pass 2 is tracked there.

    Linked as a sub-issue of #1013.

  3. removed their assignment
    on Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    optimizerWorkload optimization and plan selectionrefactor

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions