Tracking: remove duplicate, over-abstracted, and dead code in Stages 0–2 #1096

Audit of main @ 4edb47be for duplicate, over-abstracted, dead and overly complex code in Stages 0–2, plus the parts of types and accuracy they use.

Out of scope: executor, plan-selection, cost and the vendored MetricsQL parser. They were only grepped to find callers.

Every "dead" claim was checked by grepping crates/**/*.rs. LOC figures are rough estimates.

Tags: ⭐ high value · (H/M/L) confidence that the change is safe

Stage 0: frontends

Bug

• MetricsQL lowers vector negation as x * -1 (frontend-metricsql/src/lib.rs:157), while PromQL uses PromqlMap{Negative} (promql.rs:396). The comment saying they match is stale, and the mismatch breaks cross-language CSE. (H)

Duplicate

• ⭐ PromQL's range-function table is written twice (promql.rs:564, :1630). Merge it, and drop the InnerFunc enum, which exists only to be mapped to AggIntent. ~150 LOC (H)
• The 3 Outer arms differ only by intent; make them Outer::Reduce(AggIntent). Also dedupe the 2 TopK literals (promql.rs:98). ~60 LOC (H)
• The SQL conjunct/disjunct split and rebuild code repeats DataFusion's split_conjunction, split_binary and conjunction (sql/expr.rs:14,314, sql/mod.rs:2095,2118). ~40 LOC (H)
• PromQL and MetricsQL repeat the workload preamble, the selector lowering, and the single-measure Aggregate literal (6 copies). Share them via frontend-common. ~60 LOC (M)
• 4 hand-written Expr descents; replace them with one children(). Also check whether collect_metric_names skipping agg.param loses sample intervals. ~30 LOC (M)
• Small repeats: blocks in resolve.rs (:153/:171, :575/:627) and the MatchOp mapping written twice in promql.rs. ~20 LOC

Over-abstracted

• SQL: the ACCURACY thread-local and its guard should be a field on SqlLowerer (sql/mod.rs:85-115). ~30 LOC (H)
• PromQL: the 3 thread-locals (accuracy, sample intervals, histogram catalog) should become a Lowerer struct like MetricsQL's. ~70 LOC (M, a lot of signature churn)
• Option<HistogramCatalog> behaves the same as an empty catalog, so the extra entry points and the _inner forwarders can go (frontend-promql/src/lib.rs:56-161, planner/src/lib.rs:99). ~35 LOC (H)
• The SchemaCatalog trait and SchemaResolver<C> exist for a future registry; production only uses the catalog that returns None. Replace them with a free fn. ~45 LOC (H)
• pub mods with no users outside the crate; make them private. Also fix stale or misplaced docs (mark_without, histogram_arg_is_sketchable, current_accuracy). ~20 LOC

Dead / test-only

• No callers: UnresolvedOp::scalar_exprs, HistogramCatalog::is_empty, NativeFunction.arity, and the identity fn quantile_param. ~65 LOC (H)
• Test-only API: lower_sql_batch (known to be wrong, and the only constructor of SqlError::WrongLanguage), lookup_clickhouse_{scalar,window}_builtin, and MetricsQL's lower_metricsql, lower_metricsql_query and canonical_metricsql. ~85 LOC (H/M)
• Needs a human decision: production never sets discriminator_unique_key to Some. It is a deliberate hook from the Concat drops unique_keys unconditionally — no way for a producer to assert cross-branch disjointness #228 design. Keep it or drop it? ~50 LOC

Unnecessarily complex

• walk checks is_*_fn guards and then matches the same names again; use a single name match (promql.rs:370-394). ~35 LOC (M)

Stage 1: logical-asap-optimization

Duplicate

• ⭐ The old whole-DAG Pass 2 enumerator (pass2/workload_candidates.rs and most of pass2/asap_aware_cse.rs) runs in parallel with the memo rules. Its only production caller is plan-selection/.../query_pruning.rs:326; move that to the memo path and retire the old enumerator. ~1000 production + ~500 test LOC (M: check that pruning verdicts don't change)
• same_family appears twice; use cse::same_value. Separately, rename asap_aware_cse.rs:758 same_family, which shares the name but has a different meaning. ~10 LOC (H)
• The memo rules repeat the same boundary and DAG-walk code; add boundary_ptrs and nodes_within helpers. ~60 LOC (M)

Over-abstracted

• memo/window_candidates.rs and memo/shared_configurations.rs have chains of public wrappers that only tests use; collapse each to one entry. ~45 LOC (H)
• The viable: &dyn Fn parameter is always |_| true. This goes away if the enumerator is retired. ~25 LOC (H)
• Small items, ~25 LOC (H):
  • PrunedCandidate<C> has one instantiation and its name collides with plan-selection's; make it concrete.
  • computation_choices_cache should be a local instead of a parameter.
  • memo/candidates.rs is only a re-export shim.

Dead / test-only

• with_shared_candidates, StateGroupExpansionError::InvalidReaderBoundary, and the test-only may_change_rollup_candidate. ~20 LOC (H)

Unnecessarily complex

• rules/rollup.rs add_rollup_candidates is 207 lines and rediscovers facts the tree rule already knows; split it up. ~40 LOC (M)
• About 100 check_generation_deadline calls are nested at every level; one per outer loop is enough. ~40 LOC (M)
• The root-dependency check and validate_structure each run twice. ~10 LOC (M)

Layering

• pass2/memo.rs and pass2/cse.rs run before Pass 1, so pass1 currently imports pass2; move both out of pass2. 0 LOC (H)

Stage 2: physical-asap-optimization

Duplicate

• The prologue is copied 4× in lib.rs; extract it into a helper. ~30 LOC (H)
• Use the helpers already in types: QueryRoot::operator_refs instead of root_operators, and stages::output_references instead of ordered_output_references. ~20 LOC (H)
• The five-arm ReaderConstraint match appears 5×, one of them in plan-selection; add ReaderConstraint::candidate_mut(). ~45 LOC (H)
• inventory.rs has an inline copy of ordered_inputs. ~5 LOC (H)

Over-abstracted

• ⭐ summary_window.rs defines PaneLayout, WindowEdgeCoverage, PaneCoverageError and serde support for one call site that always passes Some(0). Replace it with one schedule_phase fn. ~140 LOC (H)
• catalog/realize.rs takes materializations as a parameter, but it can be derived from the selected candidates. ~15 LOC (M)

Dead / test-only

• optimize_physical_asap_assigned has one devtools caller and physical_candidate_count is used by one test. ~25 LOC (H)

Unnecessarily complex

• The summary_materialization_eligibility wrapper and SummaryEligibilityError add nothing; call summary_reader_eligibility directly. ~45 LOC (H)
• The catalog is validated 2–3× per stage, and reconstruction.rs re-validates per plan inside the refinement loop; validate once per stage. Also fix the stale doc at plan-selection dag.rs:322. ~30 LOC (M-H)
• The boundary checks in lib.rs:1017-1043 are redundant, and the ImplementationBoundary error is never matched. ~22 LOC (M)
• configurations.rs has a timing-indexed array plus a second cache. ~20 LOC (L-M)

Shared code (types, accuracy)

Duplicate

• reads_only_time_series has 3 identical copies (types, Stage 1, Stage 2); keep one on OperatorNode. ~12 LOC (H)
• The deadline checker is reimplemented in about 8 places; add one check(deadline) helper. ~50 LOC (H)
• rolled_up_build and summary_coverage::regrouped build the same rolled-up SummaryAgg; share the builder. ~25 LOC (M)

Over-abstracted

• ⭐ Accuracy has about 20 test_support forwarders and default_size_params. Move the propagation tests into crates/accuracy, then delete them. ~150 LOC (M-H)
• Small items, ~25 LOC (L-M):
  • MemoGroup has public fields and getters; keep one.
  • ColState is a marker trait; inline its serde bounds.
  • The physical/ module holds one file.

Dead / test-only

• ⭐ Deadline plumbing production never uses: flatten_before, compile_physical_asap_workload_with_node_ids_before, expand_pane_chains_in_workload_before, the set_retention deadline param (every caller passes None), and their error enums. ~200 LOC (H)
• ⭐ Reserved ASAPOp variants SummarySubtract, SummaryDelete, SummaryJoin, Extension and UNIMPLEMENTED_ASAP_OP. Nothing constructs them; their match arms only reject them. ~100 LOC (H)
• ⭐ Speculative FieldDataType::SummarySample, Wavelet and StatModel, plus their Kind/Params types. ~110 LOC (H)
• Test-only OperatorNode::validate_execution_timing repeats the ingestion-timing rule in timing.rs. ~25 LOC (H)

Unnecessarily complex

• PhysicalASAPDAGEdge stores copies of schema and state and then validates them (~70 lines); slim it to (producer, consumer, role). This changes the wire format, so snapshots and the dag-viewer need updating. ~100 LOC (M)
• The same invariants are checked in both timing.rs and validate_inputs. Also, there are two selects_states with different meanings; rename one. ~30 LOC (M)

Residue

• The serde name ExactBoundaryResidual doesn't match the Rust variant name ExactWindowEdgeResidual.

Totals

About 2,900 production LOC in all. About 1,000 of that is the Pass 2 enumerator. About 900 is high-confidence deletion that should need no behavior change.