diff --git a/Cargo.lock b/Cargo.lock index fffaac5b..fb7de5e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -343,7 +343,7 @@ dependencies = [ [[package]] name = "asap-ir" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPController?rev=7fcaf914d87e71407c3a6d7ccac613b867f9c11b#7fcaf914d87e71407c3a6d7ccac613b867f9c11b" +source = "git+https://github.com/ProjectASAP/ASAPController?rev=01745cceac857be21fd1d80584c045e6f932ebfc#01745cceac857be21fd1d80584c045e6f932ebfc" dependencies = [ "serde", "serde_json", @@ -353,7 +353,7 @@ dependencies = [ [[package]] name = "asap-l2" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPController?rev=7fcaf914d87e71407c3a6d7ccac613b867f9c11b#7fcaf914d87e71407c3a6d7ccac613b867f9c11b" +source = "git+https://github.com/ProjectASAP/ASAPController?rev=01745cceac857be21fd1d80584c045e6f932ebfc#01745cceac857be21fd1d80584c045e6f932ebfc" dependencies = [ "asap-ir", "thiserror 2.0.18", @@ -362,7 +362,7 @@ dependencies = [ [[package]] name = "asap-plan" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPController?rev=7fcaf914d87e71407c3a6d7ccac613b867f9c11b#7fcaf914d87e71407c3a6d7ccac613b867f9c11b" +source = "git+https://github.com/ProjectASAP/ASAPController?rev=01745cceac857be21fd1d80584c045e6f932ebfc#01745cceac857be21fd1d80584c045e6f932ebfc" dependencies = [ "asap-ir", "asap-sketch", @@ -382,7 +382,7 @@ dependencies = [ [[package]] name = "asap-sketch" version = "0.1.0" -source = "git+https://github.com/ProjectASAP/ASAPController?rev=7fcaf914d87e71407c3a6d7ccac613b867f9c11b#7fcaf914d87e71407c3a6d7ccac613b867f9c11b" +source = "git+https://github.com/ProjectASAP/ASAPController?rev=01745cceac857be21fd1d80584c045e6f932ebfc#01745cceac857be21fd1d80584c045e6f932ebfc" dependencies = [ "asap-ir", ] diff --git a/control_plane/Cargo.toml b/control_plane/Cargo.toml index 9df525d3..f5d846b4 100644 --- a/control_plane/Cargo.toml +++ b/control_plane/Cargo.toml @@ -61,10 +61,10 @@ asap_types.workspace = true # this doesn't pull in datafusion or any front-end weight. asap-sketch is # asap-plan's own dependency (SummaryKind/SummaryParams), needed here only # to translate Implementation into this repo's own Capability vocabulary. -asap-ir = { git = "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/ProjectASAP/ASAPController", rev = "7fcaf914d87e71407c3a6d7ccac613b867f9c11b" } -asap-l2 = { git = "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/ProjectASAP/ASAPController", rev = "7fcaf914d87e71407c3a6d7ccac613b867f9c11b" } -asap-sketch = { git = "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/ProjectASAP/ASAPController", rev = "7fcaf914d87e71407c3a6d7ccac613b867f9c11b" } -asap-plan = { git = "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/ProjectASAP/ASAPController", rev = "7fcaf914d87e71407c3a6d7ccac613b867f9c11b" } +asap-ir = { git = "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/ProjectASAP/ASAPController", rev = "01745cceac857be21fd1d80584c045e6f932ebfc" } +asap-l2 = { git = "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/ProjectASAP/ASAPController", rev = "01745cceac857be21fd1d80584c045e6f932ebfc" } +asap-sketch = { git = "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/ProjectASAP/ASAPController", rev = "01745cceac857be21fd1d80584c045e6f932ebfc" } +asap-plan = { git = "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/ProjectASAP/ASAPController", rev = "01745cceac857be21fd1d80584c045e6f932ebfc" } [dev-dependencies] tokio = { version = "1", features = ["full", "test-util"] } diff --git a/control_plane/src/emit/backend_push.rs b/control_plane/src/emit/backend_push.rs index 7c066b20..7fcba9e1 100644 --- a/control_plane/src/emit/backend_push.rs +++ b/control_plane/src/emit/backend_push.rs @@ -658,7 +658,7 @@ mod tests { use crate::physical::colored_dag::emitter::{ AggregationInput, BackendAggregation, BackendReadout, }; - use crate::sketch_algebra::physical_expr::EstimateOp; + use asap_sketch::SketchQuery; use asap_sketch::{SummaryKind, SummaryParams}; BackendStageConfig { aggregations: vec![BackendAggregation { @@ -675,7 +675,7 @@ mod tests { }], readouts: vec![BackendReadout { aggregation_id: agg_id.to_string(), - op: EstimateOp::Quantile { q: 0.99 }, + op: SketchQuery::Quantile { q: 0.99 }, }], } } diff --git a/control_plane/src/emit/mod.rs b/control_plane/src/emit/mod.rs index f559f0a1..5967661a 100644 --- a/control_plane/src/emit/mod.rs +++ b/control_plane/src/emit/mod.rs @@ -51,10 +51,12 @@ pub use trait_def::{ pub use crate::workload::WorkloadRegistry; use crate::physical::colored_dag::emitter::EdgeStageConfig; +use crate::sketch_algebra::physical_expr::L4Plan; use crate::sketch_algebra::PhysicalExpr; use crate::store::WorkloadStore; use anyhow::Result; -use asap_sketch::SummaryKind; +use asap_sketch::{L4Node, SummaryExpr, SummaryKind}; +use std::rc::Rc; /// Phase ε.1.5 — which edge runtime an agent identifies as. /// @@ -292,24 +294,55 @@ fn apply_cold_format_from_env(edge_cfg: &mut EdgeStageConfig) { /// which is correct. pub fn extract_root_sketch_kind(expr: &PhysicalExpr) -> Option { match expr { - PhysicalExpr::SketchAgg { sketch_type, .. } => Some(sketch_type.clone()), + PhysicalExpr::Committed(plan) => extract_from_plan(plan), PhysicalExpr::RawAtEdgeSketchAtBackend { family, .. } => Some(family.clone()), - PhysicalExpr::SketchEstimate { child, .. } => extract_root_sketch_kind(child), - PhysicalExpr::SketchMerge { children, .. } => { - children.iter().find_map(extract_root_sketch_kind) + PhysicalExpr::RawAtEdgePrometheusArchive { .. } => None, + } +} + +fn extract_from_plan(plan: &L4Plan) -> Option { + match plan { + L4Plan::Summary(node) => extract_from_node(node), + L4Plan::LetBinding { expr, child, .. } => { + extract_from_plan(expr).or_else(|| extract_from_plan(child)) } - PhysicalExpr::LetBinding { expr, child, .. } => { - extract_root_sketch_kind(expr).or_else(|| extract_root_sketch_kind(child)) + L4Plan::Ref { .. } => None, + } +} + +/// Is `kind` an exact accumulator (Sum/Count/MinMax/Increase/Rate) rather +/// than an approximate sketch? Exact accumulators have no sketch family +/// for the 5-sketch routing connector to route on — same as the old, +/// now-retired `PhysicalExpr::ExactAgg` variant, which this function +/// treated as `None`. +fn is_exact_accumulator(kind: &SummaryKind) -> bool { + matches!( + kind, + SummaryKind::Sum + | SummaryKind::Count + | SummaryKind::MinMax + | SummaryKind::Increase + | SummaryKind::Rate + ) +} + +fn extract_from_node(node: &Rc) -> Option { + match &node.expr { + SummaryExpr::SummaryAgg { sketch, .. } if !is_exact_accumulator(sketch) => { + Some(sketch.clone()) } - PhysicalExpr::Logical(_) - | PhysicalExpr::Ref { .. } - | PhysicalExpr::RawAtEdgePrometheusArchive { .. } - // ExactAgg has no sketch family — it produces an exact - // aggregation accumulator, not a sketch state. The - // routing emitter routes these to the - // `metrics/raw_passthrough` / exact-precompute pipeline - // alongside Logical pass-throughs. - | PhysicalExpr::ExactAgg { .. } => None, + // An exact accumulator has no sketch family beneath it (its own + // child is always a plain `Logical` leaf) — same as the old + // `ExactAgg` case. + SummaryExpr::SummaryAgg { .. } => None, + SummaryExpr::SummaryEstimate { sketch_input, .. } => extract_from_node(sketch_input), + SummaryExpr::SummaryMerge { children } => children.iter().find_map(extract_from_node), + // Not surfaced by any `Bind*` path yet (gated on rules that + // haven't landed — see `physical_expr.rs`'s module docs). + SummaryExpr::SummaryJoin { .. } + | SummaryExpr::SummarySubtract { .. } + | SummaryExpr::SummaryDelete { .. } + | SummaryExpr::Logical(_) => None, } } @@ -990,10 +1023,11 @@ mod runtime_tests { "top_endpoint_qps", Some(BTreeSet::from([SummaryKind::CountSketchWithHeap])), ), - ( - "endpoint_request_freq", - Some(BTreeSet::from([SummaryKind::Cms])), - ), + // `CountMinSketch` override re-derives statistic to + // `Frequency`, `AggIntent::Extension`-shaped — declines to + // bind pending ASAPController#150 (see + // `optimizer::rules::tests::typed_binding_endpoint_request_freq_declines_pending_upstream_extension_support`). + ("endpoint_request_freq", None), ]; for (metric, want) in &expected { let got = map.get(*metric).cloned(); @@ -1003,11 +1037,12 @@ mod runtime_tests { full map: {map:?}", ); } - // Routing table covers all 5 sketched metrics. + // Routing table covers the 4 sketched metrics (endpoint_request_freq + // and http_requests_total both decline — see above). assert_eq!( map.len(), - 5, - "routing table should have 5 entries (5 sketches; raw declines), got: {map:?}" + 4, + "routing table should have 4 entries (4 sketches; raw + Extension both decline), got: {map:?}" ); } @@ -1177,11 +1212,20 @@ mod runtime_tests { mk(AggType::Cardinality, Some(SketchType::HLL), Vec::new()), WorkloadCharacteristics::default(), ); - // Frequency → CMS (capability-matched default for Frequency). + // TopK → CountSketch-with-heap. Not plain `Frequency, None` + // (capability-matched CMS default) any more — `Frequency`'s + // capability-matched default is `AggIntent::Extension`-shaped, + // which `asap_plan::boundary::implementation_for` maps to + // `PassThrough` unconditionally (ASAPController#150), so it no + // longer contributes a family to the union at all. Use a + // `CountSketch` override instead — it re-derives the statistic to + // `TopK` (not `Extension`-shaped), so it still binds, and still + // exercises "3 distinct capabilities on one metric → union of 3 + // distinct families". store.set( METRIC, AggRole::Other, - mk(AggType::Frequency, None, Vec::new()), + mk(AggType::Frequency, Some(SketchType::CountSketch), Vec::new()), WorkloadCharacteristics::default(), ); @@ -1192,7 +1236,11 @@ mod runtime_tests { .unwrap_or_else(|| panic!("http_requests must be in the map\nmap: {map:?}")); assert_eq!( got, - BTreeSet::from([SummaryKind::DDSketch, SummaryKind::Hll, SummaryKind::Cms]), + BTreeSet::from([ + SummaryKind::DDSketch, + SummaryKind::Hll, + SummaryKind::CountSketchWithHeap + ]), "a metric queried by 3 capabilities must accumulate 3 families (UNION, not first-wins)\nmap: {map:?}" ); } diff --git a/control_plane/src/emit/stage_config.rs b/control_plane/src/emit/stage_config.rs index 931ba54c..17838170 100644 --- a/control_plane/src/emit/stage_config.rs +++ b/control_plane/src/emit/stage_config.rs @@ -62,9 +62,9 @@ use crate::physical::colored_dag::emitter::{ // archive-tier metric lists). Importing them at module scope produced an // unused-import warning on every non-test build, so they're scoped into the // test module's `use super::*` instead (P2-5). +use crate::intent_algebra::ColumnRef; use crate::physical::colored_dag::stage_id::StageId; -use crate::sketch_algebra::physical_expr::EstimateOp; -use asap_sketch::{SummaryKind, SummaryParams}; +use asap_sketch::{SketchQuery, SummaryKind, SummaryParams}; // ── YAML structural types ───────────────────────────────────────────────────── // @@ -3031,24 +3031,40 @@ fn build_backend_aggregation_json(agg: &BackendAggregation) -> JsonValue { /// `aggregations` list by the same `PolicyFingerprint` recipe. fn build_backend_readout_json(r: &BackendReadout) -> JsonValue { match &r.op { - EstimateOp::Quantile { q } => json!({ + SketchQuery::Quantile { q } => json!({ "op": "quantile", "q": q, }), - EstimateOp::Cardinality => json!({ + SketchQuery::Cardinality => json!({ "op": "cardinality", }), - EstimateOp::PointCount { key } => json!({ + SketchQuery::PointCount { key } => json!({ "op": "point_count", - "key": key, + "key": column_ref_to_wire_key(key), }), - EstimateOp::TopK { k } => json!({ + SketchQuery::TopK { k } => json!({ "op": "topk", "k": k, }), } } +/// The wire-string key for a `SketchQuery::PointCount` readout. +/// +/// `SampleValue` and `Wildcard` both wire to the legacy `"*"` sentinel +/// (`sketch_algebra::rules::bind_cms_count`, retired by Step B, used the +/// literal string `"*"` to mean "all rows / no specific key"; the L5 +/// emitter's per-group resolution already special-cases that string) — +/// there's no real queryable column for a plain `Count`/`Frequency` +/// readout in either case, so both collapse to the same sentinel. +fn column_ref_to_wire_key(col: &ColumnRef) -> String { + match col { + ColumnRef::Named(name) => name.clone(), + ColumnRef::Qualified { table, name } => format!("{table}.{name}"), + ColumnRef::SampleValue | ColumnRef::Wildcard => "*".to_string(), + } +} + /// Collapse a heap-bearing `SummaryKind` to its bare counterpart. /// Identity for every other kind. /// @@ -3518,11 +3534,11 @@ mod tests { readouts: vec![ BackendReadout { aggregation_id: "agg0".into(), - op: EstimateOp::Quantile { q: 0.99 }, + op: SketchQuery::Quantile { q: 0.99 }, }, BackendReadout { aggregation_id: "agg1".into(), - op: EstimateOp::Cardinality, + op: SketchQuery::Cardinality, }, ], }; @@ -3591,12 +3607,12 @@ mod tests { readouts: vec![ BackendReadout { aggregation_id: "agg0".into(), - op: EstimateOp::TopK { k: 10 }, + op: SketchQuery::TopK { k: 10 }, }, BackendReadout { aggregation_id: "agg1".into(), - op: EstimateOp::PointCount { - key: "user_42".into(), + op: SketchQuery::PointCount { + key: ColumnRef::Named("user_42".into()), }, }, ], @@ -3679,11 +3695,11 @@ mod tests { readouts: vec![BackendReadout { aggregation_id: "agg0".into(), op: match kind { - SummaryKind::DDSketch | SummaryKind::Kll => EstimateOp::Quantile { q: 0.99 }, - SummaryKind::Hll => EstimateOp::Cardinality, - SummaryKind::CountSketch => EstimateOp::TopK { k: 10 }, - SummaryKind::Cms => EstimateOp::PointCount { - key: "user_42".into(), + SummaryKind::DDSketch | SummaryKind::Kll => SketchQuery::Quantile { q: 0.99 }, + SummaryKind::Hll => SketchQuery::Cardinality, + SummaryKind::CountSketch => SketchQuery::TopK { k: 10 }, + SummaryKind::Cms => SketchQuery::PointCount { + key: ColumnRef::Named("user_42".into()), }, other => unreachable!( "backend_cfg_with_kind: unsupported test fixture kind {other:?}" @@ -4134,7 +4150,7 @@ mod tests { }], readouts: vec![BackendReadout { aggregation_id: "phase_b_agg0".into(), - op: EstimateOp::Quantile { q: 0.99 }, + op: SketchQuery::Quantile { q: 0.99 }, }], }; let v = emit_backend_streaming_config_json(&cfg, &[]).expect("emit ok"); @@ -5901,7 +5917,7 @@ mod tests { use crate::physical::colored_dag::emitter::{ AggregationInput, BackendAggregation, BackendReadout, BackendStageConfig, }; - use crate::sketch_algebra::physical_expr::EstimateOp; + use asap_sketch::SketchQuery; let cfg = BackendStageConfig { aggregations: vec![BackendAggregation { @@ -5918,7 +5934,7 @@ mod tests { }], readouts: vec![BackendReadout { aggregation_id: "agg0".to_string(), - op: EstimateOp::Quantile { q: 0.99 }, + op: SketchQuery::Quantile { q: 0.99 }, }], }; let v = emit_backend_streaming_config_json(&cfg, &[]).expect("emit ok"); diff --git a/control_plane/src/emit/trait_def.rs b/control_plane/src/emit/trait_def.rs index e2591e73..075ff52d 100644 --- a/control_plane/src/emit/trait_def.rs +++ b/control_plane/src/emit/trait_def.rs @@ -200,7 +200,7 @@ mod tests { BackendAggregation, BackendReadout, ExportTarget, GatewayMergeProcessor, }; use crate::physical::colored_dag::stage_id::StageId; - use crate::sketch_algebra::physical_expr::EstimateOp; + use asap_sketch::SketchQuery; use asap_sketch::{SummaryKind, SummaryParams}; use std::collections::HashMap; @@ -256,7 +256,7 @@ mod tests { }], readouts: vec![BackendReadout { aggregation_id: "agg0".to_string(), - op: EstimateOp::Quantile { q: 0.99 }, + op: SketchQuery::Quantile { q: 0.99 }, }], } } diff --git a/control_plane/src/main.rs b/control_plane/src/main.rs index 4bdadd52..1f34b256 100644 --- a/control_plane/src/main.rs +++ b/control_plane/src/main.rs @@ -595,32 +595,58 @@ async fn handle_plan(State(st): State, Json(spec): Json) -> // is no `query_string`. let raw_bps = plan.transmission_cost_summary.raw_bytes_per_sec; let budgets = StageResourceBudgets::from_workload_chars(&wc); - let mut bound_physical: Option = None; - let mut plan_summary = None; - if let Some(ref qs) = query_string { - match parse_query_expr_canonical(qs) { - Err(e) => { - warn!(query = %qs, error = %e, "parse_query_expr_canonical failed; skipping algebra pipeline") - } - Ok(qe) => { - let constraints = optimizer::engine::DeploymentConstraints::from_budgets(&budgets); - let (opt_qe, _) = - QueryOptimizer::with_constraints(raw_bps, constraints).optimize(qe); - // L4 sketch binding: lower the optimised L3 tree to the - // sketch-bound `PhysicalExpr` IR — the typed L5's input. - let accuracy = if workload.accuracy_sla >= 1.0 { - control_plane::types_v2::AccuracyTarget::Exact - } else { - control_plane::types_v2::AccuracyTarget::Epsilon(1.0 - workload.accuracy_sla) - }; - bound_physical = - control_plane::sketch_algebra::bind_query_expr(&opt_qe, accuracy).ok(); - // Cost summary for the JSON response. - let plan_node = SketchAllocator::new(budgets.clone(), raw_bps).allocate(opt_qe); - plan_summary = Some(plan_node.summarise(raw_bps)); + // Everything that touches `sketch_algebra::PhysicalExpr` (which + // carries `Rc` since Step B of the + // plan-shaped-serving migration adopted ASAPController's own + // `Rc`-based DAG sharing) is scoped to this block and resolved down + // to Send-safe outputs (`Option`, + // `Option>`) *before* any `.await` + // below — an `Rc` alive in this `async fn`'s generator state at a + // yield point would make its `Future` `!Send`, breaking + // `axum::Handler`. + let (plan_summary, stage_configs) = { + let mut bound_physical: Option = None; + let mut plan_summary = None; + if let Some(ref qs) = query_string { + match parse_query_expr_canonical(qs) { + Err(e) => { + warn!(query = %qs, error = %e, "parse_query_expr_canonical failed; skipping algebra pipeline") + } + Ok(qe) => { + let constraints = optimizer::engine::DeploymentConstraints::from_budgets(&budgets); + let (opt_qe, _) = + QueryOptimizer::with_constraints(raw_bps, constraints).optimize(qe); + // L4 sketch binding: lower the optimised L3 tree to the + // sketch-bound `PhysicalExpr` IR — the typed L5's input. + let accuracy = if workload.accuracy_sla >= 1.0 { + control_plane::types_v2::AccuracyTarget::Exact + } else { + control_plane::types_v2::AccuracyTarget::Epsilon(1.0 - workload.accuracy_sla) + }; + bound_physical = + control_plane::sketch_algebra::bind_query_expr(&opt_qe, accuracy).ok(); + // Cost summary for the JSON response. + let plan_node = SketchAllocator::new(budgets.clone(), raw_bps).allocate(opt_qe); + plan_summary = Some(plan_node.summarise(raw_bps)); + } } } - } + + let stage_configs: Option< + std::collections::HashMap< + crate::physical::colored_dag::StageId, + crate::physical::colored_dag::StageConfig, + >, + > = if physical::stage_split::typed_stage_split_enabled() { + let physical_expr = + bound_physical.or_else(|| optimizer::rules::bind_workload_typed(&workload)); + physical_expr.and_then(|pe| physical::stage_split::split_typed_three_stage(&pe)) + } else { + None + }; + + (plan_summary, stage_configs) + }; plan.precompute = build_precompute_engine_jobs(&workload, "data-plane:4317"); // B2 (metric, role): derive the role from the request's @@ -679,11 +705,7 @@ async fn handle_plan(State(st): State, Json(spec): Json) -> // parse), there is no L3 tree to bind, so we fall back to // `bind_workload_typed`, which lowers the flat `QueryWorkload` // summary to a `PhysicalExpr` directly. - if physical::stage_split::typed_stage_split_enabled() { - let physical_expr = - bound_physical.or_else(|| optimizer::rules::bind_workload_typed(&workload)); - if let Some(physical_expr) = physical_expr { - if let Some(configs) = physical::stage_split::split_typed_three_stage(&physical_expr) { + if let Some(configs) = stage_configs { for (stage_id, stage_cfg) in configs { match stage_cfg { crate::physical::colored_dag::StageConfig::Edge(mut edge) => { @@ -818,14 +840,12 @@ async fn handle_plan(State(st): State, Json(spec): Json) -> } } } - } else { - warn!( - metric = %workload.metric_name, - "[USE_TYPED_STAGE_SPLIT] split_typed_three_stage returned None; \ - legacy plan output unaffected" - ); - } - } + } else if physical::stage_split::typed_stage_split_enabled() { + warn!( + metric = %workload.metric_name, + "[USE_TYPED_STAGE_SPLIT] split_typed_three_stage returned None; \ + legacy plan output unaffected" + ); } // ── Update scrape-endpoint sketch types and agent→(metric, role) mapping ── diff --git a/control_plane/src/optimizer/rules/mod.rs b/control_plane/src/optimizer/rules/mod.rs index b2cdbea6..7bb2db28 100644 --- a/control_plane/src/optimizer/rules/mod.rs +++ b/control_plane/src/optimizer/rules/mod.rs @@ -68,11 +68,7 @@ pub fn bind_workload_typed(w: &QueryWorkload) -> Option Option BindDDSketchOnQuantile.apply(&aggregate, &accuracy), - (SummaryKind::Kll, _) => BindKllOnQuantile.apply(&aggregate, &accuracy), - (SummaryKind::Hll, _) => BindHllOnCardinality.apply(&aggregate, &accuracy), - // The capability matrix already pinned the family here, so force - // the matching recall tier rather than re-inferring it: a - // CountSketch pick is the unbiased canonical top-k (Tight); a CMS - // pick on a top-k is the cheap CMS-with-heap (Loose). This keeps - // `bind_workload_typed`'s contract-row mapping deterministic — the - // recall-aware default lives in `dispatch()` / `Rule::apply`. - (SummaryKind::CountSketch, _) => BindCountSketchOnTopK.apply_with_tier( - &aggregate, - &accuracy, - crate::sketch_algebra::rules::bind_cms_topk::TopkRecallTier::Tight, - ), - (SummaryKind::Cms, StatisticClass::TopK) => BindCountSketchOnTopK.apply_with_tier( - &aggregate, - &accuracy, - crate::sketch_algebra::rules::bind_cms_topk::TopkRecallTier::Loose, - ), - (SummaryKind::Cms, _) => BindCmsOnCount.apply(&aggregate, &accuracy), - // `kind` always comes from `pick_family`/`sketch_type_override`, - // exhaustively the 5 canonical families. - (other, _) => unreachable!("dispatch: unsupported SummaryKind {other:?}"), + // + // `StatisticClass::Frequency` (the `endpoint_request_freq` contract + // row) is `AggIntent::Extension`-shaped — `asap_plan::boundary::implementation_for` + // maps every `Extension` to `PassThrough` unconditionally (a core-vs- + // deployment-specific-shape gap, filed as ASAPController#150), so this + // now returns `None` for that one contract row until the upstream gap + // closes; the caller already falls back to the legacy `plan()` output + // in that case. + let forced = match kind { + SummaryKind::CountSketch => SummaryKind::CountSketchWithHeap, + SummaryKind::Cms if statistic == StatisticClass::TopK => SummaryKind::CmsWithHeap, + other => other, + }; + let cost_model = ForcedFamilyCostModel::new(accuracy.clone(), forced); + let node = asap_plan::bind::implement_tree_in_with( + &aggregate, + &crate::intent_algebra::BindingScope::default(), + &cost_model, + ) + .ok()?; + // `implement_tree_in_with` never *errors* on "nothing bound" — an + // intent `boundary::implementation_for` can't realize (e.g. the + // `Extension`/Frequency PassThrough case above) still returns + // `Ok(Rc)`, just wrapping the input as `SummaryExpr::Logical` + // unchanged. `bind_workload_typed`'s own contract is `None` for + // "typed path doesn't support this shape yet" — translate the two + // by checking whether anything actually got committed. + if matches!(node.expr, asap_sketch::SummaryExpr::Logical(_)) { + return None; } + Some(crate::sketch_algebra::physical_expr::PhysicalExpr::committed(node)) } pub struct RulesPlanner { @@ -480,7 +482,6 @@ mod tests { (SketchType::DDSketch, SummaryKind::DDSketch), (SketchType::KLL, SummaryKind::Kll), (SketchType::HLL, SummaryKind::Hll), - (SketchType::CountMinSketch, SummaryKind::Cms), // CountSketch override re-derives statistic as TopK (see the // override arm below), and `bind_cms_topk` always binds the // heap-bearing kind for a top-k intent — matches this @@ -501,6 +502,22 @@ mod tests { "override {ov:?} should pin family {expect:?}, not fall back to DDSketch", ); } + + // `CountMinSketch` re-derives statistic to `Frequency`, which is + // `AggIntent::Extension`-shaped (this deployment's point-frequency + // query) — `asap_plan::boundary::implementation_for` maps every + // `Extension` to `PassThrough` unconditionally, regardless of + // which `CostModel` is plugged in (ASAPController#150), so this + // override can no longer bind at all pending the upstream gap + // closing. Same accepted regression as the plain + // `endpoint_request_freq` contract row (see + // `typed_binding_endpoint_request_freq_declines_pending_upstream_extension_support`). + let mut w = workload(vec![AggType::Quantile]); + w.sketch_type_override = Some(SketchType::CountMinSketch); + assert!( + bind_workload_typed(&w).is_none(), + "CountMinSketch override should decline pending ASAPController#150", + ); } #[test] @@ -625,24 +642,12 @@ mod tests { use crate::sketch_algebra::physical_expr::PhysicalExpr; use asap_sketch::SummaryKind; - /// Walk the L4 binding output and pull out the `SketchAgg`'s family. - /// Returns `None` if no `SketchAgg` node is present (raw / pure - /// logical pass-through). + /// Walk the L4 binding output and pull out the approximate sketch + /// family. Returns `None` if no sketch node is present (raw / pure + /// logical pass-through, or an exact accumulator — see + /// `emit::extract_root_sketch_kind`, whose logic this mirrors). fn extract_family(expr: &PhysicalExpr) -> Option { - match expr { - PhysicalExpr::SketchAgg { sketch_type, .. } => Some(sketch_type.clone()), - PhysicalExpr::SketchEstimate { child, .. } => extract_family(child), - PhysicalExpr::SketchMerge { children, .. } => children.iter().find_map(extract_family), - PhysicalExpr::LetBinding { expr, child, .. } => { - extract_family(expr).or_else(|| extract_family(child)) - } - PhysicalExpr::Logical(_) | PhysicalExpr::Ref { .. } => None, - PhysicalExpr::RawAtEdgeSketchAtBackend { family, .. } => Some(family.clone()), - PhysicalExpr::RawAtEdgePrometheusArchive { .. } => None, - // ExactAgg has no sketch family — it produces an exact - // aggregation accumulator, not a sketch state. - PhysicalExpr::ExactAgg { .. } => None, - } + crate::emit::extract_root_sketch_kind(expr) } /// Build a workload with the given metric name + reasonable @@ -735,18 +740,22 @@ mod tests { } #[test] - fn typed_binding_endpoint_request_freq_picks_cms() { - // Contract: `endpoint_request_freq` → CMS (Frequency). - // The legacy AggType default for Frequency is *also* CMS via - // BindCmsOnCount, but the metric-name path goes through the - // capability-matching picker first — both produce CMS, the - // contract row ratifies it. + fn typed_binding_endpoint_request_freq_declines_pending_upstream_extension_support() { + // Contract used to be: `endpoint_request_freq` → CMS (Frequency). + // `Frequency` is `AggIntent::Extension`-shaped; + // `asap_plan::boundary::implementation_for` maps every `Extension` + // to `PassThrough` unconditionally (core has no realization + // opinion for a deployment-specific shape it doesn't know — see + // ASAPController#150). Step B of the plan-shaped-serving + // migration adopted `implement_tree_in_with` here without a local + // Frequency-binding workaround, so this contract row now declines + // the typed path (falls back to the legacy `plan()` output) until + // #150 lands an extension point. let w = workload_for("endpoint_request_freq", AggType::Frequency); - let bound = bind_workload_typed(&w).expect("endpoint_request_freq must bind"); - assert_eq!( - extract_family(&bound), - Some(SummaryKind::Cms), - "endpoint_request_freq should bind to CMS (Frequency)", + let bound = bind_workload_typed(&w); + assert!( + bound.is_none(), + "endpoint_request_freq should decline pending ASAPController#150; got {bound:?}", ); } @@ -858,11 +867,9 @@ mod tests { AggType::Frequency, Some(SummaryKind::CountSketchWithHeap), ), - ( - "endpoint_request_freq", - AggType::Frequency, - Some(SummaryKind::Cms), - ), + // `Extension`/Frequency PassThrough gap — ASAPController#150, + // see `typed_binding_endpoint_request_freq_declines_pending_upstream_extension_support`. + ("endpoint_request_freq", AggType::Frequency, None), ]; for (metric, agg, expected) in cases { let w = workload_for(metric, agg); diff --git a/control_plane/src/optimizer/trait_def.rs b/control_plane/src/optimizer/trait_def.rs index 14438076..de6162c4 100644 --- a/control_plane/src/optimizer/trait_def.rs +++ b/control_plane/src/optimizer/trait_def.rs @@ -110,30 +110,37 @@ mod tests { assert!(cats.contains(&RuleCategory::Cse)); } - /// Every Phase-C bind rule in `sketch_algebra::rules` surfaces + /// Any `sketch_algebra::rules::Rule` implementor surfaces /// `RuleCategory::Bind` through its `OptimizerRule` blanket impl. + /// + /// Step B of the plan-shaped-serving migration retired the seven + /// concrete `Bind*` rule structs this test used to exercise (KLL / + /// DDSketch / HLL / CMS-count / CMS-topk / exact-agg / archive-only) + /// in favor of `asap_plan::bind::implement_tree_in_with` + + /// `sketch_algebra::cost_model::ControlPlaneCostModel` — see + /// `sketch_algebra::lower`. The `Rule` trait and its blanket impl stay + /// (a real, if currently unused, extension point — see + /// `sketch_algebra::rules`'s module docs), so this test now pins the + /// blanket impl itself via a minimal local implementor rather than + /// the retired structs. #[test] - fn sketch_algebra_bind_rules_carry_bind_category() { - use crate::sketch_algebra::rules::{ - bind_archive_only::BindArchiveOnly, bind_cms_count::BindCmsOnCount, - bind_cms_topk::BindCountSketchOnTopK, bind_ddsketch_quantile::BindDDSketchOnQuantile, - bind_hll_cardinality::BindHllOnCardinality, bind_kll_quantile::BindKllOnQuantile, - }; - let rules: Vec> = vec![ - Box::new(BindKllOnQuantile), - Box::new(BindDDSketchOnQuantile), - Box::new(BindCmsOnCount), - Box::new(BindCountSketchOnTopK), - Box::new(BindHllOnCardinality), - Box::new(BindArchiveOnly), - ]; - for r in &rules { - assert_eq!( - r.category(), - RuleCategory::Bind, - "rule {} should carry Bind category", - r.name() - ); + fn rule_blanket_impl_carries_bind_category() { + use crate::intent_algebra::QueryExpr; + use crate::sketch_algebra::physical_expr::PhysicalExpr; + use crate::sketch_algebra::rules::Rule; + use crate::types_v2::AccuracyTarget; + + struct NoOpRule; + impl Rule for NoOpRule { + fn name(&self) -> &'static str { + "no_op_rule" + } + fn apply(&self, _expr: &QueryExpr, _accuracy: &AccuracyTarget) -> Option { + None + } } + + let rule: Box = Box::new(NoOpRule); + assert_eq!(rule.category(), RuleCategory::Bind); } } diff --git a/control_plane/src/physical/colored_dag/allocator.rs b/control_plane/src/physical/colored_dag/allocator.rs index abc06f6d..7dcd41dc 100644 --- a/control_plane/src/physical/colored_dag/allocator.rs +++ b/control_plane/src/physical/colored_dag/allocator.rs @@ -37,9 +37,13 @@ #![allow(dead_code)] use std::collections::HashMap; +use std::rc::Rc; + +use asap_sketch::{L4Node, SummaryExpr}; use crate::physical::colored_dag::dag::{ColoredDag, ColoredNode, NodeId}; use crate::physical::colored_dag::stage_id::{StageId, Topology}; +use crate::sketch_algebra::physical_expr::L4Plan; use crate::sketch_algebra::PhysicalExpr; use crate::types_v2::BindingName; @@ -96,40 +100,101 @@ impl ThreeStageWalker { /// Recursively visit `expr`, append its colored node to the DAG, /// and return its `(NodeId, StageId)`. fn visit(&mut self, expr: &PhysicalExpr) -> Result<(NodeId, StageId), AllocateError> { - // Reserve a slot for this node up-front so child IDs are - // strictly larger than the parent's; downstream `cut_edges` - // analysis assumes parents come before children in `nodes`. - let id = NodeId(self.dag.nodes.len()); - self.dag.nodes.push(ColoredNode { - id, - expr: expr.clone(), - // Placeholder — overwritten below once children are coloured. - stage: StageId::Edge, - }); + match expr { + PhysicalExpr::Committed(plan) => self.visit_plan(plan), + + // ── Phase ε.1 Mode 2: raw at edge, sketch built at backend. + // Edge ships raw OTLP — we stage as Edge so the L5 emitter's + // edge-side YAML pipeline picks it up; the sketch construction + // itself happens at the backend (no edge sketch processor). + PhysicalExpr::RawAtEdgeSketchAtBackend { child, .. } => { + let id = self.reserve_node(expr.clone()); + let (cid, _) = self.visit_plan(child)?; + self.dag.edges.push((id, cid)); + self.finish_node(id, StageId::Edge) + } - let stage = match expr { + // ── Phase ε.1 Mode 3: raw at edge, ships directly to + // Prometheus's native OTLP receiver. The agent pipeline picks + // this up via `asap.mode=prometheus_archive` routing. + PhysicalExpr::RawAtEdgePrometheusArchive { .. } => { + let id = self.reserve_node(expr.clone()); + self.finish_node(id, StageId::Edge) + } + } + } + + /// Recursively visit an [`L4Plan`] — the "what to compute" layer. + /// [`L4Plan::Summary`] delegates the actual per-node granularity to + /// [`Self::visit_l4node`] (walking `asap_sketch::L4Node`'s own DAG + /// shape); [`L4Plan::LetBinding`] / [`L4Plan::Ref`] are this crate's + /// own named-binding sharing mechanism, unchanged from before Step B. + fn visit_plan(&mut self, plan: &L4Plan) -> Result<(NodeId, StageId), AllocateError> { + match plan { + L4Plan::Summary(node) => self.visit_l4node(node), + + // ── LetBinding: colour by the bound expression's stage, + // and bring the binding into scope before walking the body. + L4Plan::LetBinding { name, expr, child } => { + let id = self.reserve_node(PhysicalExpr::Committed(plan.clone())); + let (eid, expr_stage) = self.visit_plan(expr)?; + self.dag.edges.push((id, eid)); + self.scope.insert(name.as_str().to_string(), expr_stage); + let (bid, _) = self.visit_plan(child)?; + self.dag.edges.push((id, bid)); + self.finish_node(id, expr_stage) + } + + // ── Ref: colour matches the binding's stage. Unresolved + // refs bubble up as `AllocateError::UnresolvedRef`. + L4Plan::Ref { name } => { + let id = self.reserve_node(PhysicalExpr::Committed(plan.clone())); + let stage = self + .scope + .get(name.as_str()) + .copied() + .ok_or_else(|| AllocateError::UnresolvedRef(name.as_str().to_string()))?; + self.finish_node(id, stage) + } + } + } + + /// Recursively visit one `asap_sketch::L4Node` — the sketch algebra + /// itself, owned upstream. Every semantic node gets its own + /// [`ColoredNode`] (matching the granularity the old, locally-defined + /// `PhysicalExpr::{SketchAgg,SketchEstimate,SketchMerge}` had), + /// stored back as `PhysicalExpr::Committed(L4Plan::Summary(..))` + /// wrapping just that sub-node, so downstream consumers + /// (`colored_dag::emitter`, `emit::mod`) keep pattern-matching + /// against the same `PhysicalExpr` shape. + fn visit_l4node(&mut self, node: &Rc) -> Result<(NodeId, StageId), AllocateError> { + let id = self.reserve_node(PhysicalExpr::committed(Rc::clone(node))); + + let stage = match &node.expr { // ── Logical pass-through — colour by inspecting the wrapped // L3 QueryExpr. `Scan` / `Window` always land on edge; // `Aggregate{exact}` lands on edge if its child is an edge // (scrape locality); `Ref` resolves through the lexical // scope map. - PhysicalExpr::Logical(qe) => self.colour_logical(qe)?, + SummaryExpr::Logical(qe) => self.colour_logical(qe)?, - // ── SketchAgg: always edge per design.md §6 batched-queries - // table. The "SketchAgg whose child is a Scan MUST be on - // Edge" invariant is automatically satisfied. - PhysicalExpr::SketchAgg { child, .. } => { - let (cid, _) = self.visit(child)?; + // ── SummaryAgg: always edge per design.md §6 batched-queries + // table — true for both approximate sketches (the old + // `SketchAgg`) and exact accumulators (the old `ExactAgg`); + // `SummaryKind` unifies both into the same node shape, and + // both landed on Edge before this migration too. + SummaryExpr::SummaryAgg { child, .. } => { + let (cid, _) = self.visit_l4node(child)?; self.dag.edges.push((id, cid)); StageId::Edge } - // ── SketchEstimate: always backend per design.md §6. + // ── SummaryEstimate: always backend per design.md §6. // The "SketchEstimate MUST be on the same stage as its // consumers (typically Backend)" invariant is satisfied - // because consumers above SketchEstimate are also backend. - PhysicalExpr::SketchEstimate { child, .. } => { - let (cid, child_stage) = self.visit(child)?; + // because consumers above SummaryEstimate are also backend. + SummaryExpr::SummaryEstimate { sketch_input, .. } => { + let (cid, child_stage) = self.visit_l4node(sketch_input)?; self.dag.edges.push((id, cid)); // If child is on edge or gateway, this is a cross-stage // edge — that's expected (the wire-format hop). @@ -137,66 +202,68 @@ impl ThreeStageWalker { StageId::Backend } - // ── SketchMerge: gateway under three-stage. Children are - // edge SketchAgg outputs. - PhysicalExpr::SketchMerge { children, .. } => { + // ── SummaryMerge: gateway under three-stage. Children are + // edge SummaryAgg outputs. + SummaryExpr::SummaryMerge { children } => { for child in children { - let (cid, _) = self.visit(child)?; + let (cid, _) = self.visit_l4node(child)?; self.dag.edges.push((id, cid)); } StageId::Gateway } - // ── LetBinding: colour by the bound expression's stage, - // and bring the binding into scope before walking the body. - PhysicalExpr::LetBinding { name, expr, child } => { - let (eid, expr_stage) = self.visit(expr)?; - self.dag.edges.push((id, eid)); - self.scope.insert(name.as_str().to_string(), expr_stage); - let (bid, _) = self.visit(child)?; - self.dag.edges.push((id, bid)); - expr_stage + // ── SummaryJoin / SummarySubtract / SummaryDelete: not + // surfaced by any `Bind*` path yet (gated on rules that + // haven't landed — see `physical_expr.rs`'s module docs' + // predecessor note). Conservative default matching + // SummaryMerge's multi-input-combination shape until a real + // consumer picks a placement. + SummaryExpr::SummaryJoin { outer, inner, .. } => { + let (oid, _) = self.visit_l4node(outer)?; + self.dag.edges.push((id, oid)); + let (iid, _) = self.visit_l4node(inner)?; + self.dag.edges.push((id, iid)); + StageId::Gateway } - - // ── Ref: colour matches the binding's stage. Unresolved - // refs bubble up as `AllocateError::UnresolvedRef`. - PhysicalExpr::Ref { name } => self - .scope - .get(name.as_str()) - .copied() - .ok_or_else(|| AllocateError::UnresolvedRef(name.as_str().to_string()))?, - - // ── Phase ε.1 Mode 2: raw at edge, sketch built at backend. - // Edge ships raw OTLP — we stage as Edge so the L5 emitter's - // edge-side YAML pipeline picks it up; the sketch construction - // itself happens at the backend (no edge sketch processor). - PhysicalExpr::RawAtEdgeSketchAtBackend { child, .. } => { - let (cid, _) = self.visit(child)?; - self.dag.edges.push((id, cid)); - StageId::Edge + SummaryExpr::SummarySubtract { left, right } => { + let (lid, _) = self.visit_l4node(left)?; + self.dag.edges.push((id, lid)); + let (rid, _) = self.visit_l4node(right)?; + self.dag.edges.push((id, rid)); + StageId::Gateway } - - // ── Phase ε.1 Mode 3: raw at edge, ships directly to - // Prometheus's native OTLP receiver. The agent pipeline picks - // this up via `asap.mode=prometheus_archive` routing. - PhysicalExpr::RawAtEdgePrometheusArchive { .. } => StageId::Edge, - - // ── ExactAgg (PR-6 follow-up): same shape as SketchAgg — - // produces typed state at the edge. The accumulator runs on - // the edge precompute pipeline; the backend's - // `SketchStoreSink::append_to_index` writes the final - // (sid, window, accumulator) tuples it ships. Coloured - // Edge to match the sketch path's locality. - PhysicalExpr::ExactAgg { child, .. } => { - let (cid, _) = self.visit(child)?; + SummaryExpr::SummaryDelete { sketch_input, .. } => { + let (cid, _) = self.visit_l4node(sketch_input)?; self.dag.edges.push((id, cid)); - StageId::Edge + StageId::Gateway } }; - // Patch in the resolved stage now that children have been visited. - self.dag.nodes[id.0].stage = stage; + self.finish_node(id, stage) + } + /// Reserve a slot for a node up-front so child IDs are strictly + /// larger than the parent's; downstream `cut_edges` analysis assumes + /// parents come before children in `nodes`. + fn reserve_node(&mut self, expr: PhysicalExpr) -> NodeId { + let id = NodeId(self.dag.nodes.len()); + self.dag.nodes.push(ColoredNode { + id, + expr, + // Placeholder — overwritten by `finish_node` once children + // have been coloured. + stage: StageId::Edge, + }); + id + } + + /// Patch in the resolved stage now that children have been visited. + fn finish_node( + &mut self, + id: NodeId, + stage: StageId, + ) -> Result<(NodeId, StageId), AllocateError> { + self.dag.nodes[id.0].stage = stage; Ok((id, stage)) } @@ -263,7 +330,9 @@ impl ThreeStageWalker { // stage lookup keyed by binding name. pub(crate) fn binding_stage(dag: &ColoredDag, name: &BindingName) -> Option { dag.nodes.iter().find_map(|n| match &n.expr { - PhysicalExpr::LetBinding { name: n2, .. } if n2 == name => Some(n.stage), + PhysicalExpr::Committed(L4Plan::LetBinding { name: n2, .. }) if n2 == name => { + Some(n.stage) + } _ => None, }) } @@ -274,9 +343,7 @@ pub(crate) fn binding_stage(dag: &ColoredDag, name: &BindingName) -> Option QueryExpr { @@ -317,7 +384,9 @@ mod tests { #[test] fn allocate_unsupported_topology_errors() { - let leaf = PhysicalExpr::Logical(ts_scan()); + let leaf = PhysicalExpr::committed( + asap_plan::bind::logical(&ts_scan(), &BindingScope::default()).unwrap(), + ); let err = StageAllocator .allocate(&leaf, Topology::SingleStage) .unwrap_err(); @@ -329,18 +398,25 @@ mod tests { #[test] fn three_stage_quantile_dag_basic() { - let expr = PhysicalExpr::estimate_over_agg( - EstimateOp::Quantile { q: 0.99 }, - SummaryKind::Kll, - SummaryParams::Kll { k: 200 }, - windowed_scan(), - ); + let q = QueryExpr::Aggregate { + by: crate::intent_algebra::GroupKeys::none(), + aggs: vec![crate::intent_algebra::AggIntent::Quantile { + col: None, + q: 0.99, + accuracy: crate::types_v2::AccuracyTarget::Epsilon(0.01), + }], + output_names: Vec::new(), + having: None, + child: Box::new(windowed_scan()), + }; + let node = asap_plan::bind::implement_tree(&q).unwrap(); + let expr = PhysicalExpr::committed(node); let dag = StageAllocator .allocate(&expr, Topology::ThreeStage) .unwrap(); - // root = SketchEstimate → Backend + // root = SummaryEstimate → Backend assert_eq!(dag.root().unwrap().stage, StageId::Backend); - // node 1 = SketchAgg → Edge + // node 1 = SummaryAgg → Edge assert_eq!(dag.nodes[1].stage, StageId::Edge); // node 2 = Logical(Window) → Edge assert_eq!(dag.nodes[2].stage, StageId::Edge); diff --git a/control_plane/src/physical/colored_dag/dag.rs b/control_plane/src/physical/colored_dag/dag.rs index 4f3bfed9..fe08ca8b 100644 --- a/control_plane/src/physical/colored_dag/dag.rs +++ b/control_plane/src/physical/colored_dag/dag.rs @@ -46,7 +46,7 @@ impl std::fmt::Display for NodeId { /// [`ColoredDag::edges`]). Test-friendly variant: when callers want the /// full sub-tree they can rebuild from the original `PhysicalExpr` using /// `NodeId` as the index. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct ColoredNode { /// Position-based identifier — index into `ColoredDag::nodes`. pub id: NodeId, @@ -144,30 +144,70 @@ impl ColoredDag { #[cfg(test)] mod tests { use super::*; - use crate::intent_algebra::QueryExpr; - use crate::sketch_algebra::physical_expr::EstimateOp; + use crate::intent_algebra::schema::{Column, DataType}; + use crate::intent_algebra::{BindingScope, QueryExpr, Schema, Source}; use crate::sketch_algebra::PhysicalExpr; - use asap_sketch::{SummaryKind, SummaryParams}; + + // These three dummies only need to be *structurally valid* and + // distinct `PhysicalExpr` values — the tests below only inspect + // `ColoredNode::stage`, never `expr`'s internal shape. + fn dummy_scan() -> QueryExpr { + QueryExpr::Scan { + source: Source::TimeSeries { + metric: "dummy_metric".into(), + }, + predicates: vec![], + schema: Schema::with_time_index( + vec![ + Column { + name: "ts".into(), + dtype: DataType::Timestamp, + nullable: false, + table: None, + }, + Column { + name: "value".into(), + dtype: DataType::Float64, + nullable: false, + table: None, + }, + ], + 0, + vec![vec![0]], + ), + } + } fn dummy_logical() -> PhysicalExpr { - PhysicalExpr::Logical(QueryExpr::Ref { - name: asap_ir::intent_algebra::BindingName::new("dummy"), - }) + PhysicalExpr::committed( + asap_plan::bind::logical(&dummy_scan(), &BindingScope::default()).unwrap(), + ) } fn dummy_agg() -> PhysicalExpr { - PhysicalExpr::SketchAgg { - sketch_type: SummaryKind::Kll, - params: SummaryParams::Kll { k: 200 }, - child: Box::new(dummy_logical()), - } + let q = QueryExpr::Aggregate { + by: crate::intent_algebra::GroupKeys::none(), + aggs: vec![crate::intent_algebra::AggIntent::Sum { col: None }], + output_names: Vec::new(), + having: None, + child: Box::new(dummy_scan()), + }; + PhysicalExpr::committed(asap_plan::bind::implement_tree(&q).unwrap()) } fn dummy_estimate() -> PhysicalExpr { - PhysicalExpr::SketchEstimate { - op: EstimateOp::Quantile { q: 0.99 }, - child: Box::new(dummy_agg()), - } + let q = QueryExpr::Aggregate { + by: crate::intent_algebra::GroupKeys::none(), + aggs: vec![crate::intent_algebra::AggIntent::Quantile { + col: None, + q: 0.99, + accuracy: crate::types_v2::AccuracyTarget::Epsilon(0.01), + }], + output_names: Vec::new(), + having: None, + child: Box::new(dummy_scan()), + }; + PhysicalExpr::committed(asap_plan::bind::implement_tree(&q).unwrap()) } #[test] diff --git a/control_plane/src/physical/colored_dag/emitter.rs b/control_plane/src/physical/colored_dag/emitter.rs index 2b8f6213..dcd71a4a 100644 --- a/control_plane/src/physical/colored_dag/emitter.rs +++ b/control_plane/src/physical/colored_dag/emitter.rs @@ -33,8 +33,95 @@ use serde::{Deserialize, Serialize}; use crate::physical::colored_dag::dag::ColoredDag; use crate::physical::colored_dag::stage_id::{StageId, Topology}; -use crate::sketch_algebra::physical_expr::{EstimateOp, PhysicalExpr}; -use asap_sketch::{SummaryKind, SummaryParams}; +use crate::sketch_algebra::physical_expr::{L4Plan, PhysicalExpr}; +use crate::types_v2::BindingName; +use asap_sketch::{SketchQuery, SummaryExpr, SummaryKind, SummaryParams}; + +/// Flattened view of one [`ColoredNode`](crate::physical::colored_dag::dag::ColoredNode)'s +/// `PhysicalExpr`, for the tuple-style `(&node.expr, node.stage)` matching +/// this file uses throughout. Mirrors the shape the old, locally-defined +/// flat `PhysicalExpr` had before Step B folded most of its variants into +/// `asap_sketch::SummaryExpr` — see `sketch_algebra::physical_expr`'s +/// module docs. +enum NodeKind<'a> { + Logical(&'a crate::intent_algebra::QueryExpr), + /// An approximate sketch — the old `SketchAgg`. Exact accumulators + /// (Sum/Count/MinMax/Increase/Rate) are classified as [`Self::ExactAgg`] + /// instead, matching the old `PhysicalExpr::ExactAgg`'s separate shape. + SketchAgg { + sketch_type: &'a SummaryKind, + params: &'a SummaryParams, + }, + /// An exact accumulator — the old `PhysicalExpr::ExactAgg`. This + /// emitter has never had a match arm for it (falls through to the + /// catch-all below, same as before Step B — a pre-existing gap, not + /// introduced by this migration). + ExactAgg, + SketchEstimate { query: &'a SketchQuery }, + SketchMerge, + LetBinding { name: &'a BindingName }, + Ref { name: &'a BindingName }, + RawAtEdgeSketchAtBackend { + family: &'a SummaryKind, + params: &'a SummaryParams, + }, + RawAtEdgePrometheusArchive { + metric: &'a str, + window: Option, + label_proj: &'a [String], + }, + /// `SummaryJoin` / `SummarySubtract` / `SummaryDelete` — not surfaced + /// by any `Bind*` path yet (gated on rules that haven't landed). + Other, +} + +/// Is `kind` an exact accumulator (Sum/Count/MinMax/Increase/Rate) rather +/// than an approximate sketch? +fn is_exact_accumulator(kind: &SummaryKind) -> bool { + matches!( + kind, + SummaryKind::Sum + | SummaryKind::Count + | SummaryKind::MinMax + | SummaryKind::Increase + | SummaryKind::Rate + ) +} + +fn classify(expr: &PhysicalExpr) -> NodeKind<'_> { + match expr { + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::Logical(qe) => NodeKind::Logical(qe), + SummaryExpr::SummaryAgg { sketch, params, .. } if is_exact_accumulator(sketch) => { + let _ = params; + NodeKind::ExactAgg + } + SummaryExpr::SummaryAgg { sketch, params, .. } => NodeKind::SketchAgg { + sketch_type: sketch, + params, + }, + SummaryExpr::SummaryEstimate { query, .. } => NodeKind::SketchEstimate { query }, + SummaryExpr::SummaryMerge { .. } => NodeKind::SketchMerge, + SummaryExpr::SummaryJoin { .. } + | SummaryExpr::SummarySubtract { .. } + | SummaryExpr::SummaryDelete { .. } => NodeKind::Other, + }, + PhysicalExpr::Committed(L4Plan::LetBinding { name, .. }) => NodeKind::LetBinding { name }, + PhysicalExpr::Committed(L4Plan::Ref { name }) => NodeKind::Ref { name }, + PhysicalExpr::RawAtEdgeSketchAtBackend { family, params, .. } => { + NodeKind::RawAtEdgeSketchAtBackend { family, params } + } + PhysicalExpr::RawAtEdgePrometheusArchive { + metric, + window, + label_proj, + } => NodeKind::RawAtEdgePrometheusArchive { + metric, + window: *window, + label_proj, + }, + } +} /// Errors surfaced by [`Emitter::emit_per_stage`]. #[derive(Debug, thiserror::Error, PartialEq)] @@ -73,7 +160,9 @@ pub trait Emitter { /// The variants are deliberately struct-shaped (named fields) so future /// downstream consumers can pattern-match without relying on tuple-index /// stability. -#[derive(Debug, Clone, PartialEq)] +/// Not `PartialEq` — `Backend` carries `BackendStageConfig`, which isn't +/// `PartialEq` either (see its doc comment). +#[derive(Debug, Clone)] pub enum StageConfig { /// Edge agent's logical config — what the OpAMP push for this /// agent will need to materialise into OTel collector YAML. @@ -529,7 +618,14 @@ pub struct GatewayMergeProcessor { } /// Logical content of the backend `StreamingConfig`. -#[derive(Debug, Clone, PartialEq)] +/// +/// Not `PartialEq` — `readouts: Vec` carries +/// `asap_sketch::SketchQuery`, which (like `SummaryExpr`/`L4Node`) has no +/// `PartialEq` impl upstream. Nothing on the real emit path compares +/// whole `BackendStageConfig`/`BackendReadout` values — every actual +/// wire payload goes through `build_backend_readout_json`'s hand-written +/// JSON builder, never a whole-struct comparison. +#[derive(Debug, Clone)] pub struct BackendStageConfig { /// One entry per readout query the backend must serve. The /// `aggregation_id` in each routing entry is the backend's @@ -641,12 +737,15 @@ pub enum AggregationInput { } /// One readout entry — what the backend's inference YAML asks for. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +/// +/// Not `PartialEq`/`Serialize`/`Deserialize` — `op: SketchQuery` has none +/// of those upstream (see `BackendStageConfig`'s doc comment). +#[derive(Debug, Clone)] pub struct BackendReadout { /// Aggregation this readout reads from. pub aggregation_id: String, - /// Readout op (mirror of `PhysicalExpr::SketchEstimate::op`). - pub op: EstimateOp, + /// Readout op (mirror of `SummaryExpr::SummaryEstimate::query`). + pub op: SketchQuery, } /// Abstract OTLP / HTTP endpoint description. Phase E does not resolve @@ -762,7 +861,7 @@ impl Emitter for ThreeStageEmitter { // `label_filters` — so the Pass 2 arm that also calls it // remains correct (no double-counting). for node in &dag.nodes { - if let (PhysicalExpr::Logical(qe), StageId::Edge) = (&node.expr, node.stage) { + if let (NodeKind::Logical(qe), StageId::Edge) = (classify(&node.expr), node.stage) { extract_edge_facts(qe, &mut edge); } } @@ -771,7 +870,8 @@ impl Emitter for ThreeStageEmitter { // SketchAgg up-front so SketchMerge / SketchEstimate emission // (pass 2) can resolve them regardless of node-table order. for node in &dag.nodes { - if let (PhysicalExpr::SketchAgg { .. }, StageId::Edge) = (&node.expr, node.stage) { + if let (NodeKind::SketchAgg { .. }, StageId::Edge) = (classify(&node.expr), node.stage) + { let aggregation_id = format!("agg{next_agg_index}"); next_agg_index += 1; sketch_agg_ids.insert(node.id.0, aggregation_id); @@ -780,19 +880,18 @@ impl Emitter for ThreeStageEmitter { // Pass 2 — emit per-stage facts. for node in &dag.nodes { - match (&node.expr, node.stage) { + match (classify(&node.expr), node.stage) { // Edge: source metric + label filters from Logical // — the wrapped L3 sub-tree may be Scan, Window{Scan}, // Aggregate{Window{Scan}} etc., so descend recursively. - (PhysicalExpr::Logical(qe), StageId::Edge) => { + (NodeKind::Logical(qe), StageId::Edge) => { extract_edge_facts(qe, &mut edge); } // Edge: SketchAgg becomes one EdgeSketchProcessor. ( - PhysicalExpr::SketchAgg { + NodeKind::SketchAgg { sketch_type, params, - .. }, StageId::Edge, ) => { @@ -831,7 +930,7 @@ impl Emitter for ThreeStageEmitter { // (looked up via the DAG's edges table so identical // child sub-trees don't collide on a position-by-expr // search). - (PhysicalExpr::SketchMerge { .. }, StageId::Gateway) => { + (NodeKind::SketchMerge, StageId::Gateway) => { if let Some((kind, aid)) = first_sketch_child_via_edges(dag, node.id, &sketch_agg_ids) { @@ -845,12 +944,12 @@ impl Emitter for ThreeStageEmitter { // Backend: SketchEstimate → one readout entry. The // matching aggregation_id comes from the descendant // SketchAgg (resolved by walking the DAG edges table). - (PhysicalExpr::SketchEstimate { op, .. }, StageId::Backend) => { + (NodeKind::SketchEstimate { query }, StageId::Backend) => { let aid = resolve_descendant_agg_id_via_edges(dag, node.id, &sketch_agg_ids) .unwrap_or_else(|| format!("agg{}", readouts.len())); readouts.push(BackendReadout { aggregation_id: aid, - op: op.clone(), + op: query.clone(), }); } // ── Phase ε.1 Mode 3: edge raw → Prometheus OTLP receiver. @@ -859,7 +958,7 @@ impl Emitter for ThreeStageEmitter { // pipeline. Backend gets a `prometheus_remote` storage // routing target (no aggregation entry). ( - PhysicalExpr::RawAtEdgePrometheusArchive { + NodeKind::RawAtEdgePrometheusArchive { metric, window, label_proj, @@ -868,9 +967,9 @@ impl Emitter for ThreeStageEmitter { ) => { edge.prometheus_archive_metrics .push(PrometheusArchiveMetric { - metric: metric.clone(), + metric: metric.to_string(), window_secs: window.map(|d| d.as_secs()), - label_proj: label_proj.clone(), + label_proj: label_proj.to_vec(), }); // Phase 3.2.5 (Bug a): Mode-3 metrics also land in // the Gorilla-S3 archive so the ASAP-tier @@ -879,7 +978,7 @@ impl Emitter for ThreeStageEmitter { // is emitted by the L5 emitter when this list is // non-empty. edge.archive_tier_metrics.push(ArchiveTierMetric { - metric: metric.clone(), + metric: metric.to_string(), window_secs: window.map(|d| d.as_secs()), }); } @@ -888,7 +987,7 @@ impl Emitter for ThreeStageEmitter { // BackendAggregation with the family the backend will // build at ingest. The aggregation_input=raw flag is // emitted by `emit_backend_streaming_config_json`. - (PhysicalExpr::RawAtEdgeSketchAtBackend { family, params, .. }, StageId::Edge) => { + (NodeKind::RawAtEdgeSketchAtBackend { family, params }, StageId::Edge) => { let aid = format!("agg{next_agg_index}"); next_agg_index += 1; backend_aggregations.push(BackendAggregation { @@ -1097,25 +1196,25 @@ fn first_sketch_child_via_edges( ) -> Option<(SummaryKind, String)> { for cid in children_of(dag, parent) { let cnode = dag.nodes.get(cid.0)?; - match &cnode.expr { - PhysicalExpr::SketchAgg { sketch_type, .. } => { + match classify(&cnode.expr) { + NodeKind::SketchAgg { sketch_type, .. } => { if let Some(aid) = sketch_agg_ids.get(&cid.0) { return Some((sketch_type.clone(), aid.clone())); } } - PhysicalExpr::LetBinding { .. } | PhysicalExpr::SketchMerge { .. } => { + NodeKind::LetBinding { .. } | NodeKind::SketchMerge => { if let Some(found) = first_sketch_child_via_edges(dag, cid, sketch_agg_ids) { return Some(found); } } - PhysicalExpr::Ref { name } => { + NodeKind::Ref { name } => { // Resolve the ref to its binding's expr id, then recurse. if let Some(bid) = dag .nodes .iter() .enumerate() - .find_map(|(i, n)| match &n.expr { - PhysicalExpr::LetBinding { name: n2, .. } if n2 == name => Some(i), + .find_map(|(i, n)| match classify(&n.expr) { + NodeKind::LetBinding { name: n2 } if n2 == name => Some(i), _ => None, }) { diff --git a/control_plane/src/physical/colored_dag/tests.rs b/control_plane/src/physical/colored_dag/tests.rs index d74c3eb9..642411fb 100644 --- a/control_plane/src/physical/colored_dag/tests.rs +++ b/control_plane/src/physical/colored_dag/tests.rs @@ -7,16 +7,20 @@ #![cfg(test)] +use std::rc::Rc; use std::time::Duration; +use asap_sketch::{L4Node, L4Schema, SketchQuery, SummaryExpr, SummaryKind, SummaryParams}; + +use crate::intent_algebra::{ + BindingScope, ColumnRef, GroupKeys, LabelFilter, QueryExpr, Schema, Source, WindowKind, +}; use crate::intent_algebra::schema::{Column, DataType}; -use crate::intent_algebra::{LabelFilter, QueryExpr, Schema, Source, WindowKind}; use crate::physical::colored_dag::allocator::StageAllocator; use crate::physical::colored_dag::emitter::{EmitError, Emitter, StageConfig, ThreeStageEmitter}; use crate::physical::colored_dag::stage_id::{StageId, Topology}; -use crate::sketch_algebra::physical_expr::{EstimateOp, MergeAlgebra, PhysicalExpr}; +use crate::sketch_algebra::physical_expr::{L4Plan, PhysicalExpr}; use crate::types_v2::{AccuracyTarget, BindingName}; -use asap_sketch::{SummaryKind, SummaryParams}; // ── Test fixtures ───────────────────────────────────────────────────────────── @@ -78,15 +82,99 @@ fn windowed_scan() -> QueryExpr { } } -/// `SketchEstimate{Quantile{0.99}}{SketchAgg{KLL}{Logical(Window{Scan})}}` -/// — the §6 single-query trace input. +/// Empty `L4Schema` — the coloring/emitter logic under test here never +/// inspects node schemas (only `SummaryExpr` shape + `PhysicalExpr` +/// placement), so hand-built L4 nodes below carry a placeholder, same +/// spirit as this file's old comment: "these dummies only need to be +/// *structurally valid* and distinct `PhysicalExpr` values". +fn dummy_l4_schema() -> L4Schema { + L4Schema { + fields: vec![], + time_index: None, + } +} + +/// Wrap `qe` as an unbound `Logical` L4 leaf — mirrors the old +/// `PhysicalExpr::Logical(qe)` construction for hand-built fixtures. +fn logical_l4(qe: QueryExpr) -> Rc { + asap_plan::bind::logical(&qe, &BindingScope::default()).unwrap() +} + +/// Hand-build a `SummaryAgg` node — mirrors the old +/// `PhysicalExpr::SketchAgg { sketch_type, params, child }` construction, +/// for fixtures that need a specific family without going through +/// `implement_tree`'s cost-model selection. +fn sketch_agg_l4(sketch: SummaryKind, params: SummaryParams, child: Rc) -> Rc { + Rc::new(L4Node { + expr: SummaryExpr::SummaryAgg { + child, + sketch, + params, + col: ColumnRef::SampleValue, + by: vec![], + }, + schema: dummy_l4_schema(), + }) +} + +/// Hand-build a `SummaryEstimate` node — mirrors the old +/// `PhysicalExpr::SketchEstimate { op, child }`. +fn estimate_l4(query: SketchQuery, sketch_input: Rc) -> Rc { + Rc::new(L4Node { + expr: SummaryExpr::SummaryEstimate { sketch_input, query }, + schema: dummy_l4_schema(), + }) +} + +/// Hand-build a `SummaryMerge` node — mirrors the old +/// `PhysicalExpr::SketchMerge { algebra, children }`. `SummaryMerge` (the +/// upstream replacement) carries no `algebra` field — `MergeAlgebra` was +/// this crate's own addition and doesn't exist upstream (see +/// `physical_expr.rs`'s module docs). +fn merge_l4(children: Vec>) -> Rc { + Rc::new(L4Node { + expr: SummaryExpr::SummaryMerge { children }, + schema: dummy_l4_schema(), + }) +} + +fn is_sketch_agg(expr: &PhysicalExpr) -> bool { + matches!(expr, PhysicalExpr::Committed(L4Plan::Summary(n)) if matches!(n.expr, SummaryExpr::SummaryAgg { .. })) +} +fn is_logical(expr: &PhysicalExpr) -> bool { + matches!(expr, PhysicalExpr::Committed(L4Plan::Summary(n)) if matches!(n.expr, SummaryExpr::Logical(_))) +} +fn is_sketch_estimate(expr: &PhysicalExpr) -> bool { + matches!(expr, PhysicalExpr::Committed(L4Plan::Summary(n)) if matches!(n.expr, SummaryExpr::SummaryEstimate { .. })) +} +fn is_sketch_merge(expr: &PhysicalExpr) -> bool { + matches!(expr, PhysicalExpr::Committed(L4Plan::Summary(n)) if matches!(n.expr, SummaryExpr::SummaryMerge { .. })) +} +fn is_let_binding(expr: &PhysicalExpr) -> bool { + matches!(expr, PhysicalExpr::Committed(L4Plan::LetBinding { .. })) +} +fn is_ref(expr: &PhysicalExpr) -> bool { + matches!(expr, PhysicalExpr::Committed(L4Plan::Ref { .. })) +} + +/// `SummaryEstimate{Quantile{0.99}}{SummaryAgg{Kll}{Logical(Window{Scan})}}` +/// — the §6 single-query trace input. Built via `implement_tree` (the +/// default cost model ranks Kll first for `Quantile`, matching this +/// fixture's old hand-built KLL family — see `asap-plan`'s +/// `boundary::summary_candidates`). fn quantile_kll_dag() -> PhysicalExpr { - PhysicalExpr::estimate_over_agg( - EstimateOp::Quantile { q: 0.99 }, - SummaryKind::Kll, - SummaryParams::Kll { k: 200 }, - windowed_scan(), - ) + let q = QueryExpr::Aggregate { + by: GroupKeys::none(), + aggs: vec![crate::intent_algebra::AggIntent::Quantile { + col: None, + q: 0.99, + accuracy: AccuracyTarget::Epsilon(0.01), + }], + output_names: Vec::new(), + having: None, + child: Box::new(windowed_scan()), + }; + PhysicalExpr::committed(asap_plan::bind::implement_tree(&q).unwrap()) } // ── Allocator: per-rule + edge-case tests ───────────────────────────────────── @@ -96,43 +184,42 @@ fn allocator_three_stage_basic() { let dag = StageAllocator .allocate(&quantile_kll_dag(), Topology::ThreeStage) .expect("allocate ok"); - // root = SketchEstimate → Backend + // root = SummaryEstimate → Backend assert_eq!(dag.root().unwrap().stage, StageId::Backend); - // 4 nodes total: SketchEstimate, SketchAgg, Logical(Window), - // Logical(Scan) — Phase E walks the Logical(Window) child via the - // SketchAgg path; the inner Scan only surfaces if Logical is - // recursively unfolded. Today Logical wraps the entire L3 sub-tree - // as a single PhysicalExpr node, so the count is 3. + // 3 nodes total: SummaryEstimate, SummaryAgg, Logical(Window{Scan}) + // — `Logical` wraps the entire L3 sub-tree as a single L4 node, so + // the inner Scan only surfaces if Logical is recursively unfolded + // (it isn't). assert!( dag.nodes.len() >= 3, "expected at least 3 nodes, got {}", dag.nodes.len() ); - // SketchAgg is colored Edge. + // SummaryAgg is colored Edge. let agg = dag .nodes .iter() - .find(|n| matches!(n.expr, PhysicalExpr::SketchAgg { .. })) - .expect("SketchAgg present"); + .find(|n| is_sketch_agg(&n.expr)) + .expect("SummaryAgg present"); assert_eq!(agg.stage, StageId::Edge); // Logical wrapper of Window is colored Edge. let win_or_scan = dag .nodes .iter() - .find(|n| matches!(n.expr, PhysicalExpr::Logical(_))) + .find(|n| is_logical(&n.expr)) .expect("Logical present"); assert_eq!(win_or_scan.stage, StageId::Edge); } #[test] fn allocator_sketch_agg_under_scan_pinned_edge() { - // Exact design.md §6 invariant: a SketchAgg whose child is a Scan + // Exact design.md §6 invariant: a SummaryAgg whose child is a Scan // (wrapped in Logical) MUST land on Edge. - let expr = PhysicalExpr::SketchAgg { - sketch_type: SummaryKind::Hll, - params: SummaryParams::Hll { precision: 14 }, - child: Box::new(PhysicalExpr::Logical(ts_scan("events", None))), - }; + let expr = PhysicalExpr::committed(sketch_agg_l4( + SummaryKind::Hll, + SummaryParams::Hll { precision: 14 }, + logical_l4(ts_scan("events", None)), + )); let dag = StageAllocator .allocate(&expr, Topology::ThreeStage) .unwrap(); @@ -142,44 +229,53 @@ fn allocator_sketch_agg_under_scan_pinned_edge() { #[test] fn allocator_sketch_estimate_pinned_backend() { - // SketchEstimate MUST be on Backend (the readout side). + // SummaryEstimate MUST be on Backend (the readout side). let dag = StageAllocator .allocate(&quantile_kll_dag(), Topology::ThreeStage) .unwrap(); let est = dag .nodes .iter() - .find(|n| matches!(n.expr, PhysicalExpr::SketchEstimate { .. })) - .expect("SketchEstimate present"); + .find(|n| is_sketch_estimate(&n.expr)) + .expect("SummaryEstimate present"); assert_eq!(est.stage, StageId::Backend); } #[test] fn allocator_let_binding_color_propagates() { // LetBinding takes the bound expression's stage. Bind a - // SketchAgg{KLL} (edge) and verify the LetBinding node colors edge. - let inner_agg = PhysicalExpr::SketchAgg { - sketch_type: SummaryKind::Kll, - params: SummaryParams::Kll { k: 200 }, - child: Box::new(PhysicalExpr::Logical(windowed_scan())), - }; - let bind = PhysicalExpr::LetBinding { + // SummaryAgg{Kll} (edge) and verify the LetBinding node colors edge. + // + // NOTE — shape change forced by the type system, not just syntax: + // the old fixture nested `Ref` *inside* a `SketchEstimate`'s child. + // The new `SummaryEstimate::sketch_input` field is `Rc` — + // upstream `asap_sketch`'s own type, which has no `Ref`/`LetBinding` + // concept at all — so a `Ref`/`LetBinding` can only appear where an + // `L4Plan` is expected (this crate's own named-binding sharing + // mechanism layered *above* `L4Node`, not inside it; see + // `physical_expr.rs`'s module docs). The property under test — + // LetBinding colors by its bound expression's stage — is preserved + // with the `child` position held by a bare `Ref` instead of a + // `SketchEstimate{child: Ref}`. + let inner_agg = sketch_agg_l4( + SummaryKind::Kll, + SummaryParams::Kll { k: 200 }, + logical_l4(windowed_scan()), + ); + let bind = PhysicalExpr::Committed(L4Plan::LetBinding { name: BindingName::new("kll_state"), - expr: Box::new(inner_agg), - child: Box::new(PhysicalExpr::SketchEstimate { - op: EstimateOp::Quantile { q: 0.95 }, - child: Box::new(PhysicalExpr::Ref { - name: BindingName::new("kll_state"), - }), + expr: Rc::new(L4Plan::Summary(inner_agg)), + child: Rc::new(L4Plan::Ref { + name: BindingName::new("kll_state"), }), - }; + }); let dag = StageAllocator .allocate(&bind, Topology::ThreeStage) .unwrap(); let let_node = dag .nodes .iter() - .find(|n| matches!(n.expr, PhysicalExpr::LetBinding { .. })) + .find(|n| is_let_binding(&n.expr)) .expect("LetBinding present"); // LetBinding takes its expr's stage → Edge. assert_eq!(let_node.stage, StageId::Edge); @@ -187,58 +283,56 @@ fn allocator_let_binding_color_propagates() { #[test] fn allocator_ref_resolves_to_binding_stage() { - // Ref takes the stage of its binding. Same fixture as above; Ref - // child of SketchEstimate must color Edge (the binding's stage). - let inner_agg = PhysicalExpr::SketchAgg { - sketch_type: SummaryKind::Kll, - params: SummaryParams::Kll { k: 200 }, - child: Box::new(PhysicalExpr::Logical(windowed_scan())), - }; - let bind = PhysicalExpr::LetBinding { + // Ref takes the stage of its binding. Same fixture shape as + // `allocator_let_binding_color_propagates` (see the shape-change + // note there); the Ref child of the LetBinding must color Edge (the + // binding's stage). + let inner_agg = sketch_agg_l4( + SummaryKind::Kll, + SummaryParams::Kll { k: 200 }, + logical_l4(windowed_scan()), + ); + let bind = PhysicalExpr::Committed(L4Plan::LetBinding { name: BindingName::new("shared"), - expr: Box::new(inner_agg), - child: Box::new(PhysicalExpr::SketchEstimate { - op: EstimateOp::Quantile { q: 0.5 }, - child: Box::new(PhysicalExpr::Ref { - name: BindingName::new("shared"), - }), + expr: Rc::new(L4Plan::Summary(inner_agg)), + child: Rc::new(L4Plan::Ref { + name: BindingName::new("shared"), }), - }; + }); let dag = StageAllocator .allocate(&bind, Topology::ThreeStage) .unwrap(); let ref_node = dag .nodes .iter() - .find(|n| matches!(n.expr, PhysicalExpr::Ref { .. })) + .find(|n| is_ref(&n.expr)) .expect("Ref present"); assert_eq!(ref_node.stage, StageId::Edge); } #[test] fn allocator_sketch_merge_lands_gateway() { - // SketchMerge over edge-built KLL sketches → Gateway. - let one_agg = || PhysicalExpr::SketchAgg { - sketch_type: SummaryKind::Kll, - params: SummaryParams::Kll { k: 200 }, - child: Box::new(PhysicalExpr::Logical(windowed_scan())), - }; - let merge = PhysicalExpr::SketchMerge { - algebra: MergeAlgebra::Union, - children: vec![one_agg(), one_agg()], - }; - let with_estimate = PhysicalExpr::SketchEstimate { - op: EstimateOp::Quantile { q: 0.99 }, - child: Box::new(merge), + // SummaryMerge over edge-built KLL sketches → Gateway. + let one_agg = || { + sketch_agg_l4( + SummaryKind::Kll, + SummaryParams::Kll { k: 200 }, + logical_l4(windowed_scan()), + ) }; + let merge = merge_l4(vec![one_agg(), one_agg()]); + let with_estimate = estimate_l4(SketchQuery::Quantile { q: 0.99 }, merge); let dag = StageAllocator - .allocate(&with_estimate, Topology::ThreeStage) + .allocate( + &PhysicalExpr::committed(with_estimate), + Topology::ThreeStage, + ) .unwrap(); let merge_node = dag .nodes .iter() - .find(|n| matches!(n.expr, PhysicalExpr::SketchMerge { .. })) - .expect("SketchMerge present"); + .find(|n| is_sketch_merge(&n.expr)) + .expect("SummaryMerge present"); assert_eq!(merge_node.stage, StageId::Gateway); assert_eq!(dag.root().unwrap().stage, StageId::Backend); } @@ -247,23 +341,19 @@ fn allocator_sketch_merge_lands_gateway() { #[test] fn emitter_three_stage_emits_three_configs() { - // Build a DAG with all three stages occupied: SketchEstimate over - // SketchMerge over two SketchAggs. - let one_agg = || PhysicalExpr::SketchAgg { - sketch_type: SummaryKind::Kll, - params: SummaryParams::Kll { k: 200 }, - child: Box::new(PhysicalExpr::Logical(windowed_scan())), - }; - let merge = PhysicalExpr::SketchMerge { - algebra: MergeAlgebra::Union, - children: vec![one_agg(), one_agg()], - }; - let root = PhysicalExpr::SketchEstimate { - op: EstimateOp::Quantile { q: 0.99 }, - child: Box::new(merge), + // Build a DAG with all three stages occupied: SummaryEstimate over + // SummaryMerge over two SummaryAggs. + let one_agg = || { + sketch_agg_l4( + SummaryKind::Kll, + SummaryParams::Kll { k: 200 }, + logical_l4(windowed_scan()), + ) }; + let merge = merge_l4(vec![one_agg(), one_agg()]); + let root = estimate_l4(SketchQuery::Quantile { q: 0.99 }, merge); let dag = StageAllocator - .allocate(&root, Topology::ThreeStage) + .allocate(&PhysicalExpr::committed(root), Topology::ThreeStage) .unwrap(); let configs = ThreeStageEmitter.emit_per_stage(&dag).unwrap(); assert!(configs.contains_key(&StageId::Edge)); @@ -295,12 +385,14 @@ fn emitter_edge_config_has_correct_processor_kll() { #[test] fn emitter_edge_config_has_correct_processor_ddsketch() { - let expr = PhysicalExpr::estimate_over_agg( - EstimateOp::Quantile { q: 0.99 }, - SummaryKind::DDSketch, - SummaryParams::DDSketch { alpha: 0.01 }, - windowed_scan(), - ); + let expr = PhysicalExpr::committed(estimate_l4( + SketchQuery::Quantile { q: 0.99 }, + sketch_agg_l4( + SummaryKind::DDSketch, + SummaryParams::DDSketch { alpha: 0.01 }, + logical_l4(windowed_scan()), + ), + )); let dag = StageAllocator .allocate(&expr, Topology::ThreeStage) .unwrap(); @@ -330,7 +422,9 @@ fn emitter_backend_config_routes_aggregation_id() { assert_eq!(b.aggregations[0].sketch_kind, SummaryKind::Kll); assert_eq!(b.readouts.len(), 1); assert_eq!(b.readouts[0].aggregation_id, edge_aid); - assert_eq!(b.readouts[0].op, EstimateOp::Quantile { q: 0.99 }); + // `SketchQuery` has no `PartialEq` upstream — destructure + // instead of `assert_eq!`. + assert!(matches!(b.readouts[0].op, SketchQuery::Quantile { q } if q == 0.99)); } other => panic!("expected Backend config, got {other:?}"), } @@ -357,45 +451,43 @@ fn emitter_unsupported_topology_errors_cleanly() { fn end_to_end_quantile_workload() { // Two quantile queries (q=0.99, q=0.95) and one max — the §6 // batched example. After CSE they share Window+Scan; after sketch - // reuse they share one SketchAgg{KLL}; q3 (Max) takes a separate + // reuse they share one SummaryAgg{KLL}; q3 (Max) takes a separate // exact path. Phase C/B don't yet wire CSE through the typed path, // so this test models the post-rule structure by hand. // // Structure: - // Backend: SketchEstimate{q=0.99} SketchEstimate{q=0.95} + // Backend: SummaryEstimate{q=0.99} SummaryEstimate{q=0.95} // \ / // \ / - // Gateway: SketchMerge{KLL} (and another SketchMerge for q3) - // Edge: SketchAgg{KLL} SketchAgg{KLL} SketchAgg{KLL} + // Gateway: SummaryMerge{KLL} (and another SummaryMerge for q3) + // Edge: SummaryAgg{KLL} SummaryAgg{KLL} SummaryAgg{KLL} // (Window + Scan shared in real DAG; for the // test we materialise three Logical wrappers.) // // The test asserts the per-stage bucketing matches the design.md - // table (Edge: SketchAgg + Logical(Scan/Window/Aggregate{Max}); - // Gateway: SketchMerge + Merge; Backend: SketchEstimate + final + // table (Edge: SummaryAgg + Logical(Scan/Window/Aggregate{Max}); + // Gateway: SummaryMerge + Merge; Backend: SummaryEstimate + final // root). - let agg = || PhysicalExpr::SketchAgg { - sketch_type: SummaryKind::Kll, - params: SummaryParams::Kll { k: 200 }, - child: Box::new(PhysicalExpr::Logical(windowed_scan())), - }; - let merge_kll = PhysicalExpr::SketchMerge { - algebra: MergeAlgebra::Union, - children: vec![agg(), agg(), agg()], + let agg = || { + sketch_agg_l4( + SummaryKind::Kll, + SummaryParams::Kll { k: 200 }, + logical_l4(windowed_scan()), + ) }; - // Two SketchEstimate readouts hanging off the merge — the typed + let merge_kll = merge_l4(vec![agg(), agg(), agg()]); + // Two SummaryEstimate readouts hanging off the merge — the typed // PhysicalExpr is single-rooted, so we model the workload as the // higher of the two readouts (q=0.99) and assert the underlying // colouring is correct. The second readout (q=0.95) is exercised // by `allocator_let_binding_color_propagates` and the per-rule // tests above. - let q99 = PhysicalExpr::SketchEstimate { - op: EstimateOp::Quantile { q: 0.99 }, - child: Box::new(merge_kll), - }; - let dag = StageAllocator.allocate(&q99, Topology::ThreeStage).unwrap(); + let q99 = estimate_l4(SketchQuery::Quantile { q: 0.99 }, merge_kll); + let dag = StageAllocator + .allocate(&PhysicalExpr::committed(q99), Topology::ThreeStage) + .unwrap(); let configs = ThreeStageEmitter.emit_per_stage(&dag).unwrap(); - // Edge: 3 SketchAgg processors. + // Edge: 3 SummaryAgg processors. match configs.get(&StageId::Edge).unwrap() { StageConfig::Edge(e) => { assert_eq!(e.sketch_processors.len(), 3); @@ -414,20 +506,13 @@ fn end_to_end_quantile_workload() { } _ => unreachable!(), } - // Backend: one readout for q=0.99 + 3 aggregations (one per edge SketchAgg). + // Backend: one readout for q=0.99 + 3 aggregations (one per edge SummaryAgg). match configs.get(&StageId::Backend).unwrap() { StageConfig::Backend(b) => { assert_eq!(b.aggregations.len(), 3); assert_eq!(b.readouts.len(), 1); - assert_eq!(b.readouts[0].op, EstimateOp::Quantile { q: 0.99 }); + assert!(matches!(b.readouts[0].op, SketchQuery::Quantile { q } if q == 0.99)); } _ => unreachable!(), } } - -// Quiet the unused-import lint when AccuracyTarget is gated only by -// future workflow tests. -#[allow(dead_code)] -fn _force_accuracy_target_use() -> AccuracyTarget { - AccuracyTarget::Epsilon(0.01) -} diff --git a/control_plane/src/sketch_algebra/cost_model.rs b/control_plane/src/sketch_algebra/cost_model.rs new file mode 100644 index 00000000..89bae492 --- /dev/null +++ b/control_plane/src/sketch_algebra/cost_model.rs @@ -0,0 +1,391 @@ +//! `ControlPlaneCostModel` — plugs control_plane's own sketch-family +//! selection + parameter-sizing policy into +//! `asap_plan::bind::implement_tree_in_with` via the two `CostModel` +//! extension points (`rank_candidates` for family choice, `size_params` +//! for parameter sizing — the latter added by ASAPController PR #146 +//! specifically to support this migration). +//! +//! Ports the decisions previously made by the `bind_kll_quantile` / +//! `bind_ddsketch_quantile` / `bind_hll_cardinality` / `bind_cms_count` / +//! `bind_cms_topk` `Rule`s verbatim — same accuracy-bound citations, same +//! priority order, same recall-tier logic — just re-homed behind the +//! `CostModel` trait instead of a bespoke `Rule` dispatcher, so the L3→L4 +//! walk itself (schema derivation, `col`/`by` computation, DAG +//! construction) can be `asap_plan::bind`'s rather than a forked copy. +//! +//! Two shapes `implement_tree_in_with` cannot realize even with this +//! `CostModel` plugged in, because the decision of *whether* to call into +//! `rank_candidates`/`size_params` at all is made upstream, before the +//! `CostModel` is ever consulted: +//! +//! - `AggIntent::Extension` (control_plane's `Frequency` point-query) — +//! `boundary::implementation_for` maps every `Extension` to +//! `PassThrough` unconditionally, by design (core has no realization +//! opinion for a deployment-specific shape it doesn't know). +//! - `AggIntent::TopK { accuracy: AccuracyTarget::Exact, .. }` — routes to +//! `exact_realization`, which has no accumulator form for `TopK` and +//! also returns `PassThrough`, even though control_plane's own +//! `BindCountSketchOnTopK` still binds this shape (Tight recall tier → +//! `CountSketchWithHeap`). +//! +//! Both are intercepted in `lower.rs` *before* `implement_tree_in_with` +//! runs — see that module's `bind_recursive` for the pre-pass. + +#![allow(dead_code)] + +use asap_plan::CostModel; +use asap_sketch::{SummaryKind, SummaryParams}; + +use crate::intent_algebra::AggIntent; +use crate::optimizer::cost::wire::WireCostTable; +use crate::types_v2::AccuracyTarget; + +/// See module docs. +pub struct ControlPlaneCostModel { + /// Workload-level accuracy policy — combined (tighter-of) with each + /// intent's own accuracy field, matching every `bind_*.rs` rule's old + /// `accuracy: &AccuracyTarget` parameter. + pub workload_accuracy: AccuracyTarget, +} + +impl ControlPlaneCostModel { + pub fn new(workload_accuracy: AccuracyTarget) -> Self { + Self { workload_accuracy } + } + + /// The tighter (lower) of the workload policy and an intent's own + /// accuracy target, as `(eps, delta)`. `None` when either side is + /// `Exact` — mirrors `bind_kll_quantile.rs` / `bind_ddsketch_quantile.rs` + /// / `bind_hll_cardinality.rs` / `bind_cms_count.rs`'s identical + /// `match (accuracy, &intent_accuracy) {...}` block. (`TopK` has its + /// own combination rule — an `Exact` side there picks the *other* + /// side's budget rather than bailing — see [`Self::topk_eps_delta`].) + fn combined_eps_delta(&self, intent_accuracy: &AccuracyTarget) -> Option<(f64, f64)> { + match (&self.workload_accuracy, intent_accuracy) { + (AccuracyTarget::Exact, _) | (_, AccuracyTarget::Exact) => None, + (AccuracyTarget::Epsilon(a), AccuracyTarget::Epsilon(b)) => Some((a.min(*b), 0.01)), + ( + AccuracyTarget::Epsilon(a), + AccuracyTarget::EpsilonDelta { epsilon, delta }, + ) + | ( + AccuracyTarget::EpsilonDelta { epsilon, delta }, + AccuracyTarget::Epsilon(a), + ) => Some((a.min(*epsilon), *delta)), + ( + AccuracyTarget::EpsilonDelta { epsilon: a, delta: da }, + AccuracyTarget::EpsilonDelta { epsilon: b, delta: db }, + ) => Some((a.min(*b), da.min(*db))), + } + } + + /// `TopK`'s own `(eps, delta)` combination — verbatim port of + /// `bind_cms_topk.rs`'s `bind` match. Unlike + /// [`Self::combined_eps_delta`], an `Exact` side does not bail: it + /// picks the *other* side's budget (falling back to the catalog + /// default `(0.01, 0.01)` only when both sides are `Exact`). Public + /// (within the crate) because [`crate::sketch_algebra::lower`]'s + /// `TopK { accuracy: Exact }` pre-pass needs the same combination. + pub(crate) fn topk_eps_delta(&self, intent_accuracy: &AccuracyTarget) -> (f64, f64) { + match (&self.workload_accuracy, intent_accuracy) { + (AccuracyTarget::Exact, AccuracyTarget::Exact) => (0.01, 0.01), + (AccuracyTarget::Exact, other) | (other, AccuracyTarget::Exact) => match other { + AccuracyTarget::Epsilon(a) => (*a, 0.01), + AccuracyTarget::EpsilonDelta { epsilon, delta } => (*epsilon, *delta), + AccuracyTarget::Exact => (0.01, 0.01), + }, + (AccuracyTarget::Epsilon(a), AccuracyTarget::Epsilon(b)) => (a.min(*b), 0.01), + ( + AccuracyTarget::Epsilon(a), + AccuracyTarget::EpsilonDelta { epsilon, delta }, + ) + | ( + AccuracyTarget::EpsilonDelta { epsilon, delta }, + AccuracyTarget::Epsilon(a), + ) => (a.min(*epsilon), *delta), + ( + AccuracyTarget::EpsilonDelta { epsilon: a, delta: da }, + AccuracyTarget::EpsilonDelta { epsilon: b, delta: db }, + ) => (a.min(*b), da.min(*db)), + } + } + + /// Recall-tier-filtered, wire-cost-ordered top-k family candidates. + /// Verbatim port of `bind_cms_topk.rs`'s `TopkRecallTier` + + /// `candidate_families` + `cheapest_family`. Public (within the + /// crate) for the same reason as [`Self::topk_eps_delta`]. + pub(crate) fn topk_family_order(&self, intent_accuracy: &AccuracyTarget) -> Vec { + let tight = matches!(self.workload_accuracy, AccuracyTarget::Exact) + || matches!(intent_accuracy, AccuracyTarget::Exact); + let allowed: &[SummaryKind] = if tight { + &[SummaryKind::CountSketchWithHeap] + } else { + &[SummaryKind::CmsWithHeap, SummaryKind::CountSketchWithHeap] + }; + let table = WireCostTable::default(); + let mut ranked: Vec = allowed.to_vec(); + ranked.sort_by_key(|k| table.for_kind(k).per_flush()); + ranked + } + + /// `(width, depth)` for a CMS-family sketch under `(eps, delta)`. + /// Verbatim: `w = ⌈e/eps⌉` clamped to `≥2`, `d = ⌈ln(1/delta)⌉` + /// clamped to `≥1`. + fn cms_width_depth(eps: f64, delta: f64) -> (u32, u32) { + let w = (std::f64::consts::E / eps).ceil().max(2.0) as u32; + let d = (1.0 / delta).ln().ceil().max(1.0) as u32; + (w, d) + } +} + +/// The workload-level accuracy target combined with an intent's own — +/// pulls the intent's `accuracy` field out of whichever variant carries +/// one. `_` covers every non-approximate-capable variant, unreachable in +/// practice (`rank_candidates`/`size_params` are only ever called for +/// `Quantile`/`Cardinality`/`Count`/`TopK` — the shapes +/// `boundary::bind_summary_with` handles). +fn intent_accuracy(intent: &AggIntent) -> AccuracyTarget { + match intent { + AggIntent::Quantile { accuracy, .. } + | AggIntent::Cardinality { accuracy, .. } + | AggIntent::TopK { accuracy, .. } => accuracy.clone(), + AggIntent::Count { accuracy } => accuracy.clone(), + _ => AccuracyTarget::Exact, + } +} + +impl CostModel for ControlPlaneCostModel { + fn rank_candidates(&self, intent: &AggIntent, candidates: &[SummaryKind]) -> Vec { + match intent { + // bind_ddsketch_quantile (priority 6) always wins the old + // dispatcher's tie-break over bind_kll_quantile (priority 5) + // whenever both can bind (see bind_kll_quantile.rs's + // `priority()` doc — no eps-dependent split actually exists + // between the two rules today, both read the same combined + // eps). Pure static reorder: DDSketch before Kll. + AggIntent::Quantile { .. } => { + let mut v = candidates.to_vec(); + if let Some(pos) = v.iter().position(|k| *k == SummaryKind::DDSketch) { + let dd = v.remove(pos); + v.insert(0, dd); + } + v + } + AggIntent::TopK { .. } => self.topk_family_order(&intent_accuracy(intent)), + // Cardinality → Hll, Count → Cms: control_plane only ever + // binds one family for each; asap-plan's static order already + // puts it first (`summary_candidates`), nothing to reorder. + _ => candidates.to_vec(), + } + } + + fn size_params( + &self, + kind: SummaryKind, + intent: &AggIntent, + eps: f64, + delta: f64, + ) -> SummaryParams { + match intent { + AggIntent::TopK { k, .. } => { + let (eps, delta) = self.topk_eps_delta(&intent_accuracy(intent)); + let (w, d) = Self::cms_width_depth(eps, delta); + // CountSketch/CMS columns MUST be a power of two: the + // agent (asapedgeprocessor config_validate) rejects + // non-pow2 cols. Round up — this only tightens the + // additive bound (ε ≤ e/w). + let w = w.next_power_of_two(); + let heap_size = *k as u32; + match kind { + SummaryKind::CmsWithHeap => SummaryParams::CmsWithHeap { + width: w, + depth: d, + heap_size, + }, + _ => SummaryParams::CountSketchWithHeap { + width: w, + depth: d, + heap_size, + }, + } + } + _ => { + let Some((eps, delta)) = self.combined_eps_delta(&intent_accuracy(intent)) else { + // Either side Exact: unreachable in practice for + // Quantile/Cardinality/Count (an Exact intent never + // reaches `bind_summary_with` upstream — see + // `implementation_for_with`'s `Exact => exact_realization` + // arm), kept as a safe fallback rather than a panic. + return asap_plan::boundary::default_size_params(kind, intent, eps, delta); + }; + match kind { + SummaryKind::Kll => SummaryParams::Kll { + k: kll_k_for_eps(eps), + }, + SummaryKind::DDSketch if (0.0..1.0).contains(&eps) => { + SummaryParams::DDSketch { alpha: eps } + } + SummaryKind::Hll => SummaryParams::Hll { + precision: hll_precision_for_eps(eps), + }, + SummaryKind::Cms => { + let (w, d) = Self::cms_width_depth(eps, delta); + SummaryParams::Cms { width: w, depth: d } + } + other => asap_plan::boundary::default_size_params(other, intent, eps, delta), + } + } + } + } +} + +/// A `CostModel` that forces a single family for whichever intent it's +/// asked to rank, delegating parameter sizing to an inner +/// [`ControlPlaneCostModel`]. Used by `optimizer::rules::bind_workload_typed`, +/// which already has a definitive family pick from the capability matrix +/// (or a `sketch_type_override`) and just needs the matching binding, not +/// a fresh selection decision. +pub struct ForcedFamilyCostModel { + inner: ControlPlaneCostModel, + forced: SummaryKind, +} + +impl ForcedFamilyCostModel { + pub fn new(workload_accuracy: AccuracyTarget, forced: SummaryKind) -> Self { + Self { + inner: ControlPlaneCostModel::new(workload_accuracy), + forced, + } + } +} + +impl CostModel for ForcedFamilyCostModel { + fn rank_candidates(&self, _intent: &AggIntent, _candidates: &[SummaryKind]) -> Vec { + vec![self.forced.clone()] + } + + fn size_params( + &self, + kind: SummaryKind, + intent: &AggIntent, + eps: f64, + delta: f64, + ) -> SummaryParams { + self.inner.size_params(kind, intent, eps, delta) + } +} + +/// Map an ε rank-error budget to a KLL stream-size `k`. Verbatim port of +/// `bind_kll_quantile.rs::kll_k_for_eps` — power-of-two rungs (200, 400, +/// 800, 2048, 8192) so the in-tree `algebra::directory` continues to +/// recognise the parameter. +fn kll_k_for_eps(eps: f64) -> u32 { + if eps <= 0.0 { + return 8192; + } + if eps >= 0.01 { + 200 + } else if eps >= 0.005 { + 400 + } else if eps >= 0.0025 { + 800 + } else if eps >= 0.001 { + 2048 + } else { + 8192 + } +} + +/// Map an ε standard-error budget to the HLL `precision`. Verbatim port +/// of `bind_hll_cardinality.rs::hll_precision_for_eps`. +fn hll_precision_for_eps(eps: f64) -> u8 { + if eps <= 0.0 { + return 16; + } + if eps >= 0.03 { + 10 + } else if eps >= 0.015 { + 12 + } else if eps >= 0.008 { + 14 + } else { + 16 + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::intent_algebra::{default_cardinality, default_quantile}; + + fn eps(e: f64) -> AccuracyTarget { + AccuracyTarget::Epsilon(e) + } + + #[test] + fn quantile_always_prefers_ddsketch_over_kll() { + let model = ControlPlaneCostModel::new(AccuracyTarget::Epsilon(0.1)); + let ranked = model.rank_candidates(&default_quantile(0.99), &[SummaryKind::Kll, SummaryKind::DDSketch]); + assert_eq!(ranked, vec![SummaryKind::DDSketch, SummaryKind::Kll]); + } + + #[test] + fn kll_k_matches_bind_kll_quantile_rungs() { + let model = ControlPlaneCostModel::new(AccuracyTarget::Epsilon(1.0)); + let params = model.size_params(SummaryKind::Kll, &default_quantile(0.99), 0.01, 0.01); + assert_eq!(params, SummaryParams::Kll { k: 200 }); + + let model = ControlPlaneCostModel::new(AccuracyTarget::Epsilon(1.0)); + let tight = crate::intent_algebra::AggIntent::Quantile { + col: None, + q: 0.99, + accuracy: eps(0.001), + }; + let params = model.size_params(SummaryKind::Kll, &tight, 0.01, 0.01); + assert_eq!(params, SummaryParams::Kll { k: 2048 }); + } + + #[test] + fn hll_precision_matches_bind_hll_cardinality_rungs() { + let model = ControlPlaneCostModel::new(AccuracyTarget::Epsilon(1.0)); + let params = model.size_params(SummaryKind::Hll, &default_cardinality(), 0.01, 0.01); + assert_eq!(params, SummaryParams::Hll { precision: 14 }); + } + + #[test] + fn topk_tight_tier_forces_countsketch_even_when_ranked_from_cms() { + let model = ControlPlaneCostModel::new(AccuracyTarget::Exact); + let intent = crate::intent_algebra::AggIntent::TopK { + k: 5, + accuracy: eps(0.01), + }; + let ranked = model.rank_candidates(&intent, &[SummaryKind::CmsWithHeap, SummaryKind::CountSketchWithHeap]); + assert_eq!(ranked, vec![SummaryKind::CountSketchWithHeap]); + } + + #[test] + fn topk_loose_tier_prefers_cheaper_cms_heap() { + let model = ControlPlaneCostModel::new(AccuracyTarget::Epsilon(0.1)); + let intent = crate::intent_algebra::AggIntent::TopK { + k: 5, + accuracy: eps(0.01), + }; + let ranked = model.rank_candidates(&intent, &[SummaryKind::CmsWithHeap, SummaryKind::CountSketchWithHeap]); + assert_eq!(ranked[0], SummaryKind::CmsWithHeap); + } + + #[test] + fn topk_width_is_rounded_up_to_a_power_of_two() { + let model = ControlPlaneCostModel::new(AccuracyTarget::Epsilon(0.1)); + let intent = crate::intent_algebra::AggIntent::TopK { + k: 5, + accuracy: eps(0.01), + }; + let params = model.size_params(SummaryKind::CmsWithHeap, &intent, 0.0, 0.0); + let SummaryParams::CmsWithHeap { width, heap_size, .. } = params else { + panic!("expected CmsWithHeap params"); + }; + assert!(width.is_power_of_two()); + assert_eq!(heap_size, 5); + } +} diff --git a/control_plane/src/sketch_algebra/lower.rs b/control_plane/src/sketch_algebra/lower.rs index c8f6b9de..bb98bb09 100644 --- a/control_plane/src/sketch_algebra/lower.rs +++ b/control_plane/src/sketch_algebra/lower.rs @@ -1,87 +1,80 @@ -//! L3 → L4 lowering — `QueryExpr` walk that fires `Bind*` rules. +//! L3 → L4/L5 lowering — `QueryExpr` walk that adopts +//! `asap_plan::bind::implement_tree_in_with` for the sketch algebra +//! itself (Step B of the plan-shaped-serving migration), with +//! `crate::sketch_algebra::cost_model::ControlPlaneCostModel` plugged in +//! for family selection + parameter sizing. //! -//! Per `control_plane/docs/design.md` §6 sketch_algebra (line ~616): "the -//! optimizer's job is to selectively replace logical aggregates / joins -//! with their sketch-bound variants when a binding rule fires; everything -//! else stays inside `PhysicalExpr::Logical(…)`." +//! Per `control_plane/docs/design.md` §6: "the optimizer's job is to +//! selectively replace logical aggregates / joins with their sketch-bound +//! variants when a binding rule fires; everything else stays inside +//! `Logical(…)`." //! -//! Phase C ships the bottom-up walk — every `QueryExpr` sub-tree is -//! offered to the rule dispatcher; if a rule fires, its output replaces -//! the sub-tree; otherwise we recurse into the children and wrap the -//! result in `PhysicalExpr::Logical`. -//! -//! `LetBinding` / `Ref` survive the lowering: the bound expression is -//! lowered to L4, the child is lowered against the same workload-level -//! accuracy target, and the result is a `PhysicalExpr::LetBinding` / -//! `PhysicalExpr::Ref` with the L4-bound payload. +//! Two node shapes are rewritten *before* delegating to +//! `implement_tree_in_with`, because `asap_plan::boundary::implementation_for` +//! actively binds them to an `Implementation` this deployment's data plane +//! doesn't (or, deliberately, shouldn't) serve — not something the +//! `CostModel` hook can reach, since the decision of *whether* to call +//! into `rank_candidates`/`size_params` at all is made before the +//! `CostModel` is ever consulted. See each helper's docs for the specific +//! reason. Everything else — including `AggIntent::Extension` (the +//! `Frequency` point-query) and `AggIntent::TopK { accuracy: Exact }`, +//! both of which `implementation_for` maps to `PassThrough` — is left to +//! fall through to `implement_tree_in_with`'s own `Logical` fallback +//! unchanged: both are genuine `asap-plan` coverage gaps, not something +//! this deployment can or should route around locally (filed upstream — +//! see ASAPController#150, #151). #![allow(dead_code)] +use std::rc::Rc; + +use asap_plan::bind::implement_tree_in_with; use thiserror::Error; -use crate::intent_algebra::QueryExpr; -use crate::sketch_algebra::physical_expr::PhysicalExpr; -use crate::sketch_algebra::rules::dispatch; -use crate::types_v2::AccuracyTarget; +use crate::intent_algebra::{AggIntent, BindingScope, QueryExpr}; +use crate::sketch_algebra::cost_model::ControlPlaneCostModel; +use crate::sketch_algebra::physical_expr::{L4Plan, PhysicalExpr}; +use crate::types_v2::{AccuracyTarget, BindingName}; -/// Errors surfaced by the `bind_query_expr` lowering. Reserved — Phase C -/// has no bind-time errors that aren't expressible as "no rule fires" -/// (the dispatcher returns `None` and the caller wraps the input in -/// `PhysicalExpr::Logical`). Defined now so future rules that *can* fail -/// at bind time (catalog mismatch, parameter overflow) plug in without -/// an API break. +/// Errors surfaced by the `bind_query_expr` lowering. #[derive(Debug, Error)] pub enum BindingError { - /// Carried for downstream consumers — Phase C has no producers yet. - #[error("binding failed: {0}")] - Other(String), + /// L3 schema derivation failed while lifting an edge to `L4Schema` — + /// forwarded from `asap_plan::bind`. + #[error("L3->L4 implementation failed: {0}")] + Implement(#[from] asap_plan::ImplementError), } -/// Lower an L3 `QueryExpr` to an L4 [`PhysicalExpr`] under the supplied -/// workload-level accuracy target. Bottom-up walk; `Bind*` rules consult -/// the `accuracy` param + the per-intent `accuracy` field on each -/// `Aggregate` and pick the tighter of the two. -/// -/// Return value: `Ok(PhysicalExpr)` always — Phase C never errors. The -/// caller observes "no binding" via the returned `PhysicalExpr::Logical` -/// at the matched sub-tree position. +/// Lower an L3 `QueryExpr` to L4/L5 under the supplied workload-level +/// accuracy target. The result is always [`PhysicalExpr::Committed`] — +/// this walk never picks a Phase ε.1 backend/archive placement; that's a +/// separate, later L5 decision (`optimizer::cost::wire`). pub fn bind_query_expr( expr: &QueryExpr, accuracy: AccuracyTarget, ) -> Result { - Ok(bind_recursive(expr, &accuracy)) + Ok(PhysicalExpr::Committed(bind_recursive(expr, &accuracy)?)) } -fn bind_recursive(expr: &QueryExpr, accuracy: &AccuracyTarget) -> PhysicalExpr { - // Try the rule dispatcher first — if a `Bind*` rule fires, its - // output replaces the matched sub-tree wholesale. The rule's output - // already wraps the L3 child in `PhysicalExpr::Logical(...)` per the - // `estimate_over_agg` constructor. - if let Some(bound) = dispatch(expr, accuracy) { - return bound; - } - - // No rule matched — recurse into the children to find sub-trees that - // bind. For pass-through nodes (`Scan`, `Window`, `LetBinding`, - // `Ref`) we surface the recursive structure in `PhysicalExpr` directly - // when relevant, otherwise we wrap the L3 sub-tree in `Logical`. +fn bind_recursive(expr: &QueryExpr, accuracy: &AccuracyTarget) -> Result { match expr { - QueryExpr::LetBinding { name, expr, child } => PhysicalExpr::LetBinding { - name: crate::types_v2::BindingName::new(name.as_str()), - expr: Box::new(bind_recursive(expr, accuracy)), - child: Box::new(bind_recursive(child, accuracy)), - }, - QueryExpr::Ref { name } => PhysicalExpr::Ref { - name: crate::types_v2::BindingName::new(name.as_str()), - }, + QueryExpr::LetBinding { name, expr, child } => Ok(L4Plan::LetBinding { + name: BindingName::new(name.as_str()), + expr: Rc::new(bind_recursive(expr, accuracy)?), + child: Rc::new(bind_recursive(child, accuracy)?), + }), + QueryExpr::Ref { name } => Ok(L4Plan::Ref { + name: BindingName::new(name.as_str()), + }), + // The canonical L3 IR places `Window` *above* a single-statistic - // sketchable `Aggregate` (`lower`'s window-swap). - // The `Bind*` rules match `Aggregate` with the window as its - // *child*, so push the window down under the aggregate and - // re-dispatch — the window then rides along inside the bound - // node's `Logical(...)` child, exactly as it did when the - // aggregate sat on top. A `Window` over anything else stays a - // logical pass-through. + // sketchable `Aggregate` (`lower`'s window-swap). `implement_tree_in_with` + // only recurses through the `Aggregate` spine (see its module + // docs' "conservative fallbacks" — a logical parent above a + // bindable aggregate subsumes it unbound), so push the window + // down under the aggregate and re-dispatch, exactly as the old + // hand-written walk did — the window then rides along inside the + // bound node's `Logical(...)` child. QueryExpr::Window { kind, size, @@ -112,18 +105,79 @@ fn bind_recursive(expr: &QueryExpr, accuracy: &AccuracyTarget) -> PhysicalExpr { }; bind_recursive(&pushed, accuracy) } - // For `Aggregate`, the rule dispatcher already had a chance and - // declined. For `Scan` and a `Window` over a non-`Aggregate` - // child, no binding rule applies — wrap the L3 sub-tree as a - // logical pass-through. - QueryExpr::Aggregate { .. } | QueryExpr::Scan { .. } | QueryExpr::Window { .. } => { - PhysicalExpr::Logical(expr.clone()) + + // `AggIntent::Count { accuracy: Exact }` — `boundary::implementation_for`'s + // `exact_realization` actively binds this to `SummaryKind::Count`, + // but this deployment's data plane has no count accumulator: its + // `SumAccumulator` only tracks `sum: f64`, so a `Count` + // accumulator would silently return the sum of sample VALUES, not + // the sample count (PR #200/#201, reverted — see the retired + // `bind_exact_agg.rs`). Force the same fallback `implement_tree_in_with` + // uses for unbound shapes, via the public `bind::logical` + // ASAPController exposes for exactly this "deployment knows + // better" case — no local schema-lift duplication needed. Stays + // on archive, matching today's behavior. + QueryExpr::Aggregate { + aggs, having: None, .. + } if matches!( + aggs.as_slice(), + [AggIntent::Count { + accuracy: AccuracyTarget::Exact + }] + ) => + { + Ok(L4Plan::Summary(asap_plan::bind::logical( + expr, + &BindingScope::default(), + )?)) + } + + _ => { + let rewritten = rewrite_rate_to_increase(expr); + let cost_model = ControlPlaneCostModel::new(accuracy.clone()); + let node = implement_tree_in_with(&rewritten, &BindingScope::default(), &cost_model)?; + Ok(L4Plan::Summary(node)) } - // A-variants lifted in Batch 2 of the relational migration. No - // sketch binding rule applies to these shapes today — wrap as a - // logical pass-through, matching the policy for `Aggregate` / - // `Scan` / `Window`. Rule extensions can specialise individual - // variants as the catalog grows. - _ => PhysicalExpr::Logical(expr.clone()), + } +} + +/// Rewrite every `AggIntent::Rate` reachable via the `Aggregate` spine +/// (nested `Aggregate.child` chains — the only shape `implement_tree_in_with` +/// itself recurses through; see its "conservative fallbacks" docs) to +/// `AggIntent::Increase`. +/// +/// `boundary::implementation_for` gives `Rate` its own `SummaryKind::Rate`; +/// this deployment's data plane has no accumulator family for it — rate is +/// computed as `increase / window_seconds`, a scalar division on the +/// `Increase` accumulator's output applied at readout, not a separate +/// accumulator (see the retired `bind_exact_agg.rs`, which bound both to +/// the same accumulator for the same reason). Representing `Rate` as +/// `Increase` up through L4 preserves that — the L5 emitter is still the +/// one that knows to apply the division. +fn rewrite_rate_to_increase(expr: &QueryExpr) -> QueryExpr { + match expr { + QueryExpr::Aggregate { + by, + aggs, + output_names, + having, + child, + } => QueryExpr::Aggregate { + by: by.clone(), + aggs: aggs + .iter() + .map(|intent| { + if matches!(intent, AggIntent::Rate) { + AggIntent::Increase + } else { + intent.clone() + } + }) + .collect(), + output_names: output_names.clone(), + having: having.clone(), + child: Box::new(rewrite_rate_to_increase(child)), + }, + other => other.clone(), } } diff --git a/control_plane/src/sketch_algebra/mod.rs b/control_plane/src/sketch_algebra/mod.rs index 88b13db9..c58dd754 100644 --- a/control_plane/src/sketch_algebra/mod.rs +++ b/control_plane/src/sketch_algebra/mod.rs @@ -32,6 +32,7 @@ pub mod capability; pub mod capability_matching; +pub mod cost_model; pub mod lower; pub mod matcher; pub mod physical_expr; @@ -47,4 +48,4 @@ pub use capability_matching::{ }; pub use lower::{bind_query_expr, BindingError}; pub use matcher::SummaryFamilyMatcher; -pub use physical_expr::{EstimateOp, MergeAlgebra, PhysicalExpr}; +pub use physical_expr::{L4Plan, PhysicalExpr}; diff --git a/control_plane/src/sketch_algebra/physical_expr.rs b/control_plane/src/sketch_algebra/physical_expr.rs index 7d072124..ad0e6059 100644 --- a/control_plane/src/sketch_algebra/physical_expr.rs +++ b/control_plane/src/sketch_algebra/physical_expr.rs @@ -1,130 +1,72 @@ -//! Layer 4 IR — `PhysicalExpr` DAG. +//! Layer 4/5 IR — `L4Plan` / `PhysicalExpr`. //! -//! Per `control_plane/docs/design.md` §6 "`core::sketch_algebra` — Layer 4 IR -//! (`PhysicalExpr`)" (around line ~565). +//! Per `control_plane/docs/design.md` §6 "`core::sketch_algebra` — Layer 4 IR" +//! and the L4/L5 layer-spine invariant: "sketch binding is already +//! committed by L4; L5 is about stage allocation + emission." //! -//! Two-IR split: L3 [`crate::intent_algebra::QueryExpr`] is intent-only; -//! L4 [`PhysicalExpr`] is sketch-bound. `Bind*` rules consume the L3 IR -//! and produce the L4 IR with the sketch family + parameters committed. +//! Step B of the plan-shaped-serving migration retires this crate's own +//! `PhysicalExpr`-as-L4-algebra (the old `Logical` / `SketchAgg` / +//! `SketchEstimate` / `SketchMerge` / `ExactAgg` variants) in favor of +//! ASAPController's canonical L4 IR, `asap_sketch::{SummaryExpr, L4Node}` +//! — the same move Step 3 of the enum-unification made for +//! `SketchKind → SummaryKind`, one layer up. `implement_promql_for_asap_tier` +//! (`asap_tier_implement.rs`, Step A) already builds `Rc` trees via +//! `asap_plan::bind::implement_tree_in_with`; this module gives the rest of +//! the crate (optimizer, physical, emit) the same IR shape. //! -//! The variant set ships the subset DC + PromQL needs (the orchestrator's -//! current scope-reduction). `SketchJoin`, `SketchSubtract`, `SketchDelete` -//! from design.md §6 are intentionally *not* surfaced yet — they're -//! gated on rules that haven't landed (no `Bind*OnJoin`, no subtract / -//! delete consumer) and the orchestrator's spec restricts Phase C to -//! `SketchAgg / SketchEstimate / SketchMerge / Logical`. Adding them -//! later is a purely additive enum extension. +//! Two things `asap_sketch::L4Node` genuinely doesn't have, kept here: +//! +//! - **`LetBinding` / `Ref`** — named fan-in sharing (SQL +//! `WITH name AS (expr) ...` / a `SketchAgg` shared by two `SketchEstimate` +//! readouts). `L4Node`'s own DAG sharing is structural (multiple `Rc` +//! references to the same node), not named — but the rule-firing walk in +//! this crate discovers sharing incrementally, per-node, so it still needs +//! a name to thread a bound value across sibling calls. This is a +//! deployment-specific mechanism, not a fact about the sketch algebra +//! itself — L4 concern (it's still "what to compute", just with sharing), +//! hence `L4Plan` rather than `PhysicalExpr`. +//! - **`RawAtEdgeSketchAtBackend` / `RawAtEdgePrometheusArchive`** — Phase +//! ε.1's placement decisions (where the sketch gets built, not what it +//! is). Genuinely L5 — see `optimizer::cost::wire::BindMode`, which names +//! the same three modes. +//! +//! `SketchAgg` / `SketchEstimate` / `SketchMerge` / `Logical` / `ExactAgg` +//! don't get their own variants anymore — `asap_sketch::SummaryExpr` +//! already unifies all of them (including "exact accumulator" and "sketch" +//! as the same `SummaryAgg` node, with or without a wrapping +//! `SummaryEstimate`) inside a single `L4Plan::Summary(Rc)`. #![allow(dead_code)] -use asap_sketch::{SummaryKind, SummaryParams}; -use serde::{Deserialize, Serialize}; - -use crate::intent_algebra::QueryExpr; -use crate::types_v2::BindingName; -use asap_types::AggregationType; - -/// Readout operation extracted from a built sketch state. Inverse of -/// `SketchAgg`. Mirrors design.md §6 line ~607 — `SketchEstimate` plus -/// the readout `query` that says what to extract from the state. -/// -/// PromQL convention: a `Quantile` op carries the φ; a `PointCount` op -/// carries the key; `Cardinality` and `TopK` need no payload beyond the -/// ones already on the producing `SketchAgg` (cardinality ops are -/// parameter-free; the TopK `k` rides on the `SketchAgg` for -/// CountSketch-with-heap). -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -#[serde(tag = "op", rename_all = "snake_case")] -pub enum EstimateOp { - /// φ-th quantile readout — KLL / DDSketch / t-digest input. - Quantile { q: f64 }, - /// Approximate cardinality (count-distinct) — HLL / theta-sketch input. - Cardinality, - /// Approximate point count for a key — CMS input. - PointCount { key: String }, - /// Heavy-hitter top-k extraction — CountSketch-with-heap / Misra-Gries. - TopK { k: usize }, -} - -/// Algebra of a `SketchMerge` node — at L4 today this is always a union -/// of mergeable sketch states. The enum-shape is forward-compatible with -/// future merge algebras (weighted union for sampling sketches, etc.). -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum MergeAlgebra { - /// Set-union of two or more sketch states — `KLL ∪ KLL`, `HLL ∪ HLL`, - /// etc. The catalog `mergeable` flag must be true on all inputs. - Union, -} - -/// L4 algebra node. See module doc for the variant subset rationale. -/// -/// Not `Serialize`/`Deserialize` — `asap_sketch::SummaryKind`/`SummaryParams` -/// (carried by `SketchAgg`/`RawAtEdgeSketchAtBackend`) have no serde impl -/// (nothing in ASAPController needs one; see -/// `scratchpad/artifacts/enum-unification-plan.md` §4 — the old -/// `sketch_algebra::SketchKind`/`SketchParams` this replaces derived -/// `Serialize`/`Deserialize` too, but nothing on the real emit path ever -/// exercised it — every actual wire payload goes through a hand-written -/// JSON/YAML builder, never a whole-struct serialize). Previously tagged -/// `"sketch_node"` (not `"node"`, to avoid colliding with the L3 -/// `QueryExpr`'s `"node"` tag when nested via `PhysicalExpr::Logical`); -/// that tag is dropped along with the derive. -#[derive(Debug, Clone, PartialEq)] -pub enum PhysicalExpr { - /// Logical pass-through: an L3 node that no L4 rule rewrote. A - /// `Filter`, a row-shaped `Aggregate{Sum}`, or any other operator - /// whose semantics are unchanged by the sketch-binding pass lives - /// here unchanged. - Logical(QueryExpr), +use std::rc::Rc; +use std::time::Duration; - /// Sketch aggregation — produces a sketch state on its output edge. - /// The L3 `AggIntent` was lowered by a `Bind*` rule into the - /// committed `(sketch_type, params)` pair. - SketchAgg { - /// Sketch family (KLL / DDSketch / HLL / CMS / CountSketch). - sketch_type: SummaryKind, - /// Sketch parameters (validated by the catalog at bind time). - params: SummaryParams, - /// Input sub-tree — typically `Logical(Window{...})` or - /// `Logical(Scan{...})`. - child: Box, - }, +use asap_sketch::{L4Node, SummaryKind, SummaryParams}; - /// Read out a query result from a built sketch state. Inverse of - /// `SketchAgg`. The `op` says what to extract — a quantile φ, the - /// approximate cardinality, the top-k heavy hitters. - SketchEstimate { - /// Readout operation (see [`EstimateOp`]). - op: EstimateOp, - /// Sketch-state-bearing sub-tree (a `SketchAgg`, a `SketchMerge`, - /// or a `Ref` to one). - child: Box, - }, +use crate::types_v2::BindingName; - /// Merge multiple sketches into one — set-union under - /// [`MergeAlgebra::Union`]. The L4 type checker rejects mismatched - /// families / params at plan time (design.md §6.4 invariant 1). - SketchMerge { - /// Merge algebra — currently always `Union`. - algebra: MergeAlgebra, - /// Sketch-state-bearing inputs. All must agree on `(kind, params)`. - children: Vec, - }, +/// L4 IR — "what to compute". Wraps `asap_sketch::L4Node` (the sketch +/// algebra itself, owned upstream) and adds only the named-binding sharing +/// mechanism `asap_sketch` doesn't have. See module docs. +#[derive(Debug, Clone)] +pub enum L4Plan { + /// A committed L4 sub-tree — `SummaryAgg` / `SummaryEstimate` / + /// `SummaryMerge` / `Logical`, whatever `implement_tree_in_with` (or a + /// deployment-specific pre-pass) produced. + Summary(Rc), /// SQL `WITH name AS (expr) SELECT ... FROM name` / sketch-state - /// fan-in: name a sub-expression so multiple parents can reference - /// it. `PhysicalExpr::LetBinding` carries the two-tier fan-in described - /// in design.md §1339 — outer let names a `Window` output, inner let - /// names a `SketchAgg{KLL}` shared by two `SketchEstimate` parents - /// reading different quantiles. + /// fan-in: name a sub-expression so multiple parents can reference it. + /// Carries the two-tier fan-in described in design.md §1339 — outer + /// let names a `Window` output, inner let names a `SummaryAgg{Kll}` + /// shared by two `SummaryEstimate` parents reading different quantiles. LetBinding { /// Binding name; must be unique within the surrounding scope. name: BindingName, /// Bound sub-expression. - expr: Box, + expr: Rc, /// In-scope sub-tree — references the binding via `Ref`. - child: Box, + child: Rc, }, /// Reference a `LetBinding` by name. Resolution is lexical (scope @@ -133,31 +75,36 @@ pub enum PhysicalExpr { /// Bound name. name: BindingName, }, +} + +/// L5 IR — "where/how". Wraps an already-committed [`L4Plan`] (sketch +/// binding is final by the time anything reaches here) with placement +/// info: build at the edge (the common case — `Committed` needs no extra +/// annotation since the `L4Plan` itself is the whole story), or one of +/// Phase ε.1's two backend/archive placements. +#[derive(Debug, Clone)] +pub enum PhysicalExpr { + /// Sketch built at the edge — the default placement. The committed + /// `L4Plan` alone determines the output. + Committed(L4Plan), - // ── Phase ε.1: three-mode placement variants ──────────────────────── - // - // Phase ε.1 collapses the planner's raw-vs-sketch + edge-vs-backend - // axes into a single tri-mode selector. The two new variants name - // the two new placements; the existing `SketchAgg` corresponds to - // Mode 1 (sketch at edge). See `planner::wire_cost::BindMode`. - /// Mode 2 (Phase ε.1): no sketch processor at the edge — raw OTLP + /// Phase ε.1 Mode 2: no sketch processor at the edge — raw OTLP /// forwards to the backend, which builds the sketch at ingest. The /// `family` and `params` are the sketch the backend will build, so /// the backend's `StreamingConfig` `aggregation_input` is `raw` for - /// this metric (Phase ε.2 implements the raw-input ingest path). + /// this metric. RawAtEdgeSketchAtBackend { /// Sketch family the backend will build at ingest. family: SummaryKind, /// Sketch parameters (validated by the catalog at bind time). params: SummaryParams, - /// Input sub-tree — typically `Logical(Window{...})` or - /// `Logical(Scan{...})`. Mirrors `SketchAgg`'s child field so the - /// L5 emitter's walk uniform. - child: Box, + /// Input sub-tree — typically `Summary(Logical(Window{...}))` or + /// `Summary(Logical(Scan{...}))`. + child: Box, }, - /// Mode 3 (Phase ε.1): no sketch processor at the edge — raw OTLP - /// ships directly to Prometheus's native OTLP receiver at + /// Phase ε.1 Mode 3: no sketch processor at the edge — raw OTLP ships + /// directly to Prometheus's native OTLP receiver at /// `/api/v1/otlp/v1/metrics`. The backend HTTP-forwards queries to /// Prometheus's `/api/v1/query` endpoint (the `prometheus_remote` /// engine). Accuracy is exact (ε = 0) — Prometheus owns the raw @@ -174,7 +121,7 @@ pub enum PhysicalExpr { /// metric into windowed scrape data. Prometheus stores the raw /// stream regardless; the field is informational for the L5 /// emitter so it can size scrape intervals consistently. - window: Option, + window: Option, /// Label projection — labels promoted from OTLP resource /// attributes by Prometheus's /// `otlp.promote_resource_attributes` config. Default @@ -182,66 +129,13 @@ pub enum PhysicalExpr { /// — see `deploy/configs/prometheus-otlp-receiver.yml`. label_proj: Vec, }, - - /// Exact-aggregation node — produces the exact aggregation result - /// (Sum / Count-as-Sum / Increase / MinMax / …) directly. The L4 - /// counterpart to the data-plane `AggPayload::ExactAgg` shape: - /// state at the ASAP-tier sid is a typed accumulator (not a - /// sketch byte buffer), and there's no separate readout step — - /// the accumulator's value IS the answer. - /// - /// Distinct from `SketchAgg` + `SketchEstimate` in two ways: - /// 1. No `EstimateOp` wrapper. ExactAgg is its own answer. - /// 2. No `SummaryParams`. The `AggregationType` enum captures the - /// parameterization (DDSketch's α etc. don't apply — exact - /// aggregations are parameter-free up to the accumulator - /// family choice). - /// - /// Emitted by `bind_exact_agg` for `AggIntent::Sum` / - /// `AggIntent::Rate` / `AggIntent::Increase` / - /// `AggIntent::Count{Exact}` once the analyzer flips them (PR 6 - /// follow-up). Until that PR, the variant existed dormant in the - /// data plane's `AggKind::ExactAgg`; this brings the L4 algebra - /// in line. - ExactAgg { - /// Which exact-aggregation family (Sum / Increase / MinMax / - /// SetAggregator / DeltaSetAggregator / HLL — the same enum - /// the data plane keys on at `AggKind::ExactAgg.agg_type`). - agg_type: AggregationType, - /// Input sub-tree — typically `Logical(Window{...})` or - /// `Logical(Scan{...})`. Same shape as `SketchAgg::child`. - child: Box, - }, } impl PhysicalExpr { - /// Convenience constructor for the canonical - /// `SketchEstimate{SketchAgg{Logical(qe)}}` shape produced by every - /// `Bind*` rule. Keeps rule call sites short. - pub fn estimate_over_agg( - op: EstimateOp, - sketch_type: SummaryKind, - params: SummaryParams, - logical: QueryExpr, - ) -> Self { - PhysicalExpr::SketchEstimate { - op, - child: Box::new(PhysicalExpr::SketchAgg { - sketch_type, - params, - child: Box::new(PhysicalExpr::Logical(logical)), - }), - } - } - - /// Convenience constructor for `ExactAgg{Logical(qe)}` — the exact - /// counterpart to [`Self::estimate_over_agg`]. No estimate wrapper - /// because ExactAgg produces the answer directly. - pub fn exact_agg_over_logical(agg_type: AggregationType, logical: QueryExpr) -> Self { - PhysicalExpr::ExactAgg { - agg_type, - child: Box::new(PhysicalExpr::Logical(logical)), - } + /// Convenience constructor for the common case: a committed + /// `L4Node` sketch-built at the edge, no placement wrapper. + pub fn committed(node: Rc) -> Self { + PhysicalExpr::Committed(L4Plan::Summary(node)) } } @@ -304,30 +198,38 @@ mod tests { } #[test] - fn estimate_over_agg_ctor_shape() { - let e = PhysicalExpr::estimate_over_agg( - EstimateOp::Quantile { q: 0.99 }, - SummaryKind::Kll, - SummaryParams::Kll { k: 200 }, - windowed_scan(), - ); + fn committed_wraps_an_implement_tree_result() { + let q = QueryExpr::Aggregate { + by: crate::intent_algebra::GroupKeys::none(), + aggs: vec![crate::intent_algebra::AggIntent::Quantile { + col: None, + q: 0.99, + accuracy: crate::types_v2::AccuracyTarget::Epsilon(0.01), + }], + output_names: Vec::new(), + having: None, + child: Box::new(windowed_scan()), + }; + let node = asap_plan::bind::implement_tree(&q).expect("implements"); + let e = PhysicalExpr::committed(node); match e { - PhysicalExpr::SketchEstimate { op, child } => { - assert_eq!(op, EstimateOp::Quantile { q: 0.99 }); - match *child { - PhysicalExpr::SketchAgg { - sketch_type, - params, - child, - } => { - assert_eq!(sketch_type, SummaryKind::Kll); - assert_eq!(params, SummaryParams::Kll { k: 200 }); - assert!(matches!(*child, PhysicalExpr::Logical(_))); + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + asap_sketch::SummaryExpr::SummaryEstimate { query, sketch_input } => { + assert!(matches!(query, asap_sketch::SketchQuery::Quantile { q } if *q == 0.99)); + match &sketch_input.expr { + asap_sketch::SummaryExpr::SummaryAgg { + sketch, params, child, .. + } => { + assert_eq!(sketch, &SummaryKind::Kll); + assert_eq!(params, &SummaryParams::Kll { k: 200 }); + assert!(matches!(child.expr, asap_sketch::SummaryExpr::Logical(_))); + } + other => panic!("expected SummaryAgg, got {other:?}"), } - other => panic!("expected SketchAgg, got {other:?}"), } - } - other => panic!("expected SketchEstimate, got {other:?}"), + other => panic!("expected SummaryEstimate, got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } } diff --git a/control_plane/src/sketch_algebra/rules/bind_archive_only.rs b/control_plane/src/sketch_algebra/rules/bind_archive_only.rs deleted file mode 100644 index c03d14aa..00000000 --- a/control_plane/src/sketch_algebra/rules/bind_archive_only.rs +++ /dev/null @@ -1,223 +0,0 @@ -//! `BindArchiveOnly` — route Phase β archive-only `AggIntent`s to the -//! cold tier. -//! -//! This rule is the L4 catch for [`AggIntent`]s that don't have a warm- -//! tier streaming sketch family today (`Absent`, `Present`, `Delta`, -//! `Deriv`, `PredictLinear`, `HoltWinters`, `Idelta`, `Irate`, `Resets`, -//! `Changes`). It matches a single-intent -//! `Aggregate` carrying any of those, and emits an -//! [`PhysicalExpr::Logical`] pass-through. The L5 emitter looks at the -//! enclosed [`AggIntent::archive_only`] flag and routes the corresponding -//! StreamingConfig entry to the archive (Gorilla / Thanos) tier rather -//! than the warm sketch tier. -//! -//! Why a rule rather than the recursive walker's default? -//! `lower::bind_recursive` already wraps unmatched `Aggregate` in -//! `PhysicalExpr::Logical`, but that branch fires on EVERY unmatched -//! aggregate — including intents the planner is still trying to bind -//! (an `Aggregate{Sum}` over a tabular leaf, etc.). Surfacing the -//! archive-only cases through an explicit named rule lets the -//! StreamingConfig emitter and Phase α routing emit distinguish "no -//! ASAP-tier rule fired but the intent IS warm-eligible" from "this -//! intent is intentionally archive-only, route it cold". -//! -//! Reference: `control_plane/docs/design.md` §6 line ~689 ("the optimizer -//! framework") and Phase β orchestrator scope. - -#![allow(dead_code)] - -use crate::intent_algebra::{AggIntent, QueryExpr}; -use crate::sketch_algebra::physical_expr::PhysicalExpr; -use crate::sketch_algebra::rules::Rule; -use crate::types_v2::AccuracyTarget; - -/// Route `Aggregate{}` to a `Logical` pass-through. -/// Phase α's emitter consults `AggIntent::archive_only()` to flag the -/// resulting StreamingConfig entry for the cold tier. -pub struct BindArchiveOnly; - -impl Rule for BindArchiveOnly { - fn name(&self) -> &'static str { - "bind_archive_only" - } - - fn priority(&self) -> u16 { - // Lowest priority — every ASAP-tier rule should out-rank this - // one so the only path to BindArchiveOnly is "no warm rule - // fired AND the intent is archive-only". - 1 - } - - fn apply(&self, expr: &QueryExpr, _accuracy: &AccuracyTarget) -> Option { - match expr { - QueryExpr::Aggregate { aggs, .. } => { - // Single-intent Aggregate is the canonical Phase β shape; - // multi-intent fans out to per-intent rules elsewhere. - if aggs.len() == 1 && crate::intent_algebra::archive_only(&aggs[0]) { - Some(PhysicalExpr::Logical(expr.clone())) - } else { - None - } - } - _ => None, - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::intent_algebra::schema::{Column, DataType}; - use crate::intent_algebra::{LabelFilter, Schema, Source, WindowKind}; - use std::time::Duration; - - fn ts_scan() -> QueryExpr { - let schema = Schema::with_time_index( - vec![ - Column { - name: "ts".into(), - dtype: DataType::Timestamp, - nullable: false, - table: None, - }, - Column { - name: "service".into(), - dtype: DataType::Utf8, - nullable: false, - table: None, - }, - Column { - name: "value".into(), - dtype: DataType::Float64, - nullable: false, - table: None, - }, - ], - 0, - vec![vec![0, 1]], - ); - let lf = LabelFilter { - label: "service".into(), - equals: "api".into(), - }; - let pred = crate::intent_algebra::label_filter_to_predicate(&lf, &schema) - .expect("service column present in schema"); - QueryExpr::Scan { - source: Source::TimeSeries { - metric: "http_request_duration_seconds_bucket".into(), - }, - predicates: vec![pred], - schema, - } - } - - fn windowed_scan() -> QueryExpr { - QueryExpr::Window { - kind: WindowKind::Sliding, - size: Duration::from_secs(300), - slide: None, - child: Box::new(ts_scan()), - } - } - - fn agg_with(intent: AggIntent) -> QueryExpr { - QueryExpr::Aggregate { - by: vec![].into(), - aggs: vec![intent], - output_names: Vec::new(), - having: None, - child: Box::new(windowed_scan()), - } - } - - #[test] - fn binds_absent_archive_only() { - // `histogram_quantile(...)` is no longer an L3 intent — it's a - // PromQL operator that the controller's PromQL parser substitutes - // (Step γ5) into a plain `Aggregate { Quantile(φ) }`. The - // canonical archive-only anchor for this test is `Absent` - // (which has no ASAP-tier sketch family). - let expr = agg_with(AggIntent::Absent); - let out = BindArchiveOnly - .apply(&expr, &AccuracyTarget::Epsilon(0.01)) - .expect("rule should match Absent"); - match out { - PhysicalExpr::Logical(inner) => assert_eq!(inner, expr), - other => panic!("expected Logical pass-through, got {other:?}"), - } - } - - #[test] - fn binds_each_archive_only_intent() { - let intents = vec![ - AggIntent::Absent, - AggIntent::AbsentOverTime, - AggIntent::PresentOverTime, - AggIntent::Delta, - AggIntent::Deriv, - AggIntent::PredictLinear { seconds: 60.0 }, - AggIntent::DoubleExpSmoothing { - smoothing: 0.3, - trend: 0.3, - }, - AggIntent::IDelta, - AggIntent::Resets, - AggIntent::Changes, - AggIntent::HistogramCount, - AggIntent::Group, - ]; - for intent in intents { - let expr = agg_with(intent.clone()); - let out = BindArchiveOnly.apply(&expr, &AccuracyTarget::Epsilon(0.01)); - assert!( - out.is_some(), - "BindArchiveOnly should bind {intent:?} (archive-only Phase β intent)" - ); - } - } - - #[test] - fn does_not_bind_asap_tier_intents() { - // Sum / Quantile / Cardinality / TopK are NOT archive-only — they - // must NOT trigger BindArchiveOnly (the ASAP-tier rules own them). - for intent in [ - AggIntent::Sum { col: None }, - AggIntent::Quantile { - col: None, - q: 0.99, - accuracy: AccuracyTarget::Epsilon(0.01), - }, - AggIntent::Cardinality { - col: None, - accuracy: AccuracyTarget::Epsilon(0.01), - }, - AggIntent::TopK { - k: 10, - accuracy: AccuracyTarget::Epsilon(0.05), - }, - crate::intent_algebra::frequency(AccuracyTarget::Epsilon(0.01)), - AggIntent::Rate, - AggIntent::Increase, - ] { - let expr = agg_with(intent.clone()); - assert!( - BindArchiveOnly - .apply(&expr, &AccuracyTarget::Epsilon(0.01)) - .is_none(), - "BindArchiveOnly must not bind ASAP-tier intent {intent:?}" - ); - } - } - - #[test] - fn does_not_bind_non_aggregate_shapes() { - let scan = ts_scan(); - assert!(BindArchiveOnly - .apply(&scan, &AccuracyTarget::Epsilon(0.01)) - .is_none()); - let window = windowed_scan(); - assert!(BindArchiveOnly - .apply(&window, &AccuracyTarget::Epsilon(0.01)) - .is_none()); - } -} diff --git a/control_plane/src/sketch_algebra/rules/bind_cms_count.rs b/control_plane/src/sketch_algebra/rules/bind_cms_count.rs deleted file mode 100644 index 030574bd..00000000 --- a/control_plane/src/sketch_algebra/rules/bind_cms_count.rs +++ /dev/null @@ -1,122 +0,0 @@ -//! `BindCmsOnCount` — `Aggregate{Count{accuracy}}` and `Aggregate{Frequency{accuracy}}` → CountMin. -//! -//! Reference: `control_plane/docs/design.md` §6 line ~713 lists `BindCmsOnCount` -//! in the shared rule library. CMS is the canonical sketch for both: -//! -//! - `AggIntent::Count{accuracy}` — when the user wants COUNT(*) per -//! group with relaxed accuracy. The L4 readout is `EstimateOp::PointCount` -//! over each group key (the L5 emitter materialises the per-key reads). -//! - `AggIntent::Frequency{accuracy}` — `count(*) WHERE key = k`. CMS is -//! the textbook fit (Cormode-Muthukrishnan). -//! -//! Accuracy mapping: `AccuracyTarget::EpsilonDelta { epsilon: eps, delta }` → -//! `(w, d) = (⌈e/eps⌉, ⌈ln(1/delta)⌉)`. CMS guarantees additive error -//! `≤ eps · ‖f‖₁` with probability `≥ 1 − delta` (see -//! `accuracy_profile.rs` in ASAPQuery-backend for the formal bound). -//! -//! `AccuracyTarget::Epsilon(eps)` (no delta) defaults to `delta = 0.01` -//! per the in-tree default (`SketchDefaults::count_min_sketch.delta`). -//! `Exact` does not bind. - -#![allow(dead_code)] - -use crate::intent_algebra::{AggIntent, QueryExpr}; -use crate::sketch_algebra::physical_expr::{EstimateOp, PhysicalExpr}; -use crate::sketch_algebra::rules::Rule; -use crate::types_v2::AccuracyTarget; -use asap_sketch::{SummaryKind, SummaryParams}; - -/// Bind `Aggregate{Count}` / `Aggregate{Frequency}` to CMS. -pub struct BindCmsOnCount; - -impl Rule for BindCmsOnCount { - fn name(&self) -> &'static str { - "bind_cms_count" - } - - fn priority(&self) -> u16 { - 5 - } - - fn apply(&self, expr: &QueryExpr, accuracy: &AccuracyTarget) -> Option { - let (intent_accuracy, readout, child) = match expr { - QueryExpr::Aggregate { - aggs, child, by, .. - } if aggs.len() == 1 && by.is_empty() => match &aggs[0] { - AggIntent::Count { accuracy } => { - // Generic Count without a per-key readout falls back - // to the legacy logical aggregate; the only sketch- - // bound shape we emit here is a per-key PointCount. - // Use a sentinel "*" key meaning "all rows"; the L5 - // emitter resolves it against the stage allocator's - // per-group output. - ( - accuracy.clone(), - EstimateOp::PointCount { key: "*".into() }, - child, - ) - } - intent if crate::intent_algebra::as_frequency(intent).is_some() => ( - crate::intent_algebra::as_frequency(intent).unwrap(), - EstimateOp::PointCount { key: "*".into() }, - child, - ), - _ => return None, - }, - _ => return None, - }; - - // Read `(eps, delta)` from the tighter of the workload-level - // and per-intent accuracy targets. - let (eps, delta) = match (accuracy, &intent_accuracy) { - (AccuracyTarget::Exact, _) | (_, AccuracyTarget::Exact) => return None, - (AccuracyTarget::Epsilon(a), AccuracyTarget::Epsilon(b)) => (a.min(*b), 0.01), - ( - AccuracyTarget::Epsilon(a), - AccuracyTarget::EpsilonDelta { - epsilon: eps, - delta, - }, - ) - | ( - AccuracyTarget::EpsilonDelta { - epsilon: eps, - delta, - }, - AccuracyTarget::Epsilon(a), - ) => (a.min(*eps), *delta), - ( - AccuracyTarget::EpsilonDelta { - epsilon: a, - delta: da, - }, - AccuracyTarget::EpsilonDelta { - epsilon: b, - delta: db, - }, - ) => (a.min(*b), da.min(*db)), - }; - - if eps <= 0.0 || delta <= 0.0 || delta >= 1.0 { - return None; - } - - let w = (std::f64::consts::E / eps).ceil() as u32; - let d = (1.0 / delta).ln().ceil() as u32; - let w = w.max(2); - let d = d.max(1); - - Some(PhysicalExpr::estimate_over_agg( - readout, - SummaryKind::Cms, - // Heap-LESS CMS — `BindCmsOnCount` is the - // BindCmsOnCount path (frequency / count without TopK). - // The CMS-with-heap binding fires from - // `bind_cms_with_heap_on_topk` and binds `SummaryKind::CmsWithHeap` - // instead (heap-bearing is a distinct kind in `SummaryKind`, - // not a param flag). - SummaryParams::Cms { width: w, depth: d }, - (**child).clone(), - )) - } -} diff --git a/control_plane/src/sketch_algebra/rules/bind_cms_topk.rs b/control_plane/src/sketch_algebra/rules/bind_cms_topk.rs deleted file mode 100644 index 59935ded..00000000 --- a/control_plane/src/sketch_algebra/rules/bind_cms_topk.rs +++ /dev/null @@ -1,314 +0,0 @@ -//! `BindCountSketchOnTopK` — `Aggregate{TopK{k, accuracy}}` → a -//! heavy-hitter sketch, picked **recall-SLA-aware**. -//! -//! Reference: `control_plane/docs/design.md` §6 line ~419 — "`SketchAgg -//! { intent, col }` … L4 emits `PhysicalExpr::SketchAgg`" — and -//! `intent_algebra::AggIntent::TopK` (heavy-hitter intent) maps to a -//! heavy-hitter sketch primitive. Two families answer the intent: -//! -//! * **CMS-with-heap** (Count-Min + size-`k` heap; Cormode-Muthukrishnan -//! 2005). One-sided over-estimate; ~4 KB of wire state. Recovers the -//! top-`k` heavy hitters w.h.p. — perfectly adequate for the common -//! "who are the top-k" question under a **loose recall SLA**. -//! * **CountSketch-with-heap** (Charikar-Chen-Farach-Colton; median-of- -//! rows). Unbiased / two-sided / signed estimates, supports exact-rank -//! reconstruction — but ~250 KB of wire state, ~66× the CMS-heap cost -//! (see `optimizer::cost::wire`: `count_sketch_delta` 250 KB vs -//! `count_min_delta` 4 KB). -//! -//! ## The recall-aware binding (Fig-12 cost-gap fix) -//! -//! The original rule hard-bound CountSketch for every non-exact top-k. -//! That paid the 250 KB CountSketch price even when a loose recall SLA -//! (`recall@k ≥ 0.9`, approximate heavy hitters — the common case) would -//! be satisfied by the 4 KB CMS-heap, blowing the P95 cost-gap tail the -//! Fig-12 harness measured. -//! -//! The fix makes the family choice [`TopkRecallTier`]-driven: -//! -//! | Tier | When | Bound family | Wire cost | -//! |---|---|---|---| -//! | [`TopkRecallTier::Loose`] | approximate heavy hitters, `recall@k ≥ ~0.9`, no signed/exact-rank need | **CMS-with-heap** | ~4 KB | -//! | [`TopkRecallTier::Tight`] | exact rank, very-high recall, or signed/two-sided estimate required | **CountSketch-with-heap** | ~250 KB | -//! -//! The tie-break between "both meet the SLA" picks the cheaper family by -//! the [`optimizer::cost::wire`] cost table — the same "min cost s.t. SLA" -//! the oracle uses. For a loose SLA both families clear the recall bar, so -//! CMS-heap (the cheaper one) wins; for a tight SLA only CountSketch -//! clears it, so it wins regardless of price. -//! -//! ## Where the recall SLA comes from -//! -//! There is no per-query recall field today (`AccuracyTarget` is -//! `Exact` / `Epsilon` / `EpsilonDelta` — a *frequency*-error budget, not -//! a *recall* budget). Until one is threaded through (the follow-up), the -//! tier is inferred conservatively from the accuracy target: -//! -//! * `AccuracyTarget::Exact` (on either the query policy or the intent) → -//! exact rank required → **Tight** (CountSketch) — preserving the old -//! exact-bail behaviour but as a *family* pick rather than a `None`. -//! (Note: a true exact top-k still needs HashAgg+Heap; CountSketch is -//! the closest sketch-tier approximation and the unbiased estimator.) -//! * everything else → **Loose** (CMS-heap) — the cheap common-case -//! default. -//! -//! See [`TopkRecallTier::from_accuracy`] for the mapping and the -//! module-level follow-up note. -//! -//! Accuracy → `(w, d)` mapping: `AccuracyTarget::EpsilonDelta { eps, -//! delta }` → `(w, d) = (⌈e/eps⌉, ⌈ln(1/delta)⌉)`, identical for both CMS -//! and CountSketch. The heap size is the requested `k`. See -//! `accuracy_profile.rs` (ASAPQuery-backend) for the formal heavy-hitter -//! recall guarantee (a frequency sketch + size-`k` heap recovers all -//! heavy hitters with frequency `≥ ‖f‖₁ / k` w.h.p.). - -#![allow(dead_code)] - -use crate::intent_algebra::{AggIntent, QueryExpr}; -use crate::optimizer::cost::wire::WireCostTable; -use crate::sketch_algebra::physical_expr::{EstimateOp, PhysicalExpr}; -use crate::sketch_algebra::rules::Rule; -use crate::types_v2::AccuracyTarget; -use asap_sketch::{SummaryKind, SummaryParams}; - -/// Recall tier for a top-k binding — drives the family pick. -/// -/// "Recall" here is `recall@k` of the heavy-hitter set: the fraction of -/// the true top-`k` items the sketch's heap recovers. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum TopkRecallTier { - /// Approximate heavy hitters are fine (`recall@k ≥ ~0.9`), and no - /// signed / two-sided / exact-rank estimate is needed. CMS-with-heap - /// (one-sided over-estimate) is the cheap, correct choice. - Loose, - /// Exact rank, very-high recall, or a signed / two-sided estimate is - /// required. CountSketch-with-heap (unbiased median-of-rows) is the - /// fit despite its ~66× wire cost. - Tight, -} - -impl TopkRecallTier { - /// Conservative recall tier inferred from the (policy, intent) - /// accuracy targets. No per-query recall field exists today, so: - /// - /// * either side `Exact` → exact-rank intent → [`Tight`]. - /// * otherwise → [`Loose`] (the cheap common-case default). - /// - /// **Follow-up:** thread a real per-query `recall@k` target (or a - /// `signed`/`two_sided` flag) through `AccuracyTarget` / the query - /// policy and pivot here on that instead of inferring from `Exact`. - /// - /// [`Tight`]: TopkRecallTier::Tight - /// [`Loose`]: TopkRecallTier::Loose - pub fn from_accuracy(policy: &AccuracyTarget, intent: &AccuracyTarget) -> Self { - match (policy, intent) { - (AccuracyTarget::Exact, _) | (_, AccuracyTarget::Exact) => TopkRecallTier::Tight, - _ => TopkRecallTier::Loose, - } - } -} - -pub struct BindCountSketchOnTopK; - -impl BindCountSketchOnTopK { - /// Families that satisfy a top-k recall tier, cheapest-first. - /// - /// * `Loose`: both CMS-heap (cheap) and CountSketch-heap clear the - /// recall bar, so the cost model is free to pick the cheaper one. - /// * `Tight`: only CountSketch-heap (unbiased / signed / exact-rank) - /// clears the bar. - fn candidate_families(tier: TopkRecallTier) -> &'static [SummaryKind] { - match tier { - TopkRecallTier::Loose => &[SummaryKind::Cms, SummaryKind::CountSketch], - TopkRecallTier::Tight => &[SummaryKind::CountSketch], - } - } - - /// Pick the SLA-meeting family with the lowest per-flush wire cost. - /// `candidates` is already filtered to the families that meet the - /// recall SLA (see [`Self::candidate_families`]); this is the - /// "min cost s.t. SLA" tie-break the oracle uses. - fn cheapest_family(candidates: &[SummaryKind], table: &WireCostTable) -> SummaryKind { - candidates - .iter() - .min_by_key(|k| table.for_kind(k).per_flush()) - .cloned() - // (above: k is &&SummaryKind; for_kind autoderefs to &SummaryKind) - // candidate_families never returns empty. - .unwrap_or(SummaryKind::CountSketch) - } - - /// Bind a top-k under an explicit recall tier — bypasses the - /// accuracy-inferred tier in [`Rule::apply`]. Used by - /// `optimizer::rules::bind_workload_typed`, which has already pinned - /// the family from the capability matrix / a `sketch_family_override` - /// and just needs the matching heap-bearing binding: - /// - /// * a CountSketch family pick → [`TopkRecallTier::Tight`] - /// (CountSketch-with-heap, the unbiased canonical pick). - /// * a CMS family pick on a top-k → [`TopkRecallTier::Loose`] - /// (CMS-with-heap). - pub fn apply_with_tier( - &self, - expr: &QueryExpr, - accuracy: &AccuracyTarget, - tier: TopkRecallTier, - ) -> Option { - self.bind(expr, accuracy, Some(tier)) - } - - /// Core binding. When `forced_tier` is `Some`, that tier is used; - /// otherwise the tier is inferred from the accuracy targets via - /// [`TopkRecallTier::from_accuracy`]. - fn bind( - &self, - expr: &QueryExpr, - accuracy: &AccuracyTarget, - forced_tier: Option, - ) -> Option { - let (k_topk, intent_accuracy, child) = match expr { - QueryExpr::Aggregate { - aggs, child, by, .. - } if aggs.len() == 1 && by.is_empty() => match &aggs[0] { - AggIntent::TopK { k, accuracy } => (*k, accuracy.clone(), child), - _ => return None, - }, - _ => return None, - }; - - if k_topk == 0 { - return None; - } - - // Recall tier first — it decides which families are viable, and - // (for the Tight tier) it is the only thing that keeps CountSketch - // in play. Note we no longer bail to `None` on `Exact`: an exact - // top-k intent picks the unbiased CountSketch family (the closest - // sketch-tier approximation) rather than declining the binding. - let tier = forced_tier - .unwrap_or_else(|| TopkRecallTier::from_accuracy(accuracy, &intent_accuracy)); - - // Derive (w, d) from the frequency-error budget. `Exact` on either - // side leaves the ε/δ unspecified (it's a *recall* tier signal, - // not a frequency budget), so fall back to the catalog defaults - // (eps=0.01, delta=0.01) used elsewhere for heavy-hitter sketches. - let (eps, delta) = match (accuracy, &intent_accuracy) { - (AccuracyTarget::Exact, AccuracyTarget::Exact) => (0.01, 0.01), - (AccuracyTarget::Exact, other) | (other, AccuracyTarget::Exact) => match other { - AccuracyTarget::Epsilon(a) => (*a, 0.01), - AccuracyTarget::EpsilonDelta { - epsilon: eps, - delta, - } => (*eps, *delta), - AccuracyTarget::Exact => (0.01, 0.01), - }, - (AccuracyTarget::Epsilon(a), AccuracyTarget::Epsilon(b)) => (a.min(*b), 0.01), - ( - AccuracyTarget::Epsilon(a), - AccuracyTarget::EpsilonDelta { - epsilon: eps, - delta, - }, - ) - | ( - AccuracyTarget::EpsilonDelta { - epsilon: eps, - delta, - }, - AccuracyTarget::Epsilon(a), - ) => (a.min(*eps), *delta), - ( - AccuracyTarget::EpsilonDelta { - epsilon: a, - delta: da, - }, - AccuracyTarget::EpsilonDelta { - epsilon: b, - delta: db, - }, - ) => (a.min(*b), da.min(*db)), - }; - - if eps <= 0.0 || delta <= 0.0 || delta >= 1.0 { - return None; - } - - let w = (std::f64::consts::E / eps).ceil() as u32; - let d = (1.0 / delta).ln().ceil() as u32; - // CountSketch columns MUST be a power of two: the agent - // (asapedgeprocessor config_validate) rejects non-pow2 cols because - // sketchlib bit-slices the hash with a pow2 column mask. Round the - // ε-derived width UP to the next power of two — this only tightens - // the additive bound (ε ≤ e/w) and prevents an agent-side - // "cols must be a power of two" crash on config apply. CMS does not - // require pow2 cols, but using the same width keeps the two - // families' accuracy comparable for the cost-model tie-break. - let w = w.max(2).next_power_of_two(); - let d = d.max(1); - - // Cost-aware tie-break: among the families that meet the recall - // SLA for this tier, pick the cheapest by the wire cost model - // (the same "min cost s.t. SLA" the oracle uses). - let table = WireCostTable::default(); - let family = Self::cheapest_family(Self::candidate_families(tier), &table); - - // `heap_size` — `SummaryKind::CmsWithHeap`/`CountSketchWithHeap` - // promotes the heap from a `SketchParams::{Cms,CountSketch}.with_heap` - // bool flag to a first-class identity variant carrying the real - // heap size. `k_topk` is the correct value (this is the requested - // top-k `k`); note the backend wire builder - // (`emit::stage_config::sketch_params_to_json`) doesn't read this - // field today regardless (a pre-existing gap, out of scope for - // this rename), so this is strictly more correct in-memory data - // with no observable wire-behavior change. - let heap_size = k_topk as u32; - - let (kind, params) = match family { - SummaryKind::Cms => ( - SummaryKind::CmsWithHeap, - // CMS-Heap pattern: pair the CMS matrix with a size-k - // heavy-hitter heap. The streaming-config emit promotes - // this to `CountMinSketchWithHeap` (servable as - // FrequencyTopk per `asap_tier_analysis`). - SummaryParams::CmsWithHeap { - width: w, - depth: d, - heap_size, - }, - ), - // Tight tier (and any future family) → CountSketch-with-heap. - _ => ( - SummaryKind::CountSketchWithHeap, - SummaryParams::CountSketchWithHeap { - width: w, - depth: d, - heap_size, - }, - ), - }; - - Some(PhysicalExpr::estimate_over_agg( - EstimateOp::TopK { k: k_topk }, - kind, - params, - (**child).clone(), - )) - } -} - -impl Rule for BindCountSketchOnTopK { - fn name(&self) -> &'static str { - "bind_cms_topk" - } - - fn priority(&self) -> u16 { - 5 - } - - fn apply(&self, expr: &QueryExpr, accuracy: &AccuracyTarget) -> Option { - // Recall-aware default: tier is inferred from the accuracy targets - // (loose → CMS-heap, tight/exact → CountSketch). Callers that have - // already pinned a family use [`Self::apply_with_tier`] instead. - self.bind(expr, accuracy, None) - } -} diff --git a/control_plane/src/sketch_algebra/rules/bind_ddsketch_quantile.rs b/control_plane/src/sketch_algebra/rules/bind_ddsketch_quantile.rs deleted file mode 100644 index 17b687cf..00000000 --- a/control_plane/src/sketch_algebra/rules/bind_ddsketch_quantile.rs +++ /dev/null @@ -1,86 +0,0 @@ -//! `BindDDSketchOnQuantile` — `Aggregate{Quantile{q, accuracy}}` → DDSketch. -//! -//! Reference: `control_plane/docs/design.md` §6 sketch_algebra (line ~565) -//! lists DDSketch alongside KLL as a quantile family. DDSketch has a -//! tail-relative-error guarantee — `|estimate − true| ≤ alpha · true` — -//! which makes it the preferred choice when the user asks for relative -//! tail-error rather than rank-error. -//! -//! Accuracy mapping: `AccuracyTarget::Epsilon(eps)` → `alpha = eps`. -//! DDSketch's parameter *is* the relative-error bound, so the mapping is -//! the identity. See `accuracy_profile.rs` (ASAPQuery-backend) for the -//! formal proof. -//! -//! Rule selection: this rule has priority 6 (just above -//! `BindKllOnQuantile`'s priority 5). The dispatcher's tie-break gives -//! DDSketch precedence whenever both rules fire on the same intent — -//! that matches the legacy `algebra::directory::sketch_type_for_agg` -//! behaviour, which already picks DDSketch as the default Quantile -//! sketch (see `controller/src/algebra/directory.rs` line ~59). - -#![allow(dead_code)] - -use crate::intent_algebra::{AggIntent, QueryExpr}; -use crate::sketch_algebra::physical_expr::{EstimateOp, PhysicalExpr}; -use crate::sketch_algebra::rules::Rule; -use crate::types_v2::AccuracyTarget; -use asap_sketch::{SummaryKind, SummaryParams}; - -/// Bind a single-intent `Aggregate{Quantile{q, accuracy}}` to DDSketch. -pub struct BindDDSketchOnQuantile; - -impl Rule for BindDDSketchOnQuantile { - fn name(&self) -> &'static str { - "bind_ddsketch_quantile" - } - - fn priority(&self) -> u16 { - // DDSketch wins the tie-break when both rules fire — see module - // docstring for the rationale (matches the legacy directory - // default for SP-2/SP-4). - 6 - } - - fn apply(&self, expr: &QueryExpr, accuracy: &AccuracyTarget) -> Option { - let (q, intent_accuracy, child) = match expr { - QueryExpr::Aggregate { - aggs, child, by, .. - } if aggs.len() == 1 && by.is_empty() => match &aggs[0] { - AggIntent::Quantile { q, accuracy, .. } => (*q, accuracy.clone(), child), - _ => return None, - }, - _ => return None, - }; - - if !(0.0..=1.0).contains(&q) { - return None; - } - - // DDSketch needs an explicit relative-error budget. `Exact` - // disables the rule; the dispatcher then picks KLL (or falls - // through to logical pass-through). - let alpha = match (accuracy, &intent_accuracy) { - (AccuracyTarget::Exact, _) | (_, AccuracyTarget::Exact) => return None, - (AccuracyTarget::Epsilon(a), AccuracyTarget::Epsilon(b)) => a.min(*b), - (AccuracyTarget::Epsilon(a), AccuracyTarget::EpsilonDelta { epsilon: eps, .. }) - | (AccuracyTarget::EpsilonDelta { epsilon: eps, .. }, AccuracyTarget::Epsilon(a)) => { - a.min(*eps) - } - ( - AccuracyTarget::EpsilonDelta { epsilon: a, .. }, - AccuracyTarget::EpsilonDelta { epsilon: b, .. }, - ) => a.min(*b), - }; - - if alpha <= 0.0 || alpha >= 1.0 { - return None; - } - - Some(PhysicalExpr::estimate_over_agg( - EstimateOp::Quantile { q }, - SummaryKind::DDSketch, - SummaryParams::DDSketch { alpha }, - (**child).clone(), - )) - } -} diff --git a/control_plane/src/sketch_algebra/rules/bind_exact_agg.rs b/control_plane/src/sketch_algebra/rules/bind_exact_agg.rs deleted file mode 100644 index 6c86aa1c..00000000 --- a/control_plane/src/sketch_algebra/rules/bind_exact_agg.rs +++ /dev/null @@ -1,444 +0,0 @@ -//! `BindExactAgg` — emits `PhysicalExpr::ExactAgg` for the exact-aggregation -//! intents the ASAP tier can serve from a precompute accumulator. -//! -//! ## Coverage -//! -//! | L3 `AggIntent` | L4 `PhysicalExpr::ExactAgg` | -//! |---|---| -//! | `Sum` | `agg_type: AggregationType::Sum` | -//! | `Rate { .. }` | `agg_type: AggregationType::Increase` | -//! | `Increase { .. }` | `agg_type: AggregationType::Increase` | -//! | `Min` / `Max` | `agg_type: AggregationType::MinMax` | -//! -//! `Rate` and `Increase` share the `Increase` accumulator because rate is -//! computed as `increase / window_seconds` — a scalar division on the -//! accumulator's output, not a separate accumulator family. The wrapping -//! division (when needed) is the L5 emitter's responsibility, not the -//! L4 binder's. -//! -//! `Min` and `Max` share the `MinMax` accumulator (min/max are exact and -//! mergeable by construction — comparing two partial extrema needs no -//! approximation at all) — matches ASAPController's own -//! `crates/plan/src/boundary.rs`, which realizes `Min`/`Max` as an exact -//! mergeable accumulator, same tier as `Sum`/`Rate`/`Increase`. See the -//! "What this rule does NOT bind" list below for why this superseded the -//! earlier quantile-sketch-only routing. -//! -//! ## What this rule does NOT bind -//! -//! - `AggIntent::Count { accuracy: Exact }` — `count_over_time`. The -//! PR #200/#201 follow-up bound this to `AggregationType::Sum` on the -//! "count = sum-of-1s" theory, but the data plane has no count -//! accumulator: `SumAccumulator` only tracks `sum: f64` and returns -//! it for both `Statistic::Sum` and `Statistic::Count`, so the result -//! is the sum of sample VALUES, not the sample count. Reverted — -//! `count_over_time` routes to archive (which counts correctly) -//! until a real `SumCountAccumulator` lands. -//! - `AggIntent::Avg` — needs a `(sum, count)` accumulator, same -//! missing piece. Stays on archive until `SumCountAccumulator` lands. -//! - `AggIntent::Quantile { Exact }` / `Cardinality { Exact }` / -//! `TopK { Exact }` / `Frequency { Exact }` — the exact-accuracy -//! variants of approximate-by-default intents. No exact-precompute -//! shape exists in `AggregationType` for these; they need -//! `HashAgg` / `SortAgg` / `SortMerge` from the deployment's -//! exact-physical-operator family (none of which run at the ASAP tier -//! today). -//! - `AggIntent::Min` / `AggIntent::Max` — previously left unbound here -//! on the theory that `quantile(0)`/`quantile(1)` (DDSketch/KLL) -//! already covered them, so a dedicated rule would just compete with -//! the sketch-family rules for no accuracy benefit. That theory -//! didn't hold: `bind_kll_quantile`/`bind_ddsketch_quantile` only -//! ever pattern-match `AggIntent::Quantile`, never `Min`/`Max` — no -//! rule actually implemented the promised coverage, so `Min`/`Max` -//! silently fell through to archive regardless of what -//! `capability_for` claimed. Now bound here instead: min/max are -//! exact and mergeable by construction (no approximation needed), -//! matching ASAPController's own treatment -//! (`crates/plan/src/boundary.rs`) and the data plane's existing -//! `MinMaxAccumulator`/`AggregationType::MinMax`. -//! -//! ## Priority -//! -//! `2` — above `bind_archive_only` (which has the lowest priority for a -//! catch-all on Sum / Rate / Increase etc.) but below the sketch -//! family rules (priorities 4–6). When this rule and `bind_archive_only` -//! both match an archive-routable intent (Sum / Rate / Increase), the -//! ExactAgg path wins — same direction as the analyzer flip in -//! `capability_for`. - -#![allow(dead_code)] - -use std::time::Duration; - -use asap_types::AggregationType; - -use crate::intent_algebra::{AggIntent, QueryExpr}; -use crate::sketch_algebra::physical_expr::PhysicalExpr; -use crate::sketch_algebra::rules::Rule; -use crate::types_v2::AccuracyTarget; - -/// Bind exact-aggregation intents (Sum / Rate / Increase / Min / Max) -/// to `PhysicalExpr::ExactAgg`. See module doc for the mapping table. -pub struct BindExactAgg; - -impl Rule for BindExactAgg { - fn name(&self) -> &'static str { - "bind_exact_agg" - } - - fn priority(&self) -> u16 { - // Above bind_archive_only (priority 1) but below sketch families - // (4-6). Sum / Rate / Increase are archive-only intents in the - // sketch-family world; this rule overrides that routing. - 2 - } - - fn apply(&self, expr: &QueryExpr, _accuracy: &AccuracyTarget) -> Option { - let (intent, child, keyed) = match expr { - QueryExpr::Aggregate { - aggs, child, by, .. - } if aggs.len() == 1 => (&aggs[0], child, !by.is_empty()), - _ => return None, - }; - - // Keyed aggregations (non-empty `by`) lower to the multi-pop - // accumulator variant; the data plane stores per-key state so - // the query can fan results out over the surviving labels. - // Unkeyed aggregations stay on the single-pop variant. - let agg_type = match intent { - AggIntent::Sum { .. } => { - if keyed { - AggregationType::MultipleSum - } else { - AggregationType::Sum - } - } - AggIntent::Rate | AggIntent::Increase => { - // The window used to live on the intent (`Rate { window - // }`); after the ASAPController IR merge it lives on the - // enclosing `QueryExpr::Window` node instead. The data - // plane keys the policy on (metric, attrs, agg_kind, - // filter) plus the per-policy `window_size` -- the value - // is informational here, not consumed further. Empty - // windows are semantically meaningless; reject them so - // the rule doesn't fire on a malformed L3 input. - let window = match &**child { - QueryExpr::Window { size, .. } => *size, - _ => Duration::ZERO, - }; - if window == Duration::ZERO { - return None; - } - if keyed { - AggregationType::MultipleIncrease - } else { - AggregationType::Increase - } - } - AggIntent::Min { .. } | AggIntent::Max { .. } => { - if keyed { - AggregationType::MultipleMinMax - } else { - AggregationType::MinMax - } - } - // `AggIntent::Count{Exact}` (count_over_time) is intentionally - // NOT bound here — see the module doc. It needs a real - // count accumulator, which doesn't exist yet; binding it to - // `Sum` returns sum-of-values instead of sample-count. - _ => return None, - }; - - Some(PhysicalExpr::exact_agg_over_logical( - agg_type, - (**child).clone(), - )) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::intent_algebra::{Schema, Source}; - - fn scan(metric: &str) -> QueryExpr { - // `Schema::default()` has no columns, so a "service" label filter - // could never resolve to a `Predicate` here anyway - // (`label_filter_to_predicate` would return `None`) — dropped - // rather than built-and-discarded. - QueryExpr::Scan { - source: Source::TimeSeries { - metric: metric.into(), - }, - predicates: Vec::new(), - schema: Schema::default(), - } - } - - fn agg_over(intent: AggIntent, metric: &str) -> QueryExpr { - QueryExpr::Aggregate { - aggs: vec![intent], - child: Box::new(scan(metric)), - by: vec![].into(), - output_names: Vec::new(), - having: None, - } - } - - /// Like `agg_over`, but wraps the scan in a `QueryExpr::Window` -- - /// `Rate`/`Increase` read their window off this enclosing node, not - /// off the intent (post ASAPController IR merge; the intent used to - /// carry `window: Duration` itself). - fn windowed_agg_over(intent: AggIntent, metric: &str, window: Duration) -> QueryExpr { - QueryExpr::Aggregate { - aggs: vec![intent], - child: Box::new(QueryExpr::Window { - kind: crate::intent_algebra::WindowKind::Sliding, - size: window, - slide: None, - child: Box::new(scan(metric)), - }), - by: vec![].into(), - output_names: Vec::new(), - having: None, - } - } - - fn check_binds(intent: AggIntent, expected: AggregationType) { - let expr = agg_over(intent, "test_metric"); - let bound = BindExactAgg - .apply(&expr, &AccuracyTarget::Exact) - .unwrap_or_else(|| panic!("rule didn't fire on {expected:?}")); - match bound { - PhysicalExpr::ExactAgg { agg_type, .. } => assert_eq!(agg_type, expected), - other => panic!("expected ExactAgg, got {other:?}"), - } - } - - fn check_binds_windowed(intent: AggIntent, window: Duration, expected: AggregationType) { - let expr = windowed_agg_over(intent, "test_metric", window); - let bound = BindExactAgg - .apply(&expr, &AccuracyTarget::Exact) - .unwrap_or_else(|| panic!("rule didn't fire on {expected:?}")); - match bound { - PhysicalExpr::ExactAgg { agg_type, .. } => assert_eq!(agg_type, expected), - other => panic!("expected ExactAgg, got {other:?}"), - } - } - - #[test] - fn binds_sum_to_exact_agg_sum() { - check_binds(AggIntent::Sum { col: None }, AggregationType::Sum); - } - - #[test] - fn binds_rate_to_exact_agg_increase() { - check_binds_windowed( - AggIntent::Rate, - Duration::from_secs(60), - AggregationType::Increase, - ); - } - - #[test] - fn binds_increase_to_exact_agg_increase() { - check_binds_windowed( - AggIntent::Increase, - Duration::from_secs(300), - AggregationType::Increase, - ); - } - - #[test] - fn binds_min_to_exact_agg_minmax() { - check_binds(AggIntent::Min { col: None }, AggregationType::MinMax); - } - - #[test] - fn binds_max_to_exact_agg_minmax() { - check_binds(AggIntent::Max { col: None }, AggregationType::MinMax); - } - - #[test] - fn does_not_bind_count_exact() { - // `count_over_time` (Count{Exact}) is NOT bound — the data - // plane has no count accumulator. Routes to archive instead. - // See the module doc. - let expr = agg_over( - AggIntent::Count { - accuracy: AccuracyTarget::Exact, - }, - "test_metric", - ); - assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none()); - } - - #[test] - fn does_not_bind_count_approximate() { - // Approximate count is the cardinality-sketch path's domain. - let expr = agg_over( - AggIntent::Count { - accuracy: AccuracyTarget::Epsilon(0.01), - }, - "test_metric", - ); - assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none()); - } - - #[test] - fn does_not_bind_avg() { - let expr = agg_over(AggIntent::Avg { col: None }, "test_metric"); - assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none()); - } - - #[test] - fn does_not_bind_quantile() { - let expr = agg_over( - AggIntent::Quantile { - col: None, - q: 0.99, - accuracy: AccuracyTarget::Epsilon(0.01), - }, - "test_metric", - ); - assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none()); - } - - fn agg_over_with_by(intent: AggIntent, metric: &str, by: Vec) -> QueryExpr { - QueryExpr::Aggregate { - aggs: vec![intent], - child: Box::new(scan(metric)), - by: by.into(), - output_names: Vec::new(), - having: None, - } - } - - fn windowed_agg_over_with_by( - intent: AggIntent, - metric: &str, - window: Duration, - by: Vec, - ) -> QueryExpr { - QueryExpr::Aggregate { - aggs: vec![intent], - child: Box::new(QueryExpr::Window { - kind: crate::intent_algebra::WindowKind::Sliding, - size: window, - slide: None, - child: Box::new(scan(metric)), - }), - by: by.into(), - output_names: Vec::new(), - having: None, - } - } - - fn check_keyed_binds(intent: AggIntent, expected: AggregationType) { - let expr = agg_over_with_by(intent, "test_metric", vec![0]); - let bound = BindExactAgg - .apply(&expr, &AccuracyTarget::Exact) - .unwrap_or_else(|| panic!("rule didn't fire on keyed {expected:?}")); - match bound { - PhysicalExpr::ExactAgg { agg_type, .. } => assert_eq!(agg_type, expected), - other => panic!("expected ExactAgg, got {other:?}"), - } - } - - fn check_keyed_binds_windowed(intent: AggIntent, window: Duration, expected: AggregationType) { - let expr = windowed_agg_over_with_by(intent, "test_metric", window, vec![0]); - let bound = BindExactAgg - .apply(&expr, &AccuracyTarget::Exact) - .unwrap_or_else(|| panic!("rule didn't fire on keyed {expected:?}")); - match bound { - PhysicalExpr::ExactAgg { agg_type, .. } => assert_eq!(agg_type, expected), - other => panic!("expected ExactAgg, got {other:?}"), - } - } - - #[test] - fn keyed_sum_binds_to_multiple_sum() { - check_keyed_binds(AggIntent::Sum { col: None }, AggregationType::MultipleSum); - } - - #[test] - fn keyed_rate_binds_to_multiple_increase() { - check_keyed_binds_windowed( - AggIntent::Rate, - Duration::from_secs(60), - AggregationType::MultipleIncrease, - ); - } - - #[test] - fn keyed_increase_binds_to_multiple_increase() { - check_keyed_binds_windowed( - AggIntent::Increase, - Duration::from_secs(300), - AggregationType::MultipleIncrease, - ); - } - - #[test] - fn keyed_min_binds_to_multiple_minmax() { - check_keyed_binds( - AggIntent::Min { col: None }, - AggregationType::MultipleMinMax, - ); - } - - #[test] - fn keyed_max_binds_to_multiple_minmax() { - check_keyed_binds( - AggIntent::Max { col: None }, - AggregationType::MultipleMinMax, - ); - } - - #[test] - fn keyed_count_exact_does_not_bind() { - // `count by (...) (count_over_time(...))` — Count{Exact} is - // unbound regardless of keying; no count accumulator exists. - let expr = agg_over_with_by( - AggIntent::Count { - accuracy: AccuracyTarget::Exact, - }, - "test_metric", - vec![0], - ); - assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none()); - } - - #[test] - fn unkeyed_sum_still_binds_to_single_pop_sum() { - // Regression guard: the keyed/unkeyed branch must still - // dispatch correctly on `by.is_empty()`. - check_binds(AggIntent::Sum { col: None }, AggregationType::Sum); - } - - #[test] - fn rejects_zero_window_rate() { - // Defensive — a zero-window Rate is semantically meaningless. - // `agg_over` (unwindowed child) exercises this directly: no - // enclosing `QueryExpr::Window` node means the window read - // defaults to `Duration::ZERO`. - let expr = agg_over(AggIntent::Rate, "test_metric"); - assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none()); - } - - #[test] - fn rejects_explicit_zero_window_rate() { - // Same rejection when the Window node is present but its size is - // literally zero (as opposed to no Window node at all). - let expr = windowed_agg_over(AggIntent::Rate, "test_metric", Duration::ZERO); - assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none()); - } - - #[test] - fn priority_above_archive_only() { - // Sanity check: this rule wins against bind_archive_only when - // both would fire on a Sum / Rate / Increase intent. - use crate::sketch_algebra::rules::bind_archive_only::BindArchiveOnly; - assert!(BindExactAgg.priority() > BindArchiveOnly.priority()); - } -} diff --git a/control_plane/src/sketch_algebra/rules/bind_hll_cardinality.rs b/control_plane/src/sketch_algebra/rules/bind_hll_cardinality.rs deleted file mode 100644 index 530827c1..00000000 --- a/control_plane/src/sketch_algebra/rules/bind_hll_cardinality.rs +++ /dev/null @@ -1,106 +0,0 @@ -//! `BindHllOnCardinality` — `Aggregate{Cardinality{accuracy}}` → HLL. -//! -//! Reference: `control_plane/docs/design.md` §6 line ~714 lists -//! `BindHllOnCardinality` in the shared rule library. HLL is the -//! catalog-default sketch family for COUNT DISTINCT — see -//! `algebra::directory::sketch_type_for_agg` line ~59 for the existing -//! in-tree binding. -//! -//! Accuracy mapping: HLL standard error is `≈ 1.04 / √m` where -//! `m = 2^precision`. Inverting: `precision ≈ 2 · log2(1.04 / eps)`. -//! See `accuracy_profile.rs` (ASAPQuery-backend) for the formal bound -//! and the in-tree default rungs (precision 10 / 12 / 14 / 16 covering -//! ε ≈ 3% / 1.5% / 0.8% / 0.4%). - -#![allow(dead_code)] - -use asap_sketch::{SummaryKind, SummaryParams}; - -use crate::intent_algebra::{AggIntent, QueryExpr}; -use crate::sketch_algebra::physical_expr::{EstimateOp, PhysicalExpr}; -use crate::sketch_algebra::rules::Rule; -use crate::types_v2::AccuracyTarget; - -pub struct BindHllOnCardinality; - -impl Rule for BindHllOnCardinality { - fn name(&self) -> &'static str { - "bind_hll_cardinality" - } - - fn priority(&self) -> u16 { - 5 - } - - fn apply(&self, expr: &QueryExpr, accuracy: &AccuracyTarget) -> Option { - let (intent_accuracy, child) = match expr { - QueryExpr::Aggregate { - aggs, child, by, .. - } if aggs.len() == 1 && by.is_empty() => match &aggs[0] { - AggIntent::Cardinality { accuracy, .. } => (accuracy.clone(), child), - _ => return None, - }, - _ => return None, - }; - - // Read the tighter of the workload-level and per-intent budgets. - let eps = match (accuracy, &intent_accuracy) { - (AccuracyTarget::Exact, _) | (_, AccuracyTarget::Exact) => return None, - (AccuracyTarget::Epsilon(a), AccuracyTarget::Epsilon(b)) => a.min(*b), - (AccuracyTarget::Epsilon(a), AccuracyTarget::EpsilonDelta { epsilon: eps, .. }) - | (AccuracyTarget::EpsilonDelta { epsilon: eps, .. }, AccuracyTarget::Epsilon(a)) => { - a.min(*eps) - } - ( - AccuracyTarget::EpsilonDelta { epsilon: a, .. }, - AccuracyTarget::EpsilonDelta { epsilon: b, .. }, - ) => a.min(*b), - }; - - if eps <= 0.0 { - return None; - } - - let precision = hll_precision_for_eps(eps); - - Some(PhysicalExpr::estimate_over_agg( - EstimateOp::Cardinality, - SummaryKind::Hll, - SummaryParams::Hll { precision }, - (**child).clone(), - )) - } -} - -/// Map an ε standard-error budget to the HLL `precision` (log2 register -/// count). Mirrors the in-tree default rungs in `algebra::directory` — -/// precision 10 (ε≈3.25%) / 12 (ε≈1.6%) / 14 (ε≈0.81%) / 16 (ε≈0.41%). -/// `u8` to match `asap_sketch::SummaryParams::Hll`'s field type — every -/// rung here is well within range. -fn hll_precision_for_eps(eps: f64) -> u8 { - if eps <= 0.0 { - return 16; - } - if eps >= 0.03 { - 10 - } else if eps >= 0.015 { - 12 - } else if eps >= 0.008 { - 14 - } else { - 16 - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn precision_at_canonical_rungs() { - assert_eq!(hll_precision_for_eps(0.03), 10); - assert_eq!(hll_precision_for_eps(0.015), 12); - assert_eq!(hll_precision_for_eps(0.008), 14); - assert_eq!(hll_precision_for_eps(0.001), 16); - } -} diff --git a/control_plane/src/sketch_algebra/rules/bind_kll_quantile.rs b/control_plane/src/sketch_algebra/rules/bind_kll_quantile.rs deleted file mode 100644 index 664b9d78..00000000 --- a/control_plane/src/sketch_algebra/rules/bind_kll_quantile.rs +++ /dev/null @@ -1,123 +0,0 @@ -//! `BindKllOnQuantile` — `Aggregate{Quantile{q, accuracy}}` → `SketchEstimate{Quantile{q}}` over `SketchAgg{KLL}`. -//! -//! Reference: `control_plane/docs/design.md` §6 line ~1157 — "the shared rule -//! `core::optimizer::rules::BindKllOnQuantile` matches `Aggregate { aggs: -//! [Quantile{q, accuracy}] }`, consults the sketch catalog (KLL has -//! `supported_intents: [Quantile]`, is mergeable, satisfies `ε=0.01` at -//! `k=200`), and rewrites the matched sub-DAG into a `SketchAgg` wrapped -//! in a `SketchEstimate`". -//! -//! Accuracy mapping: `AccuracyTarget::Epsilon(eps)` → KLL stream-size `k`. -//! KLL's rank-error bound is `≈ 2.6 / (eps · √π)` per Karnin-Lang-Liberty; -//! see `accuracy_profile.rs` (ASAPQuery-backend) for the formal mapping -//! table — `eps=0.01 → k=200`, `eps=0.005 → k=400`, `eps=0.001 → k=2048`, -//! `eps=0.0001 → k=8192`. `Exact` does not bind. - -#![allow(dead_code)] - -use crate::intent_algebra::{AggIntent, QueryExpr}; -use crate::sketch_algebra::physical_expr::{EstimateOp, PhysicalExpr}; -use crate::sketch_algebra::rules::Rule; -use crate::types_v2::AccuracyTarget; -use asap_sketch::{SummaryKind, SummaryParams}; - -/// Bind a single-intent `Aggregate{Quantile{q, accuracy}}` to KLL. -pub struct BindKllOnQuantile; - -impl Rule for BindKllOnQuantile { - fn name(&self) -> &'static str { - "bind_kll_quantile" - } - - fn priority(&self) -> u16 { - // KLL is the planner's preferred quantile sketch when the - // accuracy budget is unspecified or when an `Exact`-by-default - // SLA is in play (see `algebra::directory::sketch_type_for_agg`, - // which already picks KLL as the in-tree default for SP-2/SP-4). - // It edges out DDSketch on rank-error tightness when the budget - // is rank-driven; DDSketch (priority 6) wins when the user - // supplied a tail-relative epsilon. - 5 - } - - fn apply(&self, expr: &QueryExpr, accuracy: &AccuracyTarget) -> Option { - // Only the single-intent Aggregate{Quantile{...}} shape binds — - // multi-intent Aggregates and TopK-shaped quantiles take other - // rules. - let (q, intent_accuracy, child) = match expr { - QueryExpr::Aggregate { - aggs, child, by, .. - } if aggs.len() == 1 && by.is_empty() => match &aggs[0] { - AggIntent::Quantile { q, accuracy, .. } => (*q, accuracy.clone(), child), - _ => return None, - }, - _ => return None, - }; - - // Quantile φ must be in [0, 1]. - if !(0.0..=1.0).contains(&q) { - return None; - } - - // Pick the binding budget. Per the orchestrator-level convention, - // the per-intent `accuracy` field overrides the workload-level - // `AccuracyTarget` when both are present; here we use the more - // restrictive (lower-eps) of the two when both are `Epsilon`. If - // either side is `Exact`, the rule does not bind. - let eps = match (accuracy, &intent_accuracy) { - (AccuracyTarget::Exact, _) | (_, AccuracyTarget::Exact) => return None, - (AccuracyTarget::Epsilon(a), AccuracyTarget::Epsilon(b)) => a.min(*b), - (AccuracyTarget::Epsilon(a), AccuracyTarget::EpsilonDelta { epsilon: eps, .. }) - | (AccuracyTarget::EpsilonDelta { epsilon: eps, .. }, AccuracyTarget::Epsilon(a)) => { - a.min(*eps) - } - ( - AccuracyTarget::EpsilonDelta { epsilon: a, .. }, - AccuracyTarget::EpsilonDelta { epsilon: b, .. }, - ) => a.min(*b), - }; - - let k = kll_k_for_eps(eps); - - Some(PhysicalExpr::estimate_over_agg( - EstimateOp::Quantile { q }, - SummaryKind::Kll, - SummaryParams::Kll { k }, - (**child).clone(), - )) - } -} - -/// Map an ε rank-error budget to a KLL stream-size `k`. Mirrors the -/// `accuracy_profile.rs` table referenced in the module docstring. -/// Bumped to power-of-two rungs (200, 400, 800, 2048, 8192) so the -/// in-tree `algebra::directory` continues to recognise the parameter. -fn kll_k_for_eps(eps: f64) -> u32 { - if eps <= 0.0 { - return 8192; - } - if eps >= 0.01 { - 200 - } else if eps >= 0.005 { - 400 - } else if eps >= 0.0025 { - 800 - } else if eps >= 0.001 { - 2048 - } else { - 8192 - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn k_at_canonical_rungs() { - assert_eq!(kll_k_for_eps(0.01), 200); - assert_eq!(kll_k_for_eps(0.005), 400); - assert_eq!(kll_k_for_eps(0.001), 2048); - assert_eq!(kll_k_for_eps(0.0001), 8192); - } -} diff --git a/control_plane/src/sketch_algebra/rules/mod.rs b/control_plane/src/sketch_algebra/rules/mod.rs index 2de70607..61da04b0 100644 --- a/control_plane/src/sketch_algebra/rules/mod.rs +++ b/control_plane/src/sketch_algebra/rules/mod.rs @@ -1,47 +1,31 @@ -//! Layer 4 `Bind*` rules — transform L3 [`QueryExpr`] sub-trees into L4 -//! [`PhysicalExpr`] sub-trees. +//! `Rule` — the `OptimizerRule`-blanket-impl extension point +//! `optimizer::mod`'s `RuleCategory::Bind` coverage was originally built +//! around. //! -//! Per `control_plane/docs/design.md` §6 ("`core::optimizer` — Layer 4 -//! framework", around line ~689) and §6 sketch_algebra (line ~565). A -//! `Bind*` rule: -//! -//! 1. Pattern-matches on a `QueryExpr::Aggregate` shape. -//! 2. Reads the [`AccuracyTarget`] off the matched intent. -//! 3. Consults the catalog (here: the family-default capability flags in -//! [`crate::sketch_algebra::schema::SketchStateSchema::for_kind`]). -//! 4. Returns `Some(PhysicalExpr)` if it can bind, `None` otherwise. -//! -//! Rule selection is cost-aware: when multiple rules match (e.g. KLL vs -//! DDSketch on a `Quantile` intent), the dispatcher picks one by -//! consulting per-rule [`Rule::priority`] + the accuracy-driven hints -//! returned by [`Rule::cost_hint`]. The `cost_hint` is intentionally -//! coarse for Phase C — the cost-model integration is Phase F's domain. -//! -//! Each rule documents the binding with a comment referencing -//! `accuracy_profile.rs` (in ASAPQuery-backend) for the formal accuracy -//! bound that justifies the chosen parameter mapping. +//! Step B of the plan-shaped-serving migration retired the seven +//! `Bind*`-rule-struct implementors (`bind_kll_quantile` / +//! `bind_ddsketch_quantile` / `bind_hll_cardinality` / `bind_cms_count` / +//! `bind_cms_topk` / `bind_exact_agg` / `bind_archive_only`) in favor of +//! `asap_plan::bind::implement_tree_in_with` + +//! `crate::sketch_algebra::cost_model::ControlPlaneCostModel` — see +//! `sketch_algebra::lower`. The trait itself stays: `optimizer::mod`'s +//! blanket `impl OptimizerRule for R` still needs it to exist, +//! and it remains a reasonable extension point for any future +//! deployment-specific L3→L4 rule that doesn't fit the `CostModel` +//! `rank_candidates`/`size_params` shape (e.g. a rule that rewrites +//! *through* a logical parent, which `implement_tree_in_with` deliberately +//! doesn't attempt — see its module docs' "conservative fallbacks"). #![allow(dead_code)] -pub mod bind_archive_only; -pub mod bind_cms_count; -pub mod bind_cms_topk; -pub mod bind_ddsketch_quantile; -pub mod bind_exact_agg; -pub mod bind_hll_cardinality; -pub mod bind_kll_quantile; - use crate::intent_algebra::QueryExpr; use crate::sketch_algebra::physical_expr::PhysicalExpr; use crate::types_v2::AccuracyTarget; -/// Bind-rule trait. Phase C keeps the trait minimal — `apply` + a -/// `priority` for the dispatcher's tie-break + a `cost_hint` (intent + -/// accuracy → relative cost). Phase F wires `cost_hint` into the -/// per-deployment cost model. +/// Bind-rule trait. See module docs. pub trait Rule: Sync { - /// Stable, human-readable identifier (`"bind_kll_quantile"`, …) — - /// used for diagnostics + the `cargo test` matrix. + /// Stable, human-readable identifier — used for diagnostics + the + /// `cargo test` matrix. fn name(&self) -> &'static str; /// Lower the matched sub-tree under the given accuracy target. Return @@ -54,37 +38,3 @@ pub trait Rule: Sync { 0 } } - -/// Dispatch a `QueryExpr` sub-tree against the full Phase C rule set. -/// Returns the highest-priority binding that fires, or `None` if no rule -/// matches (caller wraps the input in `PhysicalExpr::Logical`). -pub fn dispatch(expr: &QueryExpr, accuracy: &AccuracyTarget) -> Option { - let rules: Vec> = vec![ - Box::new(bind_kll_quantile::BindKllOnQuantile), - Box::new(bind_ddsketch_quantile::BindDDSketchOnQuantile), - Box::new(bind_cms_count::BindCmsOnCount), - Box::new(bind_cms_topk::BindCountSketchOnTopK), - Box::new(bind_hll_cardinality::BindHllOnCardinality), - // PR-6 follow-up: ExactAgg routing for Sum / Rate / Increase / - // Count{Exact}. Priority 2 — above archive-only, below sketch - // families. When this matches alongside `bind_archive_only` - // (Sum without an explicit accuracy override etc.), ExactAgg - // wins; matches `capability_for`'s analyzer-side direction. - Box::new(bind_exact_agg::BindExactAgg), - // Phase β: archive-only catch-all. Lowest priority — fires only - // when no ASAP-tier rule matches AND the intent is archive-only. - Box::new(bind_archive_only::BindArchiveOnly), - ]; - - let mut best: Option<(u16, PhysicalExpr)> = None; - for r in &rules { - if let Some(out) = r.apply(expr, accuracy) { - let p = r.priority(); - best = match best { - Some((bp, _)) if bp >= p => best, - _ => Some((p, out)), - }; - } - } - best.map(|(_, e)| e) -} diff --git a/control_plane/src/sketch_algebra/tests.rs b/control_plane/src/sketch_algebra/tests.rs index 9a206528..347ce035 100644 --- a/control_plane/src/sketch_algebra/tests.rs +++ b/control_plane/src/sketch_algebra/tests.rs @@ -1,18 +1,18 @@ -//! Integration tests for the L4 IR + `Bind*` rules. +//! Integration tests for the L4 IR + L3→L4 binding. #![cfg(test)] +use std::rc::Rc; use std::time::Duration; +use asap_sketch::{L4Node, SketchQuery, SummaryExpr, SummaryKind, SummaryParams}; + use crate::intent_algebra::schema::{Column, DataType}; -use crate::intent_algebra::{AggIntent, LabelFilter, QueryExpr, Schema, Source, WindowKind}; +use crate::intent_algebra::{AggIntent, BindingScope, LabelFilter, QueryExpr, Schema, Source, WindowKind}; +use crate::sketch_algebra::cost_model::ForcedFamilyCostModel; use crate::sketch_algebra::lower::bind_query_expr; -use crate::sketch_algebra::physical_expr::{EstimateOp, MergeAlgebra, PhysicalExpr}; -use crate::sketch_algebra::rules::{ - bind_ddsketch_quantile::BindDDSketchOnQuantile, bind_kll_quantile::BindKllOnQuantile, Rule, -}; -use crate::types_v2::{AccuracyTarget, BindingName}; -use asap_sketch::{SummaryKind, SummaryParams}; +use crate::sketch_algebra::physical_expr::{L4Plan, PhysicalExpr}; +use crate::types_v2::AccuracyTarget; // ── Test fixtures ───────────────────────────────────────────────────────────── @@ -73,55 +73,113 @@ fn agg_quantile(q: f64, accuracy: AccuracyTarget) -> QueryExpr { } } +/// Walk a `PhysicalExpr` tree and report whether any node's `SummaryExpr` +/// wraps an archive-only `AggIntent` in an unbound `Logical(Aggregate)`. +/// Mirrors `emit::mod.rs`'s classification: `Logical`-wrapped aggregates +/// whose sole intent is `archive_only()` route the L5 emitter to the +/// cold-store tier. +fn binding_is_archive(expr: &PhysicalExpr) -> bool { + match expr { + PhysicalExpr::Committed(plan) => plan_is_archive(plan), + PhysicalExpr::RawAtEdgeSketchAtBackend { child, .. } => plan_is_archive(child), + PhysicalExpr::RawAtEdgePrometheusArchive { .. } => false, + } +} + +fn plan_is_archive(plan: &L4Plan) -> bool { + match plan { + L4Plan::Summary(node) => node_is_archive(node), + L4Plan::LetBinding { expr, child, .. } => plan_is_archive(expr) || plan_is_archive(child), + L4Plan::Ref { .. } => false, + } +} + +fn node_is_archive(node: &Rc) -> bool { + match &node.expr { + SummaryExpr::Logical(qe) => match qe.as_ref() { + QueryExpr::Aggregate { aggs, .. } => { + aggs.iter().any(crate::intent_algebra::archive_only) + } + _ => false, + }, + SummaryExpr::SummaryAgg { child, .. } => node_is_archive(child), + SummaryExpr::SummaryEstimate { sketch_input, .. } => node_is_archive(sketch_input), + SummaryExpr::SummaryMerge { children } => children.iter().any(node_is_archive), + SummaryExpr::SummaryJoin { outer, inner, .. } => { + node_is_archive(outer) || node_is_archive(inner) + } + SummaryExpr::SummarySubtract { left, right } => { + node_is_archive(left) || node_is_archive(right) + } + SummaryExpr::SummaryDelete { sketch_input, .. } => node_is_archive(sketch_input), + } +} + // ── Bind rule tests ─────────────────────────────────────────────────────────── #[test] fn bind_kll_quantile_basic() { - // The KLL rule on its own (priority 5) — DDSketch (priority 6) wins - // the dispatcher tie-break, so test the KLL rule's `apply` directly. + // The retired `BindKllOnQuantile` rule struct's direct `.apply()` + // call is replaced by forcing the Kll family via + // `ForcedFamilyCostModel` — bypassing the DDSketch/KLL dispatcher + // tie-break exercised separately by + // `bind_picks_ddsketch_over_kll_when_eps_explicit` below. let expr = agg_quantile(0.99, AccuracyTarget::Epsilon(0.01)); - let bound = BindKllOnQuantile - .apply(&expr, &AccuracyTarget::Epsilon(0.01)) - .expect("KLL rule should bind a Quantile{0.99, ε=0.01}"); - match bound { - PhysicalExpr::SketchEstimate { op, child } => { - assert_eq!(op, EstimateOp::Quantile { q: 0.99 }); - match *child { - PhysicalExpr::SketchAgg { - sketch_type, + let cost_model = ForcedFamilyCostModel::new(AccuracyTarget::Epsilon(0.01), SummaryKind::Kll); + let node = asap_plan::bind::implement_tree_in_with( + &expr, + &BindingScope::default(), + &cost_model, + ) + .expect("KLL should bind a Quantile{0.99, ε=0.01}"); + match &node.expr { + SummaryExpr::SummaryEstimate { + query, + sketch_input, + } => { + assert!(matches!(query, SketchQuery::Quantile { q } if *q == 0.99)); + match &sketch_input.expr { + SummaryExpr::SummaryAgg { + sketch, params, child, + .. } => { - assert_eq!(sketch_type, SummaryKind::Kll); - assert_eq!(params, SummaryParams::Kll { k: 200 }); - assert!(matches!( - *child, - PhysicalExpr::Logical(QueryExpr::Window { .. }) - )); + assert_eq!(sketch, &SummaryKind::Kll); + assert_eq!(params, &SummaryParams::Kll { k: 200 }); + assert!(matches!(child.expr, SummaryExpr::Logical(_))); } - other => panic!("expected SketchAgg, got {other:?}"), + other => panic!("expected SummaryAgg, got {other:?}"), } } - other => panic!("expected SketchEstimate, got {other:?}"), + other => panic!("expected SummaryEstimate, got {other:?}"), } } #[test] fn bind_ddsketch_quantile_basic() { + // Same shape as `bind_kll_quantile_basic`, forcing DDSketch instead + // of the retired `BindDDSketchOnQuantile` rule struct. let expr = agg_quantile(0.99, AccuracyTarget::Epsilon(0.01)); - let bound = BindDDSketchOnQuantile - .apply(&expr, &AccuracyTarget::Epsilon(0.01)) - .expect("DDSketch rule should bind a Quantile{0.99, ε=0.01}"); - match bound { - PhysicalExpr::SketchEstimate { op, child } => { - assert_eq!(op, EstimateOp::Quantile { q: 0.99 }); - match *child { - PhysicalExpr::SketchAgg { - sketch_type, - params, - .. + let cost_model = + ForcedFamilyCostModel::new(AccuracyTarget::Epsilon(0.01), SummaryKind::DDSketch); + let node = asap_plan::bind::implement_tree_in_with( + &expr, + &BindingScope::default(), + &cost_model, + ) + .expect("DDSketch should bind a Quantile{0.99, ε=0.01}"); + match &node.expr { + SummaryExpr::SummaryEstimate { + query, + sketch_input, + } => { + assert!(matches!(query, SketchQuery::Quantile { q } if *q == 0.99)); + match &sketch_input.expr { + SummaryExpr::SummaryAgg { + sketch, params, .. } => { - assert_eq!(sketch_type, SummaryKind::DDSketch); + assert_eq!(sketch, &SummaryKind::DDSketch); match params { SummaryParams::DDSketch { alpha } => { assert!((alpha - 0.01).abs() < 1e-12) @@ -129,34 +187,37 @@ fn bind_ddsketch_quantile_basic() { other => panic!("expected DDSketch params, got {other:?}"), } } - other => panic!("expected SketchAgg, got {other:?}"), + other => panic!("expected SummaryAgg, got {other:?}"), } } - other => panic!("expected SketchEstimate, got {other:?}"), + other => panic!("expected SummaryEstimate, got {other:?}"), } } /// Cost-aware rule selection: the dispatcher should pick DDSketch over -/// KLL for an explicit ε-driven Quantile because DDSketch has higher -/// `priority()` (6 vs 5) — that matches the legacy -/// `algebra::directory::sketch_type_for_agg` default for SP-2/SP-4. +/// KLL for an explicit ε-driven Quantile — `ControlPlaneCostModel::rank_candidates` +/// statically reorders DDSketch first (see its doc comment), matching the +/// legacy `algebra::directory::sketch_type_for_agg` default for SP-2/SP-4. #[test] fn bind_picks_ddsketch_over_kll_when_eps_explicit() { let expr = agg_quantile(0.99, AccuracyTarget::Epsilon(0.01)); let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)) .expect("bind_query_expr should not error"); match bound { - PhysicalExpr::SketchEstimate { child, .. } => match *child { - PhysicalExpr::SketchAgg { sketch_type, .. } => { - assert_eq!( - sketch_type, - SummaryKind::DDSketch, - "dispatcher should pick DDSketch (priority 6) over KLL (priority 5) on ε-driven Quantile" - ); - } - other => panic!("expected SketchAgg, got {other:?}"), + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::SummaryEstimate { sketch_input, .. } => match &sketch_input.expr { + SummaryExpr::SummaryAgg { sketch, .. } => { + assert_eq!( + sketch, + &SummaryKind::DDSketch, + "dispatcher should pick DDSketch over KLL on ε-driven Quantile" + ); + } + other => panic!("expected SummaryAgg, got {other:?}"), + }, + other => panic!("expected SummaryEstimate, got {other:?}"), }, - other => panic!("expected SketchEstimate, got {other:?}"), + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } @@ -173,33 +234,37 @@ fn agg_topk(k: usize, accuracy: AccuracyTarget) -> QueryExpr { /// Pull the bound `(SummaryKind, w, d)` out of a top-k binding. /// `SummaryKind` (unlike the retired `sketch_algebra::SketchKind`) -/// promotes `with_heap` to kind identity — `bind_cms_topk` always binds -/// `CmsWithHeap`/`CountSketchWithHeap` for a top-k intent, never the -/// bare kind, so there's no separate heap flag to return anymore. +/// promotes `with_heap` to kind identity — the top-k cost model always +/// binds `CmsWithHeap`/`CountSketchWithHeap` for a top-k intent, never +/// the bare kind, so there's no separate heap flag to return anymore. fn topk_binding_family(bound: &PhysicalExpr) -> (SummaryKind, u32, u32) { match bound { - PhysicalExpr::SketchEstimate { op, child } => { - assert_eq!(*op, EstimateOp::TopK { k: 10 }); - match &**child { - PhysicalExpr::SketchAgg { - sketch_type, - params, - .. - } => match params { - SummaryParams::CmsWithHeap { width, depth, .. } => { - (sketch_type.clone(), *width, *depth) - } - SummaryParams::CountSketchWithHeap { width, depth, .. } => { - (sketch_type.clone(), *width, *depth) - } - other => { - panic!("expected CmsWithHeap/CountSketchWithHeap params, got {other:?}") - } - }, - other => panic!("expected SketchAgg, got {other:?}"), + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::SummaryEstimate { + query, + sketch_input, + } => { + assert!(matches!(query, SketchQuery::TopK { k } if *k == 10)); + match &sketch_input.expr { + SummaryExpr::SummaryAgg { + sketch, params, .. + } => match params { + SummaryParams::CmsWithHeap { width, depth, .. } => { + (sketch.clone(), *width, *depth) + } + SummaryParams::CountSketchWithHeap { width, depth, .. } => { + (sketch.clone(), *width, *depth) + } + other => { + panic!("expected CmsWithHeap/CountSketchWithHeap params, got {other:?}") + } + }, + other => panic!("expected SummaryAgg, got {other:?}"), + } } - } - other => panic!("expected SketchEstimate, got {other:?}"), + other => panic!("expected SummaryEstimate, got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } @@ -224,14 +289,30 @@ fn bind_cms_topk_loose_recall_picks_cms_heap() { assert!(d >= 1); } -/// (b) A **tight / exact-recall** top-k (the intent carries -/// `AccuracyTarget::Exact`) binds the unbiased **CountSketch-with-heap** — -/// the family that supports exact rank / signed estimates. +/// (b) A **tight / exact-recall** top-k binds the unbiased +/// **CountSketch-with-heap** — the family that supports exact rank / +/// signed estimates. +/// +/// NOTE — behavior change forced by the new binder, not just a rename: +/// the old fixture used `AggIntent::TopK{accuracy: Exact}` (the intent's +/// OWN accuracy) to signal "tight/exact-recall". Under +/// `asap_plan::boundary::implementation_for_with`, the per-intent +/// sketch-vs-exact boundary decision checks the intent's own `accuracy` +/// field FIRST: `TopK{accuracy: Exact}` now declines to bind at all +/// (`SummaryExpr::Logical`) rather than reaching the cost model's +/// family-selection logic at all — see `topk_exact_accuracy_declines_to_bind` +/// above (a REAL, accepted behavior change — ASAPController#151 — per +/// this migration's design notes, not a bug to route around). "Tight +/// recall" (→ CountSketchWithHeap) is still live logic in +/// `ControlPlaneCostModel::topk_family_order` — it fires off the +/// WORKLOAD-level accuracy (not the intent's own) being `Exact`, which +/// still lets the intent itself bind. #[test] fn bind_cms_topk_tight_recall_picks_countsketch() { - // Intent requests Exact rank; the policy-level target is non-exact. - let expr = agg_topk(10, AccuracyTarget::Exact); - let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)) + // Intent requests a normal (non-exact) rank so binding still + // happens; the workload-level policy demands exact recall. + let expr = agg_topk(10, AccuracyTarget::Epsilon(0.01)); + let bound = bind_query_expr(&expr, AccuracyTarget::Exact) .expect("bind_query_expr should not error"); let (kind, w, d) = topk_binding_family(&bound); assert_eq!( @@ -292,39 +373,47 @@ fn bind_hll_cardinality_basic() { }; let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)).expect("no error"); match bound { - PhysicalExpr::SketchEstimate { op, child } => { - assert_eq!(op, EstimateOp::Cardinality); - match *child { - PhysicalExpr::SketchAgg { - sketch_type, - params, - .. - } => { - assert_eq!(sketch_type, SummaryKind::Hll); - match params { - SummaryParams::Hll { precision } => { - assert!( - precision >= 12, - "ε=0.01 should land on at least precision 12 (~1.6%) per the rung table" - ); + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::SummaryEstimate { + query, + sketch_input, + } => { + assert!(matches!(query, SketchQuery::Cardinality)); + match &sketch_input.expr { + SummaryExpr::SummaryAgg { + sketch, params, .. + } => { + assert_eq!(sketch, &SummaryKind::Hll); + match params { + SummaryParams::Hll { precision } => { + assert!( + *precision >= 12, + "ε=0.01 should land on at least precision 12 (~1.6%) per the rung table" + ); + } + other => panic!("expected HllParams, got {other:?}"), } - other => panic!("expected HllParams, got {other:?}"), } + other => panic!("expected SummaryAgg, got {other:?}"), } - other => panic!("expected SketchAgg, got {other:?}"), } - } - other => panic!("expected SketchEstimate, got {other:?}"), + other => panic!("expected SummaryEstimate, got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } #[test] fn sum_now_binds_to_exact_agg_after_pr_6_followup() { - // Pre-PR-6-follow-up: `Sum` had no `Bind*` rule and passed through - // as `PhysicalExpr::Logical`. The L4 binder rule `BindExactAgg` - // (added in the PR-6 follow-up) now matches and emits - // `PhysicalExpr::ExactAgg { agg_type: Sum, .. }` so the ASAP-tier - // exact-aggregation path can serve the intent. + // `AggIntent::Sum` binds to a bare `SummaryAgg` with `sketch: + // SummaryKind::Sum` and no `SummaryEstimate` wrapper (the partial + // state *is* the value — see `asap_plan::bind`'s module docs). The + // old locally-defined `PhysicalExpr::ExactAgg { agg_type, .. }` + // variant (and `asap_types::AggregationType`) no longer exist at + // the L4 IR level: `asap_sketch::SummaryExpr` unifies exact + // accumulators and approximate sketches into the same `SummaryAgg` + // node shape, keyed by `SummaryKind` (see `physical_expr.rs`'s + // module docs). let expr = QueryExpr::Aggregate { by: vec![].into(), aggs: vec![AggIntent::Sum { col: None }], @@ -334,12 +423,14 @@ fn sum_now_binds_to_exact_agg_after_pr_6_followup() { }; let bound = bind_query_expr(&expr, AccuracyTarget::Exact).expect("no error"); match bound { - PhysicalExpr::ExactAgg { agg_type, .. } => assert_eq!( - agg_type, - asap_types::AggregationType::Sum, - "Sum should bind to ExactAgg(Sum)" - ), - other => panic!("expected ExactAgg, got {other:?}"), + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::SummaryAgg { sketch, params, .. } => { + assert_eq!(sketch, &SummaryKind::Sum, "Sum should bind to SummaryAgg(Sum)"); + assert_eq!(params, &SummaryParams::Sum); + } + other => panic!("expected bare SummaryAgg(Sum), got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } @@ -350,10 +441,16 @@ fn bind_exact_accuracy_disables_quantile_binding() { // ~1254 — "the sketch path is selected, not mandated".) let expr = agg_quantile(0.99, AccuracyTarget::Exact); let bound = bind_query_expr(&expr, AccuracyTarget::Exact).expect("no error"); - assert!( - matches!(bound, PhysicalExpr::Logical(QueryExpr::Aggregate { .. })), - "Exact accuracy should disable sketch binding and pass through as Logical" - ); + match bound { + PhysicalExpr::Committed(L4Plan::Summary(node)) => { + assert!( + matches!(&node.expr, SummaryExpr::Logical(qe) if matches!(**qe, QueryExpr::Aggregate { .. })), + "Exact accuracy should disable sketch binding and pass through as Logical, got {:?}", + node.expr + ); + } + other => panic!("expected Committed(Summary(_)), got {other:?}"), + } } // ── Phase β: pattern-migration coverage ─────────────────────────────────────── @@ -366,7 +463,7 @@ fn bind_exact_accuracy_disables_quantile_binding() { /// `ONLY_TEMPORAL` — `quantile_over_time(0.99, m[5m])`. /// asap-planner-rs path: ONLY_TEMPORAL pattern 1 → KLL/DDSketch sketch. /// Control plane path: `Aggregate{Quantile{0.99}}` over `Window` → -/// `BindKllOnQuantile` (or DDSketch) → `SketchAgg{KLL/DDSketch}`. +/// binds a quantile-capable family → `SummaryAgg{KLL/DDSketch}`. #[test] fn phase_b_pattern_only_temporal_quantile_binds_to_sketch() { let expr = QueryExpr::Aggregate { @@ -382,29 +479,35 @@ fn phase_b_pattern_only_temporal_quantile_binds_to_sketch() { }; let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)).unwrap(); match bound { - PhysicalExpr::SketchEstimate { op, child } => { - assert!(matches!(op, EstimateOp::Quantile { .. })); - match *child { - PhysicalExpr::SketchAgg { sketch_type, .. } => { - assert!(matches!( - sketch_type, - SummaryKind::Kll | SummaryKind::DDSketch - )); + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::SummaryEstimate { + query, + sketch_input, + } => { + assert!(matches!(query, SketchQuery::Quantile { .. })); + match &sketch_input.expr { + SummaryExpr::SummaryAgg { sketch, .. } => { + assert!(matches!( + sketch, + SummaryKind::Kll | SummaryKind::DDSketch + )); + } + other => panic!("expected SummaryAgg under SummaryEstimate, got {other:?}"), } - other => panic!("expected SketchAgg under SketchEstimate, got {other:?}"), } - } - other => panic!("expected SketchEstimate, got {other:?}"), + other => panic!("expected SummaryEstimate, got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } /// `ONLY_TEMPORAL` — `sum_over_time(m[5m])` (and the count/avg/min/max /// variants that legacy `single_query.rs` accepts). /// -/// Control plane path (post PR-6 follow-up): `Aggregate{Sum}` over -/// `Window` → `BindExactAgg` fires → `PhysicalExpr::ExactAgg{Sum}`. -/// Pre-follow-up this fell through to `Logical` because no rule -/// matched Sum; the L5 emitter routed it to the archive engine. +/// Control plane path: `Aggregate{Sum}` over `Window` → binds to a bare +/// `SummaryAgg{sketch: SummaryKind::Sum}` (an exact mergeable +/// accumulator — see `sum_now_binds_to_exact_agg_after_pr_6_followup`'s +/// doc comment for the `ExactAgg` → `SummaryAgg` unification). #[test] fn phase_b_pattern_only_temporal_sum_binds_to_exact_agg() { let expr = QueryExpr::Aggregate { @@ -416,19 +519,24 @@ fn phase_b_pattern_only_temporal_sum_binds_to_exact_agg() { }; let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)).unwrap(); match bound { - PhysicalExpr::ExactAgg { agg_type, .. } => { - assert_eq!(agg_type, asap_types::AggregationType::Sum,) - } - other => panic!("expected ExactAgg(Sum), got {other:?}"), + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::SummaryAgg { sketch, .. } => { + assert_eq!(sketch, &SummaryKind::Sum); + } + other => panic!("expected SummaryAgg(Sum), got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } /// `ONLY_SPATIAL` — `sum by (host) (m)`. /// Control plane path: `Aggregate{Sum, by=[host]}` over a bare `Scan`. -/// Post keyed-ExactAgg follow-up, `BindExactAgg` lowers this to -/// `PhysicalExpr::ExactAgg { agg_type: MultipleSum, .. }` — the -/// multi-pop accumulator family the data plane uses for keyed -/// per-group sums. +/// +/// The old locally-defined `AggregationType::MultipleSum` (keyed vs +/// unkeyed sum) identity no longer exists at the L4 IR level — +/// `SummaryKind::Sum` covers both; the keyed/unkeyed distinction now +/// lives on `SummaryAgg::by` (non-empty ⇒ the old "MultipleSum" shape), +/// per `emit::mod.rs`'s exact-accumulator classification notes. #[test] fn phase_b_pattern_only_spatial_aggregate_binds_to_multiple_sum() { let expr = QueryExpr::Aggregate { @@ -440,16 +548,28 @@ fn phase_b_pattern_only_spatial_aggregate_binds_to_multiple_sum() { }; let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)).unwrap(); match bound { - PhysicalExpr::ExactAgg { agg_type, .. } => { - assert_eq!(agg_type, asap_types::AggregationType::MultipleSum,) - } - other => panic!("expected ExactAgg(MultipleSum), got {other:?}"), + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::SummaryAgg { sketch, by, .. } => { + assert_eq!(sketch, &SummaryKind::Sum); + assert_eq!( + by, + &vec![1], + "keyed sum must carry the group-by column (the MultipleSum-equivalent signal)" + ); + } + other => panic!("expected SummaryAgg(Sum, by=[1]), got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } /// `ONE_TEMPORAL_ONE_SPATIAL` — `sum by (host) (rate(m[5m]))`. -/// Post keyed-ExactAgg follow-up: `Rate` keyed by `host` lowers to -/// `MultipleIncrease`. +/// `bind_query_expr` (not `implement_tree` directly) rewrites +/// `AggIntent::Rate` to `AggIntent::Increase` before binding (see +/// `lower.rs`'s `rewrite_rate_to_increase` — this deployment's data +/// plane has no Rate accumulator). The old +/// `AggregationType::MultipleIncrease` identity is now +/// `SummaryKind::Increase` with a non-empty `by`. #[test] fn phase_b_pattern_temporal_and_spatial_combined_binds_to_multiple_increase() { let expr = QueryExpr::Aggregate { @@ -461,18 +581,21 @@ fn phase_b_pattern_temporal_and_spatial_combined_binds_to_multiple_increase() { }; let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)).unwrap(); match bound { - PhysicalExpr::ExactAgg { agg_type, .. } => { - assert_eq!(agg_type, asap_types::AggregationType::MultipleIncrease,) - } - other => panic!("expected ExactAgg(MultipleIncrease), got {other:?}"), + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::SummaryAgg { sketch, by, .. } => { + assert_eq!(sketch, &SummaryKind::Increase); + assert_eq!(by, &vec![1]); + } + other => panic!("expected SummaryAgg(Increase, by=[1]), got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } /// Phase β archive-only intent: any of the no-ASAP-tier-family entries -/// (`Absent`, `Present`, `Delta`, …) matches `BindArchiveOnly` → `Logical` -/// pass-through, and the L5 emitter / Phase α routing reads -/// `AggIntent::archive_only() == true` to flag the StreamingConfig entry -/// for the archive tier. +/// (`Absent`, `Present`, `Delta`, …) binds to a `Logical` pass-through, +/// and the L5 emitter / Phase α routing reads `AggIntent::archive_only() +/// == true` to flag the StreamingConfig entry for the archive tier. /// /// `histogram_quantile(...)` was previously an L3 intent here but is no /// longer — it's a PromQL/MetricsQL language-level operator that the @@ -498,10 +621,16 @@ fn phase_b_pattern_archive_only_routes_to_archive() { // The archive-only rule's output is a Logical pass-through carrying // the original Aggregate. Downstream emitters check archive_only(). match bound { - PhysicalExpr::Logical(QueryExpr::Aggregate { aggs, .. }) => { - assert_eq!(aggs, vec![intent]); - } - other => panic!("expected Logical(Aggregate(Absent)), got {other:?}"), + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::Logical(qe) => match qe.as_ref() { + QueryExpr::Aggregate { aggs, .. } => { + assert_eq!(aggs, &vec![intent]); + } + other => panic!("expected Aggregate, got {other:?}"), + }, + other => panic!("expected Logical(Aggregate(Absent)), got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } @@ -530,95 +659,21 @@ fn pipeline_l1_to_l4(query: &str, accuracy: AccuracyTarget) -> PhysicalExpr { bind_query_expr(&qe, accuracy).unwrap_or_else(|e| panic!("bind {query}: {e}")) } -/// Walk a `PhysicalExpr` and collect every `SketchAgg`'s sketch_kind. The -/// number of entries + the kind set is the wire-equivalent of -/// asap-planner-rs's "aggregation_id rows in StreamingConfig output". -fn collect_sketch_kinds(expr: &PhysicalExpr) -> Vec { - let mut out = Vec::new(); - fn walk(e: &PhysicalExpr, out: &mut Vec) { - match e { - PhysicalExpr::SketchAgg { - sketch_type, child, .. - } => { - out.push(sketch_type.clone()); - walk(child, out); - } - PhysicalExpr::SketchEstimate { child, .. } => walk(child, out), - PhysicalExpr::SketchMerge { children, .. } => { - for c in children { - walk(c, out); - } - } - PhysicalExpr::LetBinding { expr, child, .. } => { - walk(expr, out); - walk(child, out); - } - PhysicalExpr::Logical(_) | PhysicalExpr::Ref { .. } => {} - // Phase ε.1 — the new placement variants don't carry a - // SketchAgg child the legacy walk recognises. Mode 2 records - // its own family directly; Mode 3 has no sketch at all. - PhysicalExpr::RawAtEdgeSketchAtBackend { family, child, .. } => { - out.push(family.clone()); - walk(child, out); - } - PhysicalExpr::RawAtEdgePrometheusArchive { .. } => {} - // ExactAgg has no SummaryKind to collect; its child may - // carry one transitively (rare but possible if nested). - PhysicalExpr::ExactAgg { child, .. } => walk(child, out), - } - } - walk(expr, &mut out); - out -} - -/// Walk a `PhysicalExpr` and detect whether the binding ended in a -/// `Logical`-wrapped `Aggregate` carrying an archive-only intent. This is -/// the L4 signal that the L5 emitter routes the StreamingConfig entry -/// to the cold tier rather than the warm one. -fn binding_is_archive(expr: &PhysicalExpr) -> bool { - match expr { - PhysicalExpr::Logical(QueryExpr::Aggregate { aggs, .. }) => { - aggs.iter().any(crate::intent_algebra::archive_only) - } - PhysicalExpr::Logical(_) => false, - PhysicalExpr::SketchEstimate { child, .. } => binding_is_archive(child), - PhysicalExpr::SketchAgg { child, .. } => binding_is_archive(child), - PhysicalExpr::SketchMerge { children, .. } => children.iter().any(binding_is_archive), - PhysicalExpr::LetBinding { expr, child, .. } => { - binding_is_archive(expr) || binding_is_archive(child) - } - PhysicalExpr::Ref { .. } => false, - // Phase ε.1 — Mode 3 routes to the prometheus_remote engine - // (its own engine ID), which the L5 emitter handles via - // emit_backend_storage_routing rather than the warm-vs-archive - // gate this helper guards. Treat as not-archive: this helper is - // about cold-tier scan-vs-ASAP-tier-sketch decisions, not Mode 3. - PhysicalExpr::RawAtEdgeSketchAtBackend { child, .. } => binding_is_archive(child), - PhysicalExpr::RawAtEdgePrometheusArchive { .. } => false, - // ExactAgg is a ASAP-tier exact-aggregation accumulator, NOT - // an archive route. The L5 emitter writes the result through - // the precompute output sink, same path as SketchAgg. - PhysicalExpr::ExactAgg { .. } => false, - } -} - /// `quantile_over_time.yaml` — the asap-planner-rs `quantile_over_time` /// fixture maps to a KLL or DDSketch StreamingConfig row. The control plane /// path: L1 PromQL parse → L3 `Aggregate{Quantile{0.99}}` over `Window` → -/// L4 `BindKllOnQuantile` (default) or `BindDDSketchOnQuantile`. Either -/// is functionally equivalent — both are quantile sketches. +/// L4 bind picks Kll (default) or DDSketch. Either is functionally +/// equivalent — both are quantile sketches. #[test] fn phase_b_e2e_quantile_over_time_binds_to_quantile_sketch() { let bound = pipeline_l1_to_l4( "quantile_over_time(0.99, http_request_duration_seconds[5m])", AccuracyTarget::Epsilon(0.01), ); - let kinds = collect_sketch_kinds(&bound); - assert_eq!(kinds.len(), 1, "expected 1 sketch agg, got {kinds:?}"); + let kind = crate::emit::extract_root_sketch_kind(&bound); assert!( - matches!(kinds[0], SummaryKind::Kll | SummaryKind::DDSketch), - "expected quantile sketch family, got {:?}", - kinds[0] + matches!(kind, Some(SummaryKind::Kll) | Some(SummaryKind::DDSketch)), + "expected quantile sketch family, got {kind:?}" ); assert!( !binding_is_archive(&bound), @@ -628,41 +683,43 @@ fn phase_b_e2e_quantile_over_time_binds_to_quantile_sketch() { /// `sum_over_time.yaml` — the legacy planner produces an exact-sum /// aggregation row (no sketch). Control plane path: `Aggregate{Sum}` over -/// `Window` → no ASAP-tier rule fires → `Logical` pass-through. -/// Functional equivalence: both produce a single non-sketch row. +/// `Window` → binds to an exact accumulator (`SummaryAgg{Sum}`), which is +/// neither an approximate sketch (so `extract_root_sketch_kind`, which +/// excludes exact accumulators — see its doc comment — returns `None`) +/// nor archive-routed. #[test] fn phase_b_e2e_sum_over_time_falls_through_to_logical() { let bound = pipeline_l1_to_l4( "sum_over_time(http_requests_total[5m])", AccuracyTarget::Epsilon(0.01), ); - let kinds = collect_sketch_kinds(&bound); assert!( - kinds.is_empty(), - "sum_over_time should not produce a sketch agg, got {kinds:?}" + crate::emit::extract_root_sketch_kind(&bound).is_none(), + "sum_over_time should not produce an approximate sketch" ); assert!( !binding_is_archive(&bound), - "Sum is exact-warm, not archive — bind output should stay Logical without archive flag" + "Sum is exact-warm, not archive" ); } /// `sum_by.yaml` — `sum by (label) (sum_over_time(...))`. Spatial-and- /// temporal aggregation; the legacy planner emits an exact-sum row keyed /// on the by-label. Control plane path: `Aggregate{Sum, by=[…]}` over -/// `Window` → no ASAP-tier rule fires → `Logical` pass-through. The -/// by-label is preserved on the L3 group-by-id list, which Phase α's -/// routing emit reads to build the per-label rollup partition. +/// `Window` → binds to an exact accumulator (`SummaryAgg{Sum, by=[…]}`) — +/// no approximate sketch family. The by-label is preserved on the L3 +/// group-by-id list, which Phase α's routing emit reads to build the +/// per-label rollup partition. #[test] fn phase_b_e2e_sum_by_preserves_grouping_label() { let bound = pipeline_l1_to_l4( "sum by (instance) (sum_over_time(http_requests_total[5m]))", AccuracyTarget::Epsilon(0.01), ); - // No sketch family for plain Sum. - assert!(collect_sketch_kinds(&bound).is_empty()); - // The end shape may be Logical(Aggregate{by, ...}) when the Aggregate - // node survives the lowering, or Logical(Window{...}) when the + // No approximate sketch family for plain Sum. + assert!(crate::emit::extract_root_sketch_kind(&bound).is_none()); + // The end shape may carry `Logical(Aggregate{by, ...})` beneath a + // `SummaryAgg{Sum}` wrapper, or `Logical(Window{...})` when the // ParsedQuery → QueryExpr lowering drops the Aggregate (legacy // ParsedQuery only carries `aggregations: Vec` not the // by-axis directly). In either case the metric name + label survive @@ -683,16 +740,17 @@ fn phase_b_e2e_sum_by_preserves_grouping_label() { /// `rate_increase.yaml` — the legacy planner emits a MultipleIncrease /// (counter-reset adjusted) row. Control plane path: `Aggregate{Rate}` over -/// `Window` → no streaming-rate sketch family today → `Logical`. Both -/// paths produce a single non-sketch streaming row; the L5 emitter is -/// the one that picks the actual MultipleIncrease processor. +/// `Window` → `bind_query_expr` rewrites `Rate` to `Increase` and binds an +/// exact accumulator (`SummaryAgg{Increase}`) — no approximate sketch +/// family. Both paths produce a single non-sketch streaming row; the L5 +/// emitter is the one that picks the actual MultipleIncrease processor. #[test] fn phase_b_e2e_rate_falls_through_to_logical() { let bound = pipeline_l1_to_l4( "rate(http_requests_total[5m])", AccuracyTarget::Epsilon(0.01), ); - assert!(collect_sketch_kinds(&bound).is_empty()); + assert!(crate::emit::extract_root_sketch_kind(&bound).is_none()); assert!( !binding_is_archive(&bound), "Rate is ASAP-tier, not archive" @@ -718,7 +776,7 @@ fn phase_b_e2e_topk_well_formed() { // Logical pass-through (engine handles it). Both are accepted L4 // shapes — Phase β's contract is just "doesn't panic, produces a // legitimate PhysicalExpr". - let _ = collect_sketch_kinds(&bound); + let _ = crate::emit::extract_root_sketch_kind(&bound); } /// Archive-only routing through the full L1→L3→L4 pipeline. Asserts the @@ -748,8 +806,8 @@ fn phase_b_e2e_archive_only_e2e_binding() { binding_is_archive(&bound), "archive-only intent must surface archive flag through L4 binding" ); - // No ASAP-tier sketch fires for archive-only intents. - assert!(collect_sketch_kinds(&bound).is_empty()); + // No approximate sketch fires for archive-only intents. + assert!(crate::emit::extract_root_sketch_kind(&bound).is_none()); } /// Cross-cutting: every Phase β archive-only intent reaches @@ -789,14 +847,72 @@ fn phase_b_archive_only_intents_round_trip_through_binder() { let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)).expect("bind should succeed"); match bound { - PhysicalExpr::Logical(QueryExpr::Aggregate { aggs, .. }) => { - assert_eq!(aggs.len(), 1); - assert!( - crate::intent_algebra::archive_only(&aggs[0]), - "{intent:?} should preserve archive_only() flag through bind" - ); - } - other => panic!("expected Logical(Aggregate({intent:?})), got {other:?}"), + PhysicalExpr::Committed(L4Plan::Summary(node)) => match &node.expr { + SummaryExpr::Logical(qe) => match qe.as_ref() { + QueryExpr::Aggregate { aggs, .. } => { + assert_eq!(aggs.len(), 1); + assert!( + crate::intent_algebra::archive_only(&aggs[0]), + "{intent:?} should preserve archive_only() flag through bind" + ); + } + other => panic!("expected Aggregate({intent:?}), got {other:?}"), + }, + other => panic!("expected Logical(Aggregate({intent:?})), got {other:?}"), + }, + other => panic!("expected Committed(Summary(_)) for {intent:?}, got {other:?}"), + } + } +} + +// ── Deliberate behavior changes (ASAPController#150 / #151) ────────────────── +// +// `AggIntent::Extension` (this deployment's `Frequency` point-query, +// built via `crate::intent_algebra::frequency(accuracy)`) and +// `AggIntent::TopK { accuracy: Exact }` both now decline to bind +// (`SummaryExpr::Logical`) rather than sketch — a REAL, accepted +// behavior change introduced by this migration (core's +// `boundary::implementation_for` maps every `Extension` to +// `PassThrough` unconditionally, and `TopK{Exact}`'s `exact_realization` +// has no accumulator form for it either — see `lower.rs`'s module docs +// and `cost_model.rs`'s module docs). Mirrors the pattern already used in +// `optimizer::rules::mod::tests::typed_binding_endpoint_request_freq_declines_pending_upstream_extension_support`. + +#[test] +fn frequency_extension_declines_pending_upstream_extension_support() { + let intent = crate::intent_algebra::frequency(AccuracyTarget::Epsilon(0.01)); + let expr = QueryExpr::Aggregate { + by: vec![].into(), + aggs: vec![intent], + output_names: Vec::new(), + having: None, + child: Box::new(windowed_scan()), + }; + let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)).expect("no error"); + match bound { + PhysicalExpr::Committed(L4Plan::Summary(node)) => { + assert!( + matches!(&node.expr, SummaryExpr::Logical(_)), + "Frequency (AggIntent::Extension) should decline pending ASAPController#150, got {:?}", + node.expr + ); + } + other => panic!("expected Committed(Summary(_)), got {other:?}"), + } +} + +#[test] +fn topk_exact_accuracy_declines_to_bind() { + let expr = agg_topk(10, AccuracyTarget::Exact); + let bound = bind_query_expr(&expr, AccuracyTarget::Exact).expect("no error"); + match bound { + PhysicalExpr::Committed(L4Plan::Summary(node)) => { + assert!( + matches!(&node.expr, SummaryExpr::Logical(_)), + "TopK{{accuracy: Exact}} should decline pending ASAPController#151, got {:?}", + node.expr + ); } + other => panic!("expected Committed(Summary(_)), got {other:?}"), } } diff --git a/crates/asap_types/Cargo.toml b/crates/asap_types/Cargo.toml index 51550581..c3b6b683 100644 --- a/crates/asap_types/Cargo.toml +++ b/crates/asap_types/Cargo.toml @@ -16,4 +16,4 @@ xxhash-rust = { version = "0.8", features = ["xxh64"] } # via this crate): WindowType -> asap_ir::intent_algebra::query_expr::WindowKind # unification (scratchpad/artifacts/enum-unification-plan.md). Pin matches # control_plane's -- see control_plane/Cargo.toml's comment for the rationale. -asap-ir = { git = "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/ProjectASAP/ASAPController", rev = "7fcaf914d87e71407c3a6d7ccac613b867f9c11b" } +asap-ir = { git = "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/ProjectASAP/ASAPController", rev = "01745cceac857be21fd1d80584c045e6f932ebfc" }