Skip to content

[Refactor] Repo-wide naming survey: overloaded and misleading names #948

Open

Description

@milindsrivastava1997

Example

PhysicalCandidate (plan-selection/src/search/candidates/graph.rs:18) is one operator node in the dependency graph. Before configuration it is neither physical nor a whole candidate. CandidateEvaluation, in the same module, evaluates a whole MILP assignment. PhysicalWorkloadPlan (physical-asap-optimization/src/lib.rs:133) documents itself as "One CandidatePhysicalASAPDAG". A reader meets the word "candidate" with six meanings, and the word "physical" with five, before reaching the selection code.

Similarly, PhysicalCandidate.local_cost is documented as "Local work and state only". In practice it is overwritten with lower bounds (costing/lower_bounds.rs:237), refinement weights (refinement/objectives.rs:132, refinement/search.rs:222) and zero placeholders. It is the MILP objective coefficient, not a cost.

Concrete maintenance problem

A repo-wide naming survey found that many central names don't say what the code does. They are overloaded across crates, have several synonyms, or survive from the old whole-workload planner after #933/#934. The findings are listed below. Paths are relative to crates/ unless noted.

1. Overloaded terms (highest impact)

Term Meanings in use
candidate (a) a whole Post-ASAP alternative (glossary); (b) a selectable unit of execution (proposals/joint-planning-search.md); (c) one graph node (PhysicalCandidate); (d) a whole MILP assignment (CandidateEvaluation, EvaluatedCandidate, CandidateSearchResult in refinement/search.rs; SelectedPhysicalCandidate, PhysicalCandidateError in candidates/physical.rs); (e) a rewrite-rule match (SharingCandidate, CapabilitySharingCandidate, PaneCandidate, RollupCandidate, TopKReuseCandidate in asap_aware_cse.rs); (f) a realization method (ComputationCandidate = Exact/ExactAccumulator/Summary, per_computation_candidates.rs:141); (g) the executor's precompute/query split (compile_candidate, CandidateSelection)
physical PhysicalASAPDAG (a timed logical DAG; types/src/ir/physical_export.rs:98); PhysicalWorkloadPlan (a Stage 2 candidate); PhysicalCandidate (an unconfigured graph node); PhysicalOperator lowering; the types::physical module; PhysicalResources (cost types)
memo the Cascades memo (LogicalMemo, MemoGroup, CandidateMemo), and also pointer caches: TimingMemo (types/.../timing.rs:196), type Memo (physical-asap-optimization/src/lib.rs:1132, per_computation_candidates.rs:1567), CompatibilityMemo (candidates/factoring.rs:26), NodeKeys.memo (memo/groups.rs:421)
state summary state; ExecutionDataState / output_state / data_state (edge timing+payload; physical_export.rs:74,91); trait ColState (column-reference representation; operator_properties.rs:12)
population IR MaintainPopulation (set of current series); executor $population column (one series' label map; physical_planner/precompute.rs:16); ExactAccumulator "one population's state" (one group/key); Hydra "subpopulation"
sample raw data sample and summary sample (glossary); limitk/limit_ratio series subsetting (SampleKind, PromqlSeriesSample, frontend Outer::Sample); SamplingKind; SQL sample stddev
raw DataPrimitive::Raw (execution_data_state.rs:36) includes approximate summary estimates. This conflicts with the glossary's "raw data sample" and with reads_raw_at_query_time / query_time_raw_windows.
selection Stage 3 plan selection vs SummaryCoverage.selection (rows covered; summary_coverage.rs:36). Both appear as fields named selection in planner/src/installable.rs.
family / kind SketchKind is a full config struct, but its siblings ExactKind/SamplingKind/WaveletKind are algorithm enums. family is a FieldDataType (SummaryAgg), a SketchAlgorithm (SupportedSummary), and free text (StatModelParams). The executor imports FieldDataType as SummaryFamilyType and then uses SummaryFamilyType::Plain(..) 48 times for ordinary columns.
grouping GroupingStrategy (per-subpopulation vs Hydra layout) sits next to reduction (the actual GROUP BY) on SummaryAgg
pass Stage 1 Pass 1/Pass 2 vs OptimizationPass / Stage1ToStage3Pass (the whole optimizer) vs CandidateOrigin::Pass1
domain PaneWidthDomain, DomainProgress (pane-width search round), DomainOperand (accuracy value ranges), OperandDomain, WeightDomain
configuration CandidateConfiguration (timing, materialization) vs PaneConfiguration (pane width, horizon, sketch size). There are two modules named state_configurations.
catalog SqlCatalog (schemas), HistogramCatalog (metric → sample type), sql-function-catalog (function registry), and the "catalog" of candidates
target AccuracyTarget; per-computation targets (candidate_labels); planner_fuzz::Check.target (a query index)
source (executor) physical_planner::Source (a bound frontier input), Kind::Source(Vec<Batch>), operators/source.rs (Constant/Union/Absent/VectorToScalar), RawSource, MemorySource, and the planner Source
lowering frontend → IR (lower_promql_*, LoweringError) vs logical → PhysicalOperator (design docs)
canonical canonical_metricsql (re-printed text), the canonicalize pass, AggSemantic "canonical semantic", and "canonical intent algebra"

2. Several names for one concept

  • Summary state in memory: AggregateCore (trait), accumulator (*Accumulator, take_accumulator), kernel (summary_kernels), StateColumn, ScalarState (meaning "unkeyed"). There are also two builder types: AccumulatorUpdater and StateColumnUpdater.
  • Reading a value out of a summary: SketchStatistic, SummaryEstimate.query, EvaluatePopulation.evaluation, "readout", StatisticKind, and executor Statistic. In the executor, Kind::KeyedEvaluation (really top-k) is displayed as "SummaryEstimate", while Kind::Evaluation, which implements IR SummaryEstimate, is displayed as "SummaryEvaluation".
  • Which aggregate: AggIntent (types), AggSemantic (sql-function-catalog), Outer/InnerFunc (promql), executor Statistic/Reduction.
  • Producer / consumer: also called supplier / reader. SupplierCandidates is stored in a field of type CompatibleProducers. Counts: producer 528, supplier 182, reader 670, consumer 222.
  • Off-query-path computation: ingestion / precompute / maintenance. Scope::Ingestion { window_start_ms, window_end_ms } is really one pane.
  • A series' label set: labels, $population, $promql_series_identity.
  • The joint search: "memo" (dag-viewer DOM/JSON), --search candidate-connections (CLI), SearchAlgorithm::CandidateConnections (Rust), "Joint search" (UI), MILP. The viewer still maps the stale memo-milp / memo-brute-force names.
  • The candidate set: catalog / inventory / archive. "Inventory" also names two different types: CandidateInventory (the lowered dependency graph) and CandidateSearchContext.inventory: &PerComputationCandidates (the Pass 1 targets).
  • Query outputs: query root / output reference / output group / boundary (BoundaryConsumer).
  • Identical-subexpression sharing: share_common_sub_dags, memo hash-consing, "identical-expression rule".
  • Shared summary builds: summary-capability rule / CapabilitySharingCandidate / CandidateOrigin::SharedBuild / shared_builds. Windows have the same problem: window-composition rule / CandidateOrigin::Panes / PaneCandidate / window_candidates.rs / SharedPaneChain.
  • The materialization cut: frontier / materialized_outputs / precompute "boundary" / InputContract.
  • The materialization choice for one candidate: PhysicalWorkloadPlan.materialization (node → option map), .assignment (Vec<Materialization>), and MaterializationAssignment (a third, pointer-keyed form). The field named assignment is not of type MaterializationAssignment. MaterializationCandidate and is_materialization_point name the same concept.
  • Recurrence: Repeated / Repeating / RepeatedDemand / RepetitionInterval / QueryRecurrence. The unit of execution is likewise called invocation / request / run / evaluation.

3. Names whose meaning no longer matches the code (legacy of #933/#934)

Location Name Reality Suggested
plan-selection/src/lib.rs:1200 PlanSelector (select_plan, offer, finish) The MILP selects. This type validates, prices and archives each assignment. AssignmentEvaluator
plan-selection/src/search/candidates/graph.rs:22 local_cost MILP objective coefficient (lower bound / refinement weight / 0) objective_weight
plan-selection/src/search/pipeline.rs:32-62 LogicalCandidate.choice, .pass2, combinations Always Vec::new() / None (pipeline.rs:279-281, route.rs:907-922). The Cartesian product is no longer enumerated. delete
plan-selection/src/search/pipeline.rs LogicalCandidate, stage1_logical_asap_optimization Logical roots rebuilt from an evaluated assignment after selection EvaluatedAssignmentRoots, evaluated_logical_plans
logical-asap-optimization/src/workload_candidates.rs:32,55 Pass2Candidate, Stage1Candidate Legacy whole-workload rewrite enumeration, now used by query_pruning.rs:325, devtools and tests WorkloadRewrite, or move it to a legacy module
asap_aware_cse.rs (used from memo/rules/*) *Pass2Candidate wrappers Rewritten root sets. Pass 2 vocabulary has leaked into the memo rules. *Rewrite
physical-asap-optimization/src/lib.rs:133 PhysicalWorkloadPlan A Stage 2 physical ASAP candidate; the glossary reserves "physical plan" for execution/deployment PhysicalASAPCandidate
plan-selection/src/lib.rs:1073-1075 CandidateConnectionReport.logical_candidates / immediate_candidates / physical_configurations Counts of ImplementationCandidates, unconfigured nodes, and configured nodes Align with the type names
plan-selection/src/search/candidates/graph.rs:45 ReaderConstraint::ActivationRequires Symmetry-breaking between scope copies, unrelated to readers SelectionConstraint::ScopeCopyOrder
logical-asap-optimization/src/memo/state_groups.rs:26 StateGroupExpansionError Error type for all memo rules MemoRuleError
plan-selection/src/search/pipeline.rs:113 milp_time_limit_ms Whole planning budget (generation + solve + validation) planning_time_limit_ms
refinement/search.rs:37 max_refinements Fed from max_evaluations max_evaluations
devtools/src/bin/stage_pipeline.rs:405,382 --exhaustive, --max-combinations, SearchKind::Exhaustive, EXHAUSTIVE_MAX_COMBINATIONS Keeps all evaluated plans; caps evaluated assignments. The file's own docs deny both "exhaustive" and "Cartesian product". --archive-evaluated, --max-evaluated-assignments
devtools/src/bin/show_logical_dag.rs show_logical_dag Prints the Pre-ASAP IR (Stage 0). "Logical" elsewhere means Stage 1 output. show_pre_asap_ir
devtools/src/bin/stage_pipeline.rs:258 Failure::Usage Also used for queries that don't plan, so it prints USAGE split into Usage / Planning

4. Other misleading names

types / planner / physical-asap-optimization / frontend-common

  • types/src/ir/physical_export.rs:98 PhysicalASAPDAG / module physical_export: post-asap-ir.md says this DAG is logical. Suggested: TimedASAPDAG.
  • types/src/ir/schema/sketch.rs SketchStatistic: covers every summary family, not only sketches. SummaryEstimate.query collides with workload Query (its doc has to warn about this). Suggested: SummaryStatistic, with one field name statistic.
  • types/src/workload/parsed_workload.rs:32 ParsedWorkload.exprs: holds lowered and canonicalized IR roots. Suggested: LoweredWorkload.roots.
  • physical-asap-optimization/src/lib.rs:353 MaterializationCandidate: a materialization point. Suggested: MaterializationPoint.
  • summary_coverage.rs:162 Constraint: per-column value set. It is re-exported by the planner and collides with MILP constraints. Suggested: ColumnConstraint.
  • non_asap.rs:43,159 NonASAPOp::TimeRange + TimeRangeKind::Instant: an instant selector modeled as a range. Suggested: SelectorWindow.
  • planner/src/lib.rs:97 UserInput / frontend_specific: Suggested PlanRequest / frontend.
  • frontend-common/src/schema_resolver.rs:39 SchemaResolver: derives a leaf schema from usage and binds nothing, while resolve.rs uses "resolve" for binding.
  • IngestionPruneReason: also used as an eligibility result. Suggested: IngestionIneligibility.
  • GuaranteeSource: a provenance step. AccuracySpec: vague.
  • StatisticKind::Distinct vs SketchStatistic::Cardinality.

logical-asap-optimization / plan-selection

  • ComputationCandidate: a realization method. Suggested: RealizationMethod; also rename summary_candidates() to summary_families_for_intent.
  • SupplierCandidates, SupplierSetInterner: Suggested CompatibleProducers.
  • CandidateMemo: documented as "the hash-consed workload". Suggested: GroupImplementations.
  • NodeCost.multiplier, CandidateDemand.multiplier: Suggested computations_per_execution.
  • EdgeStatistics describes a node's output. Suggested: OutputStatistics. PropagationStats should be PropagationEvidence.

executor (paths relative to executor/src/)

  • operators/mod.rs:126 Kind::Window (displayed as "WindowAggregate"): also runs HistogramQuantile with window: None, and coordinate is sometimes the le bound.
  • operators/mod.rs:69-111, VectorBinary/RangeWindow/HistogramQuantile vs SeriesBinary/SeriesWindow/SeriesHistogramQuantile: the real difference is label-map rows vs flattened series rows. SeriesLabels does label matching.
  • statistic.rs:4 Statistic: exact-family readout. Only 6 of its variants are reachable. Suggested: ExactStatistic.
  • physical_planner/candidates.rs:9 CompiledPhysicalPlan: a precompute/query split of a CompiledPhysicalDAG. Suggested: PrecomputeQuerySplit.
  • summary_kernels/exact.rs:14 ScalarState (.scalar / .keyed): also used as the per-key value. Suggested: ExactRecord with unkeyed / by_key.
  • key_by_label_values.rs:6 KeyByLabelValues.labels: holds values and is used for SQL item keys too. Suggested: ItemKey { values }.
  • summary_kernels/increase.rs:7 IncreaseAccumulator: also backs Rate. Suggested: CounterAccumulator. Measurement is a bare f64 newtype.
  • module query_time: unused CMS error-bound math. Suggested: sketch_error_estimation.
  • samples.rs:214 TableRowSelector: called a "raw-data-sample window" although it handles SQL rows. Kind::Join is displayed as "RelationalJoin".

frontends / sql-function-catalog / devtools / dag-viewer

  • frontend-promql/src/lib.rs:32,82 and frontend-metricsql/src/lib.rs:80,94: lower_promql_workload vs lower_promql_query_workload (and lower_metricsql vs lower_metricsql_query) differ only in whether scalar roots are allowed.
  • frontend-promql/src/promql.rs:98-171 Outer / OuterIntent / Inner / InnerFunc: named by position, not meaning. Suggested: CrossSeriesAgg / SeriesSelection / RangeFunction. Its comments also mention an "Unresolved Window" node that no longer exists.
  • sql-function-catalog/src/lib.rs:262,277 ClickHouseBuiltin / CLICKHOUSE_BUILTINS: the aggregate table, which includes asap_rate / asap_increase.
  • frontend-sql/src/sql/mod.rs:1243 PlanningBridge: holds relation markers, but the module docs use "bridge" for asap_rate too.
  • frontend-metricsql/src/lib.rs:40 canonical_metricsql: returns normalized query text.
  • Import aliases: UnresolvedOp as Unresolved (promql/sql) vs as U (metricsql); UnresolvedScalar as Scalar; both frontend error types are imported as LoweringError. The lowerers are inconsistent: PromqlLowerer (a unit struct), SqlLowerer, and a bare Lowerer.
  • devtools/src/planner_fuzz/mod.rs:371 Planner: collides with the product name. Suggested: FuzzPlanCache.
  • devtools/src/planner_fuzz/relations.rs:201 Witness: already stored as derivation. Suggested: Derivation.
  • devtools/src/planner_fuzz/mod.rs:352 Priced.derived_root: PrivateNodes: a node set, not a root.
  • devtools/src/rqe_atomic_costs.rs: "RQE" is undefined. Suggested: sketch_bench_operation_costs.
  • devtools/src/lib.rs:1, bin/variant_coverage.rs:1: docs say cargo run -p asap-lower, but the crate is asap-devtools.
  • HistogramKind::RawDataSamples: the metric isn't a histogram. Suggested: HistogramSampleType.

5. Stale documentation

  • docs/design_docs/architecture/asap-aware-plan-search.md and about 10 other docs still reference CandidateLogicalASAPDAGs, TargetSubDAGCandidates, RankedTargetSubDAGCandidates, global_selection and assemble_selected_dag. Apart from one mention in the logical-asap-optimization/src/lib.rs header, none of these exist in code.
  • The glossary's definition of "Candidate" conflicts with proposals/joint-planning-search.md.
  • executor/src/capability.rs:10 refers to a nonexistent binding module.
  • FieldDataType::Sketch doc says SketchKind carries the grouping layout, but grouping is the second tuple field.
  • QueryWorkload.repeating_queries doc says "fixed interval", but RepeatedDemand also allows Scheduled / EstimatedRate.

Behavior that must remain unchanged

Planner output, plan costs, executor results, CLI semantics, and dag-viewer behavior. Renames only, plus deletion of the always-empty legacy fields.

Before and after structure

Before: overlapping terms as listed above. After: each term in sections 1–2 has one meaning, recorded in docs/design_docs/concepts/glossary.md, and the code follows it. Proposed order:

  1. Decide glossary meanings for candidate, physical, memo, state, population, sample, raw, and producer/consumer.
  2. Delete the dead legacy fields (LogicalCandidate.choice, .pass2, combinations).
  3. Rename names that say something false: PlanSelector, local_cost, --exhaustive / --max-combinations, show_logical_dag, SummaryFamilyType, milp_time_limit_ms.
  4. Unify synonyms crate by crate, following the step-1 glossary.
  5. Fix the stale docs.

Why the proposed structure is minimally complex

No new types or layers. Renames and deletions only.

Existing unit and end-to-end protection

Existing unit and integration tests (crates/integration-tests) cover the renamed types. Renames are compile-checked. The dag-viewer and JSON output field renames need the viewer and its consumers updated together.

Compatibility or migration impact

  • Public API renames in asap-types, asap-planner and the frontends affect downstream callers.
  • Devtools CLI flag renames (--exhaustive, --max-combinations) and binary names affect scripts and docs.
  • Report/JSON field renames affect tools/dag-viewer and any saved outputs.

Found by an AI-assisted naming survey (four read-only sub-surveys). Each sub-survey says it read the definition and a few uses of everything it lists. Only a few of the larger claims were re-checked against the code.

🤖 Generated with Claude Code

Activity

  1. zzylol commented on Oct 29, 2026

    @zzylol
    Contributor

    Two *Candidate names that don't match what the type holds. Found while aligning #1047 with #1050 (main at cef05d2). Both types are crate-private and outside #1047/#1050.

    The rule agreed on 2026-10-29 in #1050: *Candidate names one concrete choice the planner can pick. An enum of choice kinds is named for what it describes, so ComputationCandidate → HowToComputeAggregateMeasure. Every other *Candidate type on main points at concrete nodes or plans, except these two:

    1. MaterializationCandidate (physical-asap-optimization/src/lib.rs:353)

      • What it holds: one summary build (node), the queries that read it (readers), whether MatAtIngestion is allowed (at_ingestion), and whether its result may be kept across evaluations (may_keep_across_evaluations).
      • Why the name is wrong: it is a decision point listing the allowed options for one build. The choice is the Materialization picked for it.
      • Name: it was renamed from MaterializationUnit under the earlier "everything choosable is *Candidate" rule; that rename needs revisiting.
    2. SelectedPhysicalCandidate (plan-selection/src/search/candidates/physical.rs:42)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions