Skip to content

refactor(controller): retire algebra/+planner/+config/+query_language/+analyzer.rs (Steps 2b/c/d/e/f) - #130

Merged
zzylol merged 1 commit into
mainfrom
refactor/controller-layered-cleanup
May 11, 2026
Merged

zzylol merged 1 commit into
mainfrom
refactor/controller-layered-cleanup

Conversation

@zzylol

@zzylol zzylol commented May 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Per docs/design.md §5 target layout. Restructure controller/src/ from the historic mixed layout (algebra/ + planner/ + config/ as ill-defined catch-alls, plus stage_split.rs + analyzer.rs scattered top-level files) into the design.md modular split.

This is Step 2b/c/d/e/f from the architectural cleanup chain that started with #129 (Step 2a — capability consolidation).

Old → new path mapping

Old New
algebra/expr.rs intent_algebra/legacy_expr.rs (kept; 91 consumers — unification follow-up)
algebra/lower.rs intent_algebra/legacy_lower.rs
algebra/{directory,physical,allocator,plan}.rs physical/{sketch_catalog,planner,allocator,plan}.rs
algebra/optimizer.rs optimizer/engine.rs
planner/{cost_model,delta,online,pareto,tco,wire_cost}.rs optimizer/cost/{mod,delta,online,pareto,tco,wire}.rs
planner/rules.rs optimizer/rules/mod.rs
planner/baseline_planner.rs optimizer/baseline.rs
planner/stage_split.rs physical/stage_split.rs
analyzer.rs pipeline.rs
stage_split/ physical/colored_dag/ (colored_dag.rs → dag.rs)
query_language/ query_parser/language/
config/workloads.rs workload.rs
config/{agent,backend,asapquery_backend,precompute,stage_config,stage_config_otap,stage_config_telegraf}.rs emit/{agent,backend,asapquery_backend,precompute,stage_config,otap,telegraf}.rs
(new) physical/topology.rs, emit/trait_def.rs, optimizer/trait_def.rs, deployment_model.rs

Pragmatic concessions (4 documented TODOs in §16.2)

  1. algebra/expr.rsintent_algebra/legacy_expr.rs — the original brief assumed only one stray consumer existed of algebra::expr::AggIntent. Reality: ~91 sites across query_parser, language_logical_plan, planner, algebra, config use the legacy QueryExpr/AggIntent/WindowSpec IR. Treating these as "duplicates to delete" would have required rewriting every consumer. The file is moved with a doc-comment marking it as legacy IR pending unification with canonical intent_algebra::{agg_intent,query_expr,schema}. Same for algebra/lower.rsintent_algebra/legacy_lower.rs.
  2. config/stage_config.rs (3,020 lines) moved whole as emit/stage_config.rs rather than split into emit/{opamp,streaming_config,inference_config}.rs. The monolith mixes OTel-collector YAML, ASAPQuery-backend JSON, storage-routing JSON, and shared internals; a clean split needs ownership reorganisation, not file renames.
  3. Back-compat module aliases in lib.rs (pub use intent_algebra::legacy_expr as algebra; etc.) — controller/src/main.rs references crate::stage_split::*, crate::algebra::*, etc. (bin crate). Keeping shims lets main.rs continue compiling without per-line edits.
  4. hll_accuracy() / countmin_accuracy() moved from legacy_expr.rssketch_algebra/capability.rs (their structural home). legacy_expr.rs retains thin pub use re-exports.

