Skip to content

refactor(logical-optimizer): move Stage 1's realization helpers out of the legacy search - #631

Draft
zzylol wants to merge 2 commits into
stack/cleanup-6-delete-candidate-selectionfrom
stack/cleanup-7-stage1-helpers
Draft

zzylol wants to merge 2 commits into
stack/cleanup-6-delete-candidate-selectionfrom
stack/cleanup-7-stage1-helpers

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

Rebased: added fix: integrate with #621: the executor's UnivMon test imports default_size_params from pass1::realization.

Problem

Q68 (a) deletes the legacy search (ASAPStrategies, search_workload*, legacy grouping/composition/propagation). Stage 1 still reached into it: pass1/logical_candidates.rs, pass2/summary_capability.rs, pass2/window_composition.rs and the accuracy estimators imported their realization catalogue and summary-input rules from pass1/replacement.rs, pass1/grouping.rs and pass2/reconciliation.rs. Deleting those modules would break the stage pipeline.

Changes

Pure move, no behaviour change:

  • New pass1/realization.rs (Stage 1's realization catalogue and input rules), moved verbatim from the legacy modules:
    • from replacement.rs: Realization, DEFAULT_DELTA, summary_candidates, accuracy_target, accuracy_budget, default_size_params, PhysicalSummaryInput, PhysicalSummaryInputRuleResult, realize_keyed_additive_summary_input, schema_column_ref, summarised_column, column_ref, summarised_input
    • from grouping.rs: has_subpopulations (with its tests)
    • from reconciliation.rs: dominates
  • hydra_guarantee (and its test) moves from grouping.rs to accuracy/estimators/mod.rs, its only Stage 1 caller.
  • The one signature change the move needs: summarised_input returns Result<_, &'static str> instead of the legacy RealizationError; the legacy caller wraps it in RealizationError::PhysicalRealization, with the same message.
  • Doc comments on the moved items no longer link to legacy functions.
  • Callers repointed: the legacy modules, the crate re-exports (Realization, summary_candidates, has_subpopulations now come from pass1::realization), and the external paths in plan-selection's cost_model.rs / empirical_cost.rs and three tests (planner/tests/summary_sharing.rs, frontend-sql/tests/frequency_l2.rs, integration-tests/tests/planner_layering_example2.rs). No re-export stays at the old paths.

After this PR, the stage pipeline's modules (pass1/logical_candidates.rs, pass1/realization.rs, pass2/identical_expressions.rs, pass2/summary_capability.rs, pass2/window_composition.rs, accuracy/estimators/*) import nothing from pass1::{replacement, grouping, exact_composition, rollup, rewrite, explanation, maintained_population} or pass2::{reconciliation, topk_reuse} (checked by grep). accuracy/mod.rs still imports exact_composition::ExactOperation for AccuracyModel::exact_operation_rule. That trait method is dropped in step 8 (Q53).

Stacked on #630.

Test plan

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
  • cargo test --workspace --locked: 1519 passed, 0 failed, 14 ignored (after the rebase on main d4869a7; was 1495 passed, 22 ignored)

🤖 Generated with Claude Code

zzylol and others added 2 commits October 5, 2026 05:58
…f the legacy search

Stage 1 (pass1/logical_candidates.rs, pass2/*) and the accuracy
estimators imported their realization catalogue and summary-input rules
from the legacy search modules (pass1/replacement.rs, pass1/grouping.rs,
pass2/reconciliation.rs). Move those items into a new
pass1/realization.rs module, and hydra_guarantee into
accuracy/estimators, so the stage pipeline no longer reaches the legacy
search. No behaviour change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ization

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the stack/cleanup-7-stage1-helpers branch from a35b447 to c41b508 Compare October 5, 2026 06:21
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