Skip to content

refactor(legacy_expr): step γ4 — TopK bridge (heavy-hitter vs Sort+Limit split) - #141

Merged
zzylol merged 1 commit into
mainfrom
refactor/legacy-expr-step-gamma4-topk
May 12, 2026
Merged

zzylol merged 1 commit into
mainfrom
refactor/legacy-expr-step-gamma4-topk

Conversation

@zzylol

@zzylol zzylol commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Per design.md §3 + §6 "What was removed" row 1: TopK collapses two distinct concepts — heavy-hitter intent (sketch primitive) + generic Sort + Limit. The bridge splits these explicitly.

Follows γ1's strategy (c): keep legacy QueryExpr::TopK variant in place; provide a canonical-view helper consumers call on demand. No construction-site rewrites.

New: topk_bridge.rs (333 lines, 7 tests)

pub enum BridgedTopK {
    HeavyHitter { k: usize, by: Vec<ColumnId>, intent: AggIntent },
    SortLimit   { k: usize, by: Vec<ColumnId> },
}
pub fn bridge_topk(k: usize, by: &[ColumnRef], schema: &Schema)
    -> Result<BridgedTopK, BridgeError>;

HeavyHitter.intent carries AggIntent::TopK { k, accuracy: Epsilon(0.05) } — L4 BindCmsTopK rules read it directly.

Classification: defaults to HeavyHitter

Only L1 form lowering to legacy TopK in this tree is PromQL topk(…) (both heavy-hitter intent). SQL ORDER BY name LIMIT 10 already goes through legacy::Sort + Limit. γ7 refines via context flag if a SQL distinction becomes needed.

Demo wire

physical/allocator.rs::alloc_node TopK arm — wraps Vec<String>Vec<ColumnRef::Named>, calls bridge, folds canonical intent kind + resolved col count into NodeAnnotation.rationale.

Build + test

  • cargo build --release -p controller clean
  • cargo test -p controller --lib695 passed (was 688; +7 new tests)

Diff: 3 files, +374 / -1

🤖 Generated with Claude Code

…mit split)

Per design.md §3 + §6 "What was removed" row 1: TopK collapses two
distinct concepts — heavy-hitter intent (sketch primitive) + generic
`Sort + Limit`. The bridge splits these explicitly.

Follows γ1's strategy (c): keep legacy `QueryExpr::TopK` variant in
place; provide a canonical-view helper consumers call on demand. No
construction-site rewrites.

## New: `controller/src/intent_algebra/topk_bridge.rs` (333 lines, 7 tests)

```rust
pub enum BridgedTopK {
    HeavyHitter { k: usize, by: Vec<ColumnId>, intent: AggIntent },
    SortLimit   { k: usize, by: Vec<ColumnId> },
}
pub fn bridge_topk(k: usize, by: &[ColumnRef], schema: &Schema)
    -> Result<BridgedTopK, BridgeError>;
```

`HeavyHitter.intent` carries an embedded `AggIntent::TopK { k, accuracy: Epsilon(0.05) }` so L4 BindCmsTopK rules can read it directly.

`ColumnRef` is the canonical `query_expr::ColumnRef` (matches `AggIntent::TopK { by: Vec<ColumnRef> }` per design.md §6 line 584).

## Classification: defaults to HeavyHitter

PromQL `topk(…)` is the only L1 form lowering to legacy `TopK` in this
tree (`promql.rs:231`, `optimizer/engine.rs:485` — both heavy-hitter
intent). Pure SQL `ORDER BY name LIMIT 10` goes through `legacy::Sort + Limit`
directly. γ7 can refine via context flag if a SQL TopK→Sort+Limit
distinction becomes needed.

## Demo wire in `physical/allocator.rs::alloc_node`'s TopK arm

Wraps legacy `by: Vec<String>` into `Vec<ColumnRef::Named>`, calls
`bridge_topk`, folds canonical intent kind + resolved col count into
`NodeAnnotation.rationale`. Legacy emit shape unchanged.

## Tests added (7, all passing)

- `bridge_global_topk_resolves_empty_by` — empty `by` (PromQL `topk(5, m)`)
- `bridge_resolves_single_by_key` — `host` → ColumnId 2
- `bridge_resolves_multi_key_by` — `host, dc` → `[2, 3]`
- `bridge_unresolvable_column_surfaces_resolve_error` — `NotFound`
- `bridge_sample_value_resolves_to_value_column` — `SampleValue` → 1
- `bridge_wildcard_is_not_positional` — `Wildcard` → `WildcardNotPositional`
- `bridged_intent_carries_topk_with_default_accuracy` — k=42, Epsilon(0.05)

## Build + test

- `cargo build --release -p controller` — clean
- `cargo test -p controller --lib` — **695 passed** (was 688 after γ1; +7 new)

## Diff: 3 files, +374 / -1

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit d47008c into main May 12, 2026
@zzylol
zzylol deleted the refactor/legacy-expr-step-gamma4-topk 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