docs/design.md updates

  • §3 layer table — rows rewritten with new paths + *Refactor 2026-05 absorbed …* italicised notes.
  • §5 Target repo layout — blockquote: multi-crate split deferred; lookup table mapping crates/core/* → single-crate paths.
  • §6 Core crate details — blockquote: 16-row lookup table.
  • §16 NEW — ADR section. §16.1 documents PR refactor: consolidate Capability into sketch_algebra; warm_tier_analysis becomes facade (Step 2a) #129's capability consolidation; §16.2 documents this PR's retirement with the full mapping table and four TODOs.

Build + test

Diff: 55 files changed, +568 / -309

(File renames + import-path rewires. Net additions are mostly the new placeholder trait files + the design.md §16 ADR.)

Follow-up PRs queued

  1. Migrate consumers of intent_algebra::legacy_expr::* onto canonical types; delete the legacy_*.rs files.
  2. Split emit/stage_config.rs (3,020 lines) per design.md §5.
  3. Implement the placeholder traits (OptimizerRule, PlanEmitter, DeploymentModelRegistry).
  4. Migrate controller/src/main.rs off the back-compat module aliases; remove the shims.
  5. sketch_algebra/ touch-up (the user's open request): drop AggIntent::HistogramQuantile; map Min/Max → QuantileApprox; consolidate SketchCapability vs SketchCapabilities via the "rename schema.rs::SketchCapabilities → SketchStateMetadata" approach; rename params.rssketch_params.rs; add FrequencyEstimate(SketchKindHandle) capability variant + CountSketchWithHeap handle. PromQL/MetricsQL surface: sum by (item) (rate(m[r]))FrequencyEstimate; topk(k, …)FrequencyTopk.

🤖 Generated with Claude Code

…/+analyzer.rs

Per design.md §5 target layout. Restructure controller/src/ from the
historic layout (algebra/ + planner/ + config/ as ill-defined catch-alls
mixed with stage_split.rs + analyzer.rs scattered top-level files) into
the design.md modular split: query_parser/ (L1) → language_logical_plan/
(L2) → intent_algebra/ (L3) → sketch_algebra/ (L4 IR) → optimizer/ (L4
framework) → physical/ (L5 framework) → emit/ (L5 emitters) → pipeline.rs
(L1→L5 driver).

This is Step 2b/c/d/e/f from the architectural cleanup chain that started
with #129 (Step 2a — capability consolidation).

## Old → new path mapping

| Old | New |
|---|---|
| `algebra/expr.rs` | `intent_algebra/legacy_expr.rs` (kept; 91 consumers — unification is a follow-up) |
| `algebra/lower.rs` | `intent_algebra/legacy_lower.rs` |
| `algebra/{directory,physical,allocator,plan}.rs` | `physical/{sketch_catalog,planner,allocator,plan}.rs` |
| `algebra/optimizer.rs` | `optimizer/engine.rs` |
| `planner/{cost_model,delta,online,pareto,tco,wire_cost}.rs` | `optimizer/cost/{mod,delta,online,pareto,tco,wire}.rs` |
| `planner/rules.rs` | `optimizer/rules/mod.rs` |
| `planner/baseline_planner.rs` | `optimizer/baseline.rs` |
| `planner/stage_split.rs` | `physical/stage_split.rs` |
| `analyzer.rs` | `pipeline.rs` |
| `stage_split/` | `physical/colored_dag/` (with colored_dag.rs → dag.rs) |
| `query_language/` | `query_parser/language/` |
| `config/workloads.rs` | `workload.rs` |
| `config/{agent,backend,asapquery_backend,precompute,stage_config,stage_config_otap,stage_config_telegraf}.rs` | `emit/{agent,backend,asapquery_backend,precompute,stage_config,otap,telegraf}.rs` |
| (new) | `physical/topology.rs`, `emit/trait_def.rs`, `optimizer/trait_def.rs`, `deployment_model.rs` |

## Pragmatic concessions (documented TODOs)

- **`algebra/expr.rs` (1,230 lines) → `intent_algebra/legacy_expr.rs`**:
  the brief assumed only one stray consumer of `algebra::expr::AggIntent`
  existed, but reality is ~91 sites across query_parser, language_logical_plan,
  planner, algebra, and config use the legacy `QueryExpr` / `AggIntent` /
  `WindowSpec` IR. Treating these as "duplicates to delete" would have
  required rewriting every consumer in one PR. Moved the file with a
  doc-comment marking it as legacy IR pending unification with the
  canonical `intent_algebra::{agg_intent,query_expr,schema}` types.
  Same for `algebra/lower.rs` → `intent_algebra/legacy_lower.rs`.

- **`config/stage_config.rs` (3,020 lines)** moved whole as
  `emit/stage_config.rs` rather than split into the design.md-prescribed
  `emit/{opamp,streaming_config,inference_config}.rs`. The monolith
  mixes OTel-collector YAML emit, ASAPQuery-backend JSON emit,
  storage-routing JSON emit, and many shared internals; a clean split
  needs ownership reorganisation, not file renames.

- **Back-compat module aliases** in `lib.rs` (`pub use intent_algebra::
  legacy_expr as algebra;` etc.) — `controller/src/main.rs` references
  `crate::stage_split::*`, `crate::algebra::*`, `crate::planner::*`,
  `crate::config::*`, `crate::analyzer::*` (the bin crate, not the lib).
  Keeping shims in `lib.rs` lets main.rs continue compiling without
  per-line edits. Remove the shims once main.rs migrates.

- **`hll_accuracy()` / `countmin_accuracy()`** moved from `legacy_expr.rs`
  → `sketch_algebra/capability.rs` (their structural home — sketch-family
  error bounds). `legacy_expr.rs` retains thin `pub use` re-exports so
  in-file callers (e.g. `AggIntent::default_cardinality`) keep
  compiling.

## docs/design.md updates

- §3 layer table — rows rewritten with new `controller/src/*` paths +
  italicised `*Refactor 2026-05 absorbed ...*` notes.
- §5 Target repo layout — prepended blockquote: multi-crate split
  deferred until ≥2 deployment models ship; lookup table mapping
  `crates/core/*` targets to current single-crate paths.
- §6 Core crate details — prepended blockquote: 16-row lookup table.
- **§16 NEW** — ADR section. §16.1 documents PR #129's capability
  consolidation; §16.2 documents this PR's retirement plan with the
  full mapping table and four TODOs.

## Build + test

- `cargo build --release -p controller` → clean (4 warnings, pre-existing)
- `cargo build --release -p query_engine_rust` → clean (3 warnings)
- `cargo test -p controller --lib` → **633/633 pass** (unchanged from #129)
- `cargo test -p query_engine_rust --lib -- engines::warm_tier` → **13/13 pass**

## Diff: 55 files, +568 / -309 (file renames + import-path rewires)

## Follow-ups

1. Migrate consumers of `intent_algebra::legacy_expr::*` onto canonical
   `intent_algebra::{query_expr,agg_intent,schema}` types; delete the
   `legacy_*.rs` files.
2. Split `emit/stage_config.rs` (3,020 lines) per design.md §5.
3. Implement the placeholder traits (`OptimizerRule`, `PlanEmitter`,
   `DeploymentModelRegistry`). Existing free-function emitters + rule
   loops keep behaviour stable.
4. Migrate `controller/src/main.rs` off the back-compat module aliases
   in `lib.rs` (`crate::algebra::*` etc.); remove the shims.
5. (Separate task #32) `sketch_algebra/` touch-up: drop
   `AggIntent::HistogramQuantile` from semantic IR; map Min/Max to
   QuantileApprox; consolidate SketchCapability vs SketchCapabilities;
   rename params.rs → sketch_params.rs; add `FrequencyEstimate`
   capability variant + `CountSketchWithHeap` handle.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 0bda240 into main May 11, 2026
zzylol added a commit that referenced this pull request May 11, 2026
…DO 3+4) (#132)

Two of the four PR #130 follow-up TODOs land here. The other two are
blocked by architectural reality (see below).

## TODO 3 — Placeholder traits made functional ✅

### `OptimizerRule` trait
- `controller/src/optimizer/trait_def.rs`: extended trait with `name()`,
  `category()`, `apply()`. Added two new `RuleCategory` variants
  (`Cse`, `Decorrelate`) beyond the original four.
- `controller/src/optimizer/engine.rs`: per-rule `OptimizerRule` impls
  for all 12 engine rules (delegating to the existing `RewriteRule`
  free fns). New `default_rules_as_optimizer_rules()` helper returns
  `Vec<Box<dyn OptimizerRule>>` for callers that want the trait surface.
- `controller/src/optimizer/mod.rs`: blanket impl for every
  `sketch_algebra::rules::Rule` (lives outside sketch_algebra/ to
  respect Step #32's territory boundary).

### `PlanEmitter` trait
- `controller/src/emit/trait_def.rs`: kept the associated-type shape;
  added 4 concrete impls — `OpampEmitter`, `OpampGatewayEmitter`,
  `StreamingConfigEmitter`, `InferenceConfigEmitter` — each wraps the
  existing free-function emitter. `InferenceConfigInput<'a>` bundles
  per-tenant routing inputs (tenant id + metric plans + Mode-3
  metrics). `emit_borrowed` variant avoids forcing `'static` on the
  trait-method path.

### `DeploymentModelRegistry`
- `controller/src/deployment_model.rs`: extended from stub
  `HashMap<DeploymentModelId, ()>` to a real `HashMap<…, DeploymentModel>`
  where `DeploymentModel` owns `(id, rules: Vec<Box<dyn OptimizerRule>>,
  emitters: EmitterSet)`. `Default::default()` registers
  `asaplifecycle` with the engine's 12-rule library + the 4 demo
  emitter names so `pipeline::run_pipeline` doesn't need manual
  registration.

## TODO 4 — main.rs migrated off back-compat aliases ✅

Rewrote all 52 reference sites in `controller/src/main.rs`:
- `algebra::*` → `optimizer::engine::*` / `physical::*` /
  `intent_algebra::legacy_expr::*` (depending on context)
- `analyzer::*` → `pipeline::*`
- `config::*` → `emit::*` / `workload::*`
- `planner::*` → `optimizer::*` / `physical::stage_split` /
  `optimizer::cost::online as online_cost_model`
- `stage_split::*` → `physical::colored_dag::*`

Removed all back-compat shims from `lib.rs`:
- `pub use emit as config`
- `pub use pipeline as analyzer`
- `pub mod algebra { … }`
- `pub use physical::colored_dag as stage_split`
- `pub mod planner { … }`

One leftover internal ref (`crate::planner::rules::bind_workload_typed`
inside `emit/mod.rs::collect_metric_to_family`) retargeted to
`crate::optimizer::rules::bind_workload_typed`.

`lib.rs` shrank from 184 → 95 lines.

## TODO 1 — legacy_expr migration: BLOCKED 🚫

The canonical `intent_algebra::{query_expr,agg_intent,schema}` is NOT a
structural superset of the legacy `intent_algebra::legacy_expr`. The
canonical `QueryExpr` has 5 variants (`Scan`, `Window`, `Aggregate`,
`LetBinding`, `Ref`); legacy has ~26 (`Source`, `Filter`, `Project`,
`Aggregate`, `Window`, `SketchAgg`, `WindowedAgg`, `Partition`, `Dedup`,
`TopK`, `Merge`, `Join`, `JoinSketch`, `SetOp`, `Sort`, `Limit`,
`Subquery`, `LetBinding`, `WindowFunc`, `HistogramQuantile`,
`PromQLSubquery`, `BinaryOp`, `Ref`) plus a full `ScalarExpr` IR
(Column, Literal, BinaryOp, UnaryOp, FunctionCall, ScalarSubquery,
InList, InSubquery, Between, IsNull, Case, Cast, VectorBinaryOp).

The canonical IR's own module docs flag this: *"the full design.md list
is larger; they are deferred to follow-up phases as the planner grows
consumers for them"*.

Migrating consumers requires first GROWING the canonical IR to absorb
these variants — a substantial separate refactor, not a follow-up
cleanup. 50+ consumer sites in query_parser/, language_logical_plan/,
optimizer/engine.rs, physical/{allocator,planner,plan,stage_split}.rs
depend on the legacy variant set.

Recommend retitling the follow-up as "grow canonical L3 to absorb
legacy variants" rather than "migrate consumers". The legacy_expr
module stays in place pending that work.

## TODO 2 — stage_config.rs split: DEFERRED 🟡

The 3,020-line `emit/stage_config.rs` monolith mixes OTel YAML emit,
backend JSON emit, and storage-routing emit with intricately shared
helpers (`sketch_kind_to_processor_name`, `build_otlp_exporter`,
`sketch_kind_tag`, …) and 1,580 lines of tightly-interleaved tests
(line 1441 onward).

The wrapper structs from TODO 3 above already provide the polymorphic
`PlanEmitter` surface (`OpampEmitter` / `StreamingConfigEmitter` /
`InferenceConfigEmitter`), so the file-level split is no longer
load-bearing for the trait work. Recommend addressing as a separate
PR with dedicated time budget if/when the monolith becomes a
review-bottleneck.

## Build + test

- `cargo build --release -p controller` — clean
- `cargo build --release -p query_engine_rust` — clean
- `cargo test --release -p controller --lib` — **646 passed** (was 633; +13 trait tests)
- `cargo test --release -p controller --bin controller` — **27 passed**
- `cargo test --release -p query_engine_rust --lib -- engines::warm_tier` — **13 passed**

## Caveat — `controller/src/store/`

PR #130 left `lib.rs` declaring `pub mod store;` but `controller/src/store/`
is in `.gitignore` (line ~10 of repo root .gitignore — the broad
`store/` pattern). The directory is untracked; this needs a separate
decision on whether it should be tracked or remain a runtime artifact.
The build works because the directory exists locally on the dev box;
fresh clones would fail. Not in scope for this PR; flagging as
follow-up.

## Diff: 8 files, +785 / -207

Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 11, 2026
… + dedupe (#133)

User-flagged cleanup of sketch_algebra/ after PR #130's layered refactor.

## 1. Drop `AggIntent::HistogramQuantile`

`histogram_quantile()` is a PromQL/MetricsQL language-level operator,
NOT a semantic intent. Removed the variant from
`intent_algebra::agg_intent::AggIntent`. The PromQL operator still
lives on as `intent_algebra::legacy_expr::QueryExpr::HistogramQuantile`
(unreachable from canonical L3 — it was never produced by the
lowerer, but PR #130's legacy_expr preservation keeps it for back-compat
until that module is fully retired in a separate follow-up).

## 2. `AggIntent::Min` + `AggIntent::Max` → `QuantileApprox(Any)`

Quantile sketches (DDSketch, KLL) answer min = quantile(0) and
max = quantile(1) directly. Previously these returned `None` from
`capability_for`; now they map to warm-tier-answerable.

## 3. Consolidate `SketchCapabilities` ↔ `SketchCapability` (Option A)

Renamed `controller/src/sketch_algebra/schema.rs::SketchCapabilities`
→ `SketchStateMetadata` to remove the name collision with
`capability.rs::SketchCapability`. Both files keep their roles:
- `schema.rs::SketchStateMetadata` = L4 type-system role (wraps
  `SketchStateSchema` with sketch-family metadata per design.md §6.4).
- `capability.rs::SketchCapability` = perf/feasibility/intent-routing
  metadata (consumed by optimizer + cost model).

## 4. Rename `params.rs` → `sketch_params.rs`

Naming clarity. The file holds `SketchParams` + its KLL/DDSketch/Hll/
Cms/CountSketch variants. `sketch_algebra/mod.rs` retains
`pub use sketch_params as params;` back-compat alias so the ~10
in-tree `crate::sketch_algebra::params::*` call sites keep
compiling without further migration.

## 5. Frequency Capability surface: split FrequencyTopk vs FrequencyEstimate

User-clarified MetricsQL surface mapping:

| MetricsQL surface | AggIntent | Capability |
|---|---|---|
| `sum by (item) (rate(m[r]))` w/ Epsilon | `Frequency{accuracy}` | `FrequencyEstimate(Any)` |
| `topk(k, sum by (item) (rate(m[r])))` | `TopK{k, accuracy}` | `FrequencyTopk(CmsWithHeap)` |
| (Exact) | (any) | `None` — warm-tier doesn't carry exact |

Concrete changes in `capability.rs`:
- Added `Capability::FrequencyEstimate(SketchKindHandle)` (NEW —
  heap-LESS; answers bare per-key frequency without top-k extraction).
- Existing `Capability::FrequencyTopk(SketchKindHandle)` stays
  (heap-BEARING; answers top-k).
- Added `SketchKindHandle::CountSketchWithHeap` variant (alongside
  existing `CmsWithHeap`).
- Updated `capability_for`:
  - `Frequency{!Exact}` → `Some(FrequencyEstimate(Any))` (was
    `FrequencyTopk(CmsWithHeap)` — wrong, that's for top-k only)
  - `TopK{Exact}` → `None` (was `FrequencyTopk` — exact top-k uses
    HashAgg+Heap, not a sketch)
- Updated `is_satisfied_by`:
  - `FrequencyTopk(req)`: indexed must be `FrequencyTopk(have)` with
    `have ∈ {CmsWithHeap, CountSketchWithHeap}`. Rejects heap-less.
  - `FrequencyEstimate(req)`: indexed `FrequencyEstimate` with any
    frequency-family handle OR `FrequencyTopk` with heap-bearing
    handle (heap is additional info — underlying matrix answers the
    point query).

## Backend wire-in

- `asap-query-engine/src/engines/warm_tier/sketch_reducer.rs`: new
  `QueryFamily::FrequencyEstimate` dispatch arm + heap-bearing
  variant routing. New `decode_frequency_total` helper emits
  per-window row-0 sum of the CMS/CountSketch matrix as the
  total-frequency scalar.
- `asap-query-engine/src/drivers/ingest/otel.rs`: ingest-side
  capability mapping at sid registration time — heap-less sketches
  classify as `FrequencyEstimate`, heap-bearing as `FrequencyTopk`.

## Build + test

- `cargo build --release -p controller` — clean
- `cargo build --release -p query_engine_rust` — clean
- `cargo test --release -p controller --lib` — **643/643 pass**
  (was 633 in PR #132; +10 new tests covering Min/Max, FrequencyEstimate
  semantics, is_satisfied_by wildcard + heap rules)
- `cargo test --release -p query_engine_rust --lib -- engines::warm_tier`
  — **13/13 pass** (FrequencyEstimate ingest+reducer arm landed
  but no new fixture; the existing arms already exercise the
  Capability-keyed dispatch)

## Diff: 12 files, +546 / −140

## Follow-ups (out of scope)

- **FrequencyEstimate per-key lookup**: today's `decode_frequency_total`
  returns the row-0 sum (i.e., total frequency across the entire CMS
  matrix). A real per-key `frequency(metric, key)` reducer needs the
  key as an arg, which today's `function_args: &[f64]` signature
  can't carry. Documented inline.
- **`AggIntent::HistogramQuantile` ghost in legacy_expr**: until
  legacy_expr is fully retired (PR #130 TODO 1), the variant lives
  on as a PromQL-operator placeholder. Removal happens with the
  legacy retirement.

Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 11, 2026
… + typed Predicate (#136)

Additive lift of all 10 A-classified legacy variants into the canonical
`intent_algebra::query_expr::QueryExpr`, plus the minimal typed
`Predicate` IR for `Filter.pred` covering the 4 used-and-cleanly-shaped
ScalarExpr variants.

## What landed

### Canonical `QueryExpr` grew from 5 → 15 variants

Pre-existing (PR #130 / canonical): `Scan`, `Window`, `Aggregate`,
`LetBinding`, `Ref`.

New (this PR): `Filter`, `Project`, `Partition`, `Distinct`, `Merge`,
`Join`, `SetOp`, `Sort`, `Limit`, `BinaryOp`.

Naming convention: single-input variants use `child:` (matches existing
canonical `Window`/`Aggregate`/`LetBinding`). Multi-input variants
follow design.md §6 shapes exactly: `Merge { children }`,
`Join { kind, pred, left, right }`, `SetOp { kind, all, left, right }`,
`BinaryOp { op, lhs, rhs, vector_match }`.

### Supporting types lifted alongside

`ColumnRef`, `PartitionKeys`, `BinaryOpKind`, `JoinKind`, `SetOpKind`,
`SortKey`, `VectorMatch`/`VectorMatchKind`/`VectorGrouping`/`GroupSide`,
`LiteralValue`, `ProjectItem`. All re-exported from
`intent_algebra::mod.rs`.

### New typed `Predicate`

```rust
pub enum Predicate {
    Column(ColumnRef),
    Literal(LiteralValue),
    BinaryOp { op: BinaryOpKind, lhs: Box<Predicate>, rhs: Box<Predicate> },
    IsNull { expr: Box<Predicate>, negated: bool },
}
```

Plus `Predicate::from_legacy_scalar(&legacy::ScalarExpr) ->
Result<Predicate, QueryExprError>`. Translates the 4 supported
variants (Column / Literal / BinaryOp / IsNull); returns
`QueryExprError::UnsupportedLegacyScalar(name)` for the 4 E-deferred
(FunctionCall / ScalarSubquery / InList / Between). Per user decision,
those stay in legacy until a real consumer demands the typed shape.

Translation handles `LiteralValue::Duration → Int(nanos)` fold (canonical
LiteralValue is deliberately narrower).

### Consumer migration deferred to subsequent batches

**Zero consumer-site redirects in this PR** — and that's the right
call. Every legacy-A-variant consumer (`query_parser/`, `physical/
{stage_split,planner,allocator}.rs`, `optimizer/engine.rs`,
`legacy_lower.rs`) simultaneously matches A-variants AND C-variants
(`Source`, `Aggregate`, `Window`, `SketchAgg`, `WindowedAgg`, `TopK`,
`HistogramQuantile`, `PromQLSubquery`) AND constructs `ScalarExpr`
predicates with E-deferred variants. Migrating them mid-pipeline would
require lifting C-variants and E-variants in the same PR — explicit
out-of-scope per the batch plan.

The consumers migrate one C-batch at a time:
- Batch 3a (PR #11): `AggFunc → AggIntent`
- Batch 3b (PR #10): `TopK` split
- Batch 3c (PR #8): `WindowedAgg` un-fusion
- Batch 3d (PR #9): `SketchAgg → Aggregate@L3`
- Batch 3e (PR #12): PromQL parser `histogram_quantile → Quantile`
- Batch 13: retire `legacy_expr.rs` + `legacy_lower.rs`

### Catch-all arms added in 6 canonical-side consumers

Files: `optimizer/cost/mod.rs`, `physical/colored_dag/{allocator,emitter}.rs`,
`sketch_algebra/lower.rs`, `warm_tier_analysis.rs`. These previously
matched canonical `QueryExpr` exhaustively over the 5 pre-existing
variants; now they need conservative handling for the 10 new ones
(Edge stage / Logical wrap / child-walk / zero-cost) with TODO
markers for the follow-up reshape batches.

### Serde tag note

Initially gave `ColumnRef`/`LiteralValue`/`PartitionKeys`/`Predicate`
internally-tagged `#[serde(tag = "kind", rename_all = "snake_case")]`
attrs, but serde rejects internally-tagged newtype variants containing
primitives (e.g. `Named(String)`). Switched to externally-tagged
`#[serde(rename_all = "snake_case")]`. Caught by the round-trip test.

## Build + test

- `cargo build --release -p controller` — clean
- `cargo build --release -p query_engine_rust` — clean
- `cargo test -p controller --lib` — **666 passed** (was 655 after
  Batch 1; +11 from new typed-Predicate + schema tests)
- `cargo test -p query_engine_rust --lib -- engines::warm_tier` — 13/13

## Diff: 8 files, +794 / -8

Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the refactor/controller-layered-cleanup branch July 17, 2026 20:06
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