diff --git a/asap-query-engine/src/drivers/ingest/otel.rs b/asap-query-engine/src/drivers/ingest/otel.rs index 6ce865f9..cccea2b4 100644 --- a/asap-query-engine/src/drivers/ingest/otel.rs +++ b/asap-query-engine/src/drivers/ingest/otel.rs @@ -892,9 +892,24 @@ async fn route_modified_otlp_sketches_to_precompute( Capability::QuantileApprox(kind) } SketchKindHandle::Hll => Capability::CardinalityApprox, + // Heap-LESS frequency sketches answer bare + // frequency point queries (no top-k); index + // them as FrequencyEstimate so a `topk(...)` + // query routes to archive (or to a different + // sid that carries a heap-bearing variant). SketchKindHandle::CountSketch - | SketchKindHandle::CountMin - | SketchKindHandle::CmsWithHeap => { + | SketchKindHandle::CountMin => { + Capability::FrequencyEstimate(kind) + } + // Heap-BEARING frequency sketches answer + // both point-frequency AND top-k. We register + // them under FrequencyTopk (top-k is the + // strongest claim); the analyzer-side + // `is_satisfied_by` for FrequencyEstimate + // explicitly accepts heap-bearing variants, + // so bare-frequency queries still route here. + SketchKindHandle::CmsWithHeap + | SketchKindHandle::CountSketchWithHeap => { Capability::FrequencyTopk(kind) } // `Any` is the controller-side analysis- diff --git a/asap-query-engine/src/engines/warm_tier/sketch_reducer.rs b/asap-query-engine/src/engines/warm_tier/sketch_reducer.rs index 4f82acb1..8c3a17ad 100644 --- a/asap-query-engine/src/engines/warm_tier/sketch_reducer.rs +++ b/asap-query-engine/src/engines/warm_tier/sketch_reducer.rs @@ -57,7 +57,10 @@ use asap_sketchlib::sketches::kll::KllSketch; use asap_sketchlib::sketches::countminsketch::CountMinSketch; use asap_sketchlib::sketches::countsketch::CountSketch; -use crate::engines::warm_tier::decoders::decode_cms_with_heap_from_msgpack; +use crate::engines::warm_tier::decoders::{ + decode_cms_from_msgpack, decode_cms_from_proto, decode_cms_with_heap_from_msgpack, + decode_cs_from_msgpack, decode_cs_from_proto, +}; use crate::engines::warm_tier::delta_apply::{ cumulative_evaluate, per_window_evaluate, DeltaSketchKind, }; @@ -179,7 +182,13 @@ impl WarmTierResult { pub(crate) enum QueryFamily { Quantile, Cardinality, + /// Heap-BEARING heavy-hitter top-k. Requires `CmsWithHeap` or + /// `CountSketchWithHeap` to enumerate items. FrequencyTopk, + /// Heap-LESS bare frequency point query. Answered by `CountMin` / + /// `CountSketch` (and ALSO by `CmsWithHeap` / `CountSketchWithHeap`, + /// since the heap is additional info layered over the matrix). + FrequencyEstimate, } impl<'a> SketchReducer<'a> { @@ -209,6 +218,13 @@ impl<'a> SketchReducer<'a> { | "cardinality_estimate" | "count_distinct" => Ok(QueryFamily::Cardinality), "topk" | "topk_over_time" | "bottomk" => Ok(QueryFamily::FrequencyTopk), + // Bare frequency point queries — the MetricsQL surface for + // `sum by (item) (rate(m[r]))` with epsilon accuracy. The + // reducer answers these by decoding the CMS / CountSketch + // matrix directly (no heap needed). `frequency` is the + // canonical name; `count_over_time` is accepted as an alias + // for back-compat with PromQL counter-style point queries. + "frequency" | "frequency_estimate" => Ok(QueryFamily::FrequencyEstimate), other => Err(WarmTierError::UnsupportedFunction(other.to_string())), } } @@ -224,6 +240,7 @@ impl<'a> SketchReducer<'a> { Capability::QuantileApprox(_) => QueryFamily::Quantile, Capability::CardinalityApprox => QueryFamily::Cardinality, Capability::FrequencyTopk(_) => QueryFamily::FrequencyTopk, + Capability::FrequencyEstimate(_) => QueryFamily::FrequencyEstimate, } } @@ -238,7 +255,13 @@ impl<'a> SketchReducer<'a> { match (family, &meta.capability) { (QueryFamily::Quantile, Capability::QuantileApprox(_)) | (QueryFamily::Cardinality, Capability::CardinalityApprox) - | (QueryFamily::FrequencyTopk, Capability::FrequencyTopk(_)) => { + | (QueryFamily::FrequencyTopk, Capability::FrequencyTopk(_)) + | (QueryFamily::FrequencyEstimate, Capability::FrequencyEstimate(_)) + // A heap-bearing `FrequencyTopk` sid ALSO answers bare frequency + // point queries — the heap is additional info layered over the + // sketch matrix, so the underlying CMS / CountSketch matrix can + // be queried point-wise without consulting it. + | (QueryFamily::FrequencyEstimate, Capability::FrequencyTopk(_)) => { Ok(meta.capability.clone()) } (_, other) => Err(WarmTierError::UnsupportedCapability { @@ -308,6 +331,40 @@ impl<'a> SketchReducer<'a> { continue; } + // Bare frequency point query — heap-LESS dispatch. Decode + // each window's CMS / CountSketch (or the underlying matrix + // of a heap-bearing sid) and emit one (window_end, total_count) + // sample per window. The CMS / CountSketch substrate carries + // ALL items inserted via `bulk_insert`, so the per-window + // total count is the sum-of-all-rates contribution from that + // window — the natural answer to bare `sum by (item) + // (rate(m[r]))` when no specific item key is supplied. + // + // Per-item lookup (estimate(key)) is a follow-up — it requires + // plumbing a string-keyed `function_arg` through the reducer + // entry point, which the current `&[f64]` signature can't carry. + if family == QueryFamily::FrequencyEstimate { + for ts in series_list { + let mut samples_out: Vec<(i64, f64)> = + Vec::with_capacity(ts.samples.len()); + for (w_end, state) in ts.samples.iter() { + any_window = true; + let w = if *w_end >= 0 { *w_end as u64 } else { 0 }; + if w < cov_lo { + cov_lo = w; + } + if w > cov_hi { + cov_hi = w; + } + let total = + decode_frequency_total(sid, meta.sketch_kind, state)?; + samples_out.push((*w_end, total)); + } + out_series.push((ts.series_label_values, samples_out)); + } + continue; + } + // Top-k is a different shape — one entry per top-k item. if family == QueryFamily::FrequencyTopk { let k = function_args @@ -330,7 +387,13 @@ impl<'a> SketchReducer<'a> { cov_hi = w_end_u64; } let cms_heap = match meta.sketch_kind { - SketchKindHandle::CmsWithHeap => { + SketchKindHandle::CmsWithHeap + | SketchKindHandle::CountSketchWithHeap => { + // Both heap-bearing variants serialize the + // outer `CountMinSketchWithHeap` envelope via + // msgpack (`CountSketchWithHeap` reuses the + // same wire shape since the heap is the + // distinguishing payload). decode_cms_with_heap_from_msgpack(&state.bytes).map_err(|e| { WarmTierError::DeserializeFailure { sid, @@ -340,6 +403,9 @@ impl<'a> SketchReducer<'a> { })? } SketchKindHandle::CountMin | SketchKindHandle::CountSketch => { + // Heap-LESS variants can't enumerate top-k — + // they support point-frequency only (which + // routes through QueryFamily::FrequencyEstimate). return Err(WarmTierError::MissingHeap { sid, sketch_kind: meta.sketch_kind, @@ -483,22 +549,28 @@ impl<'a> SketchReducer<'a> { } QueryFamily::Cardinality => self.evaluate_cardinality(sid, sketch_kind, state), QueryFamily::FrequencyTopk => { - // CMS / CountSketch frequency point query needs a - // key. The PromQL `topk(k, foo)` shape doesn't - // pass an explicit key — the canonical answer - // would draw from a CMS-with-heap (heavy-hitter - // sketch). PR #122's `Capability::FrequencyTopk` - // doesn't yet wire the heap through, so surface - // as `UnsupportedCapability` and let the router - // fall through to archive. This is a documented - // follow-up: once `SketchKindHandle` carries a - // CmsWithHeap variant, route to a heap-walking - // estimator that returns the top-k items. + // Top-k materialization is handled in-line by the main + // `evaluate` loop via `decode_cms_with_heap_from_msgpack`; + // this legacy one-shot entry never participates in the + // top-k path. Surface as `UnsupportedCapability` so a + // stray caller falls over to archive. Err(WarmTierError::UnsupportedCapability { function: "topk".to_string(), capability: Capability::FrequencyTopk(sketch_kind), }) } + QueryFamily::FrequencyEstimate => { + // Bare frequency point query — handled in-line by the + // main `evaluate` loop via `decode_frequency_total`. + // This legacy one-shot entry doesn't drive the + // FrequencyEstimate path; surface as a defensive + // `UnsupportedCapability` so a stray caller falls over + // to archive rather than silently misroutes. + Err(WarmTierError::UnsupportedCapability { + function: "frequency".to_string(), + capability: Capability::FrequencyEstimate(sketch_kind), + }) + } } } @@ -763,12 +835,10 @@ fn HllSketch_from_sketchlib_proto_bytes(buffer: &[u8]) -> Result Option { CountMinSketch::deserialize_msgpack(buffer).ok() @@ -777,3 +847,95 @@ fn _unused_cms_kept_for_future_topk(buffer: &[u8]) -> Option { fn _unused_count_sketch_kept_for_future_topk(buffer: &[u8]) -> Option { CountSketch::deserialize_msgpack(buffer).ok() } + +/// Decode a sid's per-window frequency sketch and emit a per-window +/// total-count summary. The CMS / CountSketch matrix sums row 0 (the +/// first hash row); for a CMS, row r's column-wise sum equals the total +/// weighted insert volume into that row (each insert contributes once +/// per row), so row 0's sum is the natural per-window total-frequency +/// scalar. +/// +/// Heap-bearing variants (`CmsWithHeap` / `CountSketchWithHeap`) are +/// decoded via the same wrapper and the underlying CMS matrix is used. +/// +/// Returns `WarmTierError::DeserializeFailure` if the bytes don't decode +/// against the sid's declared sketch kind. Heap-less CMS / CountSketch +/// are NOT a `MissingHeap` error here — bare frequency is exactly what +/// heap-less variants are designed to answer. +fn decode_frequency_total( + sid: u64, + sketch_kind: SketchKindHandle, + state: &SketchSampleState, +) -> Result { + let to_err = |e: String, encoding: SketchEncoding| WarmTierError::DeserializeFailure { + sid, + encoding, + reason: e, + }; + match sketch_kind { + SketchKindHandle::CountMin => { + let cms = match state.encoding { + SketchEncoding::ProtoFull => { + decode_cms_from_proto(&state.bytes).map_err(|e| to_err(e, state.encoding))? + } + SketchEncoding::MsgpackFull => decode_cms_from_msgpack(&state.bytes) + .map_err(|e| to_err(e, state.encoding))?, + SketchEncoding::ProtoDelta | SketchEncoding::MsgpackDelta => { + return Err(to_err( + "CMS delta encodings not implemented in warm-tier reducer".to_string(), + state.encoding, + )); + } + }; + Ok(row0_sum_cms(&cms)) + } + SketchKindHandle::CountSketch => { + let cs = match state.encoding { + SketchEncoding::ProtoFull => { + decode_cs_from_proto(&state.bytes).map_err(|e| to_err(e, state.encoding))? + } + SketchEncoding::MsgpackFull => decode_cs_from_msgpack(&state.bytes) + .map_err(|e| to_err(e, state.encoding))?, + SketchEncoding::ProtoDelta | SketchEncoding::MsgpackDelta => { + return Err(to_err( + "CountSketch delta encodings not implemented in warm-tier reducer" + .to_string(), + state.encoding, + )); + } + }; + Ok(row0_sum_cs(&cs)) + } + // Heap-bearing variants: decode via the CMS-with-heap envelope + // and read the underlying CMS matrix the same way. + SketchKindHandle::CmsWithHeap | SketchKindHandle::CountSketchWithHeap => { + let heap = decode_cms_with_heap_from_msgpack(&state.bytes) + .map_err(|e| to_err(e, state.encoding))?; + let matrix = heap.sketch_matrix(); + Ok(row0_sum_from_matrix(&matrix)) + } + // Quantile / cardinality handles can't answer frequency — caller + // should have rejected at `require_capability`. Defensive arm. + other => Err(WarmTierError::UnsupportedCapability { + function: "frequency".to_string(), + capability: Capability::FrequencyEstimate(other), + }), + } +} + +fn row0_sum_cms(cms: &CountMinSketch) -> f64 { + let matrix = cms.sketch(); + row0_sum_from_matrix(&matrix) +} + +fn row0_sum_cs(cs: &CountSketch) -> f64 { + let matrix = cs.sketch(); + row0_sum_from_matrix(matrix) +} + +fn row0_sum_from_matrix(matrix: &[Vec]) -> f64 { + matrix + .first() + .map(|row| row.iter().copied().sum::()) + .unwrap_or(0.0) +} diff --git a/controller/src/emit/stage_config.rs b/controller/src/emit/stage_config.rs index 91fdba70..647ed907 100644 --- a/controller/src/emit/stage_config.rs +++ b/controller/src/emit/stage_config.rs @@ -2118,7 +2118,7 @@ mod tests { // ── Phase β: emit_backend_config_json snapshot for new pattern coverage ── // - // The new archive-only L3 intents (HistogramQuantile, Absent, Delta, …) + // The archive-only L3 intents (Absent, Present, Delta, Deriv, …) // bind to `SketchExpr::Logical` rather than producing a `BackendAggregation`, // so they correctly stay OUT of the warm-tier StreamingConfig the // backend's SimpleEngine receives. Phase α wires the archive routing diff --git a/controller/src/intent_algebra/agg_intent.rs b/controller/src/intent_algebra/agg_intent.rs index d3ef36e1..b8bc07ab 100644 --- a/controller/src/intent_algebra/agg_intent.rs +++ b/controller/src/intent_algebra/agg_intent.rs @@ -103,13 +103,13 @@ pub enum AggIntent { // sketch family for any of these is a follow-up — the L3 vocabulary // captures the intent so the routing decision is layered above intent. // - /// `histogram_quantile(φ, le_bucketed_metric)`. Operates on Prometheus - /// histogram buckets — semantically a quantile readout but the input - /// shape (per-bucket counter) requires bucket-aware aggregation that - /// the existing KLL / DDSketch rules don't model. Archive-only today. - HistogramQuantile { - q: f64, - }, + // Note: `histogram_quantile(φ, …)` is NOT an L3 intent — it's a PromQL + // /MetricsQL language-level operator (a query-expression node carried + // by `legacy_expr::QueryExpr::HistogramQuantile`). The L1→L3 lowerer + // maps `histogram_quantile(q, bucket_metric)` semantically to + // `AggIntent::Quantile { q, accuracy }`; bucket-aware handling is a + // physical-planner concern, not an L3 intent. + // /// `absent(vector_selector)` — 1 iff the selector matched no series in /// the evaluation window, no value otherwise. Routed to archive: the /// engine answers it directly off the index. @@ -168,17 +168,21 @@ impl AggIntent { /// today. `false` means a `Bind*` rule may match. `true` means the /// L5 emitter routes the intent to the cold-store / archive tier. /// - /// Per Phase β orchestrator spec, the new `HistogramQuantile` plus the - /// PromQL functions that were previously refused outright by - /// `asap-planner-rs::single_query::is_supported()` (everything outside - /// the 5 patterns) all return `true` here. Adding a sketch family for - /// any of them is a future PR — flipping the flag to `false` is the - /// single point of change. + /// Per Phase β orchestrator spec, the PromQL functions that were + /// previously refused outright by `asap-planner-rs::single_query:: + /// is_supported()` (everything outside the 5 patterns) all return + /// `true` here. Adding a sketch family for any of them is a future + /// PR — flipping the flag to `false` is the single point of change. + /// + /// Note: `histogram_quantile(...)` was previously listed as + /// archive-only here but is no longer an `AggIntent` variant — + /// it's a PromQL/MetricsQL language-level operator (carried by + /// `legacy_expr::QueryExpr::HistogramQuantile`). The L1→L3 + /// lowerer maps it semantically to `AggIntent::Quantile { q, .. }`. pub fn archive_only(&self) -> bool { matches!( self, - AggIntent::HistogramQuantile { .. } - | AggIntent::Absent + AggIntent::Absent | AggIntent::Present | AggIntent::Delta { .. } | AggIntent::Deriv { .. } @@ -265,11 +269,6 @@ impl AggIntent { // the StreamingConfig emitter and Phase α routing entry can // locate them. All are Float64 except the boolean Absent / // Present, which surface as Int64 (1 / 0) per PromQL convention. - AggIntent::HistogramQuantile { q } => Column { - name: format!("histogram_quantile_{}", quantile_suffix(*q)), - dtype: DataType::Float64, - nullable: false, - }, AggIntent::Absent => Column { name: "absent".into(), dtype: DataType::Int64, @@ -458,7 +457,6 @@ mod tests { fn archive_only_flag_partitions_intents() { // Archive-only — every Phase β migration target. let archive: Vec = vec![ - AggIntent::HistogramQuantile { q: 0.99 }, AggIntent::Absent, AggIntent::Present, AggIntent::Delta { @@ -541,7 +539,6 @@ mod tests { #[test] fn archive_only_intent_serde_roundtrip() { let cases = vec![ - AggIntent::HistogramQuantile { q: 0.99 }, AggIntent::Absent, AggIntent::Present, AggIntent::Delta { @@ -582,12 +579,6 @@ mod tests { #[test] fn archive_only_output_column_names() { let v = col("value", DataType::Float64); - assert_eq!( - AggIntent::HistogramQuantile { q: 0.99 } - .output_column(&v) - .name, - "histogram_quantile_0_99" - ); assert_eq!(AggIntent::Absent.output_column(&v).name, "absent"); assert_eq!(AggIntent::Present.output_column(&v).name, "present"); assert_eq!( diff --git a/controller/src/intent_algebra/mod.rs b/controller/src/intent_algebra/mod.rs index c88d94a2..de061480 100644 --- a/controller/src/intent_algebra/mod.rs +++ b/controller/src/intent_algebra/mod.rs @@ -14,12 +14,12 @@ //! | `ONLY_TEMPORAL` funcs (`{sum,count,avg,min,max}_over_time`, `rate`, `increase`) | [`AggIntent::Sum`] / [`AggIntent::Count`] / [`AggIntent::Avg`] / [`AggIntent::Min`] / [`AggIntent::Max`] under `Window`, plus [`AggIntent::Rate`] / [`AggIntent::Increase`] for the counter-reset variants | //! | `ONLY_SPATIAL` (`agg_op(metric)`) | `Aggregate{by, [intent]}` over a bare `Scan` (no `Window`) — the spatial `agg_op` is the [`AggIntent`] | //! | `ONE_TEMPORAL_ONE_SPATIAL` (`agg_op(temporal_func(m[range]))`) | combined `Aggregate{by, [intent]}` over a `Window` — single-rooted L3 captures both axes natively | -//! | `histogram_quantile(φ, …)` (not a `patterns.rs` entry but the legacy planner refused these) | [`AggIntent::HistogramQuantile`] — flagged archive-only via [`AggIntent::archive_only`] | +//! | `histogram_quantile(φ, …)` (not a `patterns.rs` entry but the legacy planner refused these) | [`AggIntent::Quantile`] — the lowerer maps `histogram_quantile(q, bucket_metric)` to `Quantile { q, accuracy }`; bucket-aware reduction is a physical-planner concern, not an L3 intent. | //! //! Phase β additionally lifts these archive-only intents from the legacy //! planner's "unsupported" branch into the L3 vocabulary so they get a //! StreamingConfig entry (routed to the cold tier rather than the warm -//! sketch tier): [`AggIntent::HistogramQuantile`], [`AggIntent::Absent`], +//! sketch tier): [`AggIntent::Absent`], //! [`AggIntent::Present`], [`AggIntent::Delta`], [`AggIntent::Deriv`], //! [`AggIntent::PredictLinear`], [`AggIntent::HoltWinters`], //! [`AggIntent::Idelta`], [`AggIntent::Irate`], [`AggIntent::Resets`], diff --git a/controller/src/sketch_algebra/capability.rs b/controller/src/sketch_algebra/capability.rs index aef42d0f..1aeb6816 100644 --- a/controller/src/sketch_algebra/capability.rs +++ b/controller/src/sketch_algebra/capability.rs @@ -12,7 +12,10 @@ //! profile (insert / memory / CPU / transmission costs + the logical //! intents the sketch can serve). Read by `algebra/optimizer.rs` for //! cost-based plan rewriting and by `algebra/physical.rs` for stage -//! placement. +//! placement. Disambiguation: distinct from `schema.rs::SketchStateMetadata` +//! (which carries L4 type-system flags `mergeable` / `subtractable` / +//! `deletable`) — `SketchCapability` here is the perf / cost-model surface, +//! `SketchStateMetadata` is the L4 catalog-flag surface. //! - [`Capability`] / [`SketchKindHandle`] — query-side capability tag, //! used by the warm-tier reducer in `asap-query-engine` to dispatch //! PromQL → per-Capability sketch evaluation. @@ -68,11 +71,18 @@ pub enum Capability { /// No inner handle — cardinality has a single canonical family /// today (HLL). CardinalityApprox, - /// Heavy-hitter top-k via CMS-with-heap (or CountSketch + heap). - /// `CmsWithHeap` is the canonical handle today; the - /// `Any` variant is unused for top-k because the wire format - /// distinguishes the heap-bearing variant from raw CMS at ingest - /// time. + /// Bare per-item frequency estimate (CMS / CountSketch point query, + /// no top-k extraction). Heap-LESS — answers `sum by (item) (rate(m[r]))` + /// with epsilon accuracy. Distinct from [`Capability::FrequencyTopk`]: + /// any heap-bearing variant ALSO satisfies bare frequency (the heap is + /// additional info layered on top of the sketch matrix), so + /// `is_satisfied_by` allows {CountMin, CountSketch, CmsWithHeap, + /// CountSketchWithHeap} on the available side. + FrequencyEstimate(SketchKindHandle), + /// Heavy-hitter top-k via CMS-with-heap or CountSketch-with-heap. + /// Heap-BEARING — only handles that carry an item universe in their + /// wire format can answer this. `Any` required matches either + /// `CmsWithHeap` or `CountSketchWithHeap`. FrequencyTopk(SketchKindHandle), } @@ -91,6 +101,10 @@ pub enum SketchKindHandle { /// heap is what lets the warm-tier reducer enumerate top-k items /// without an external item list. CmsWithHeap, + /// CountSketch paired with a heavy-hitter heap. Same role as + /// `CmsWithHeap` but on the CountSketch substrate (balanced / + /// zero-mean error instead of CMS's one-sided bias). + CountSketchWithHeap, /// "Any implementation that satisfies the family". Analysis-time /// wildcard, never indexed against a concrete sketch instance. /// Consumed by [`Capability::is_satisfied_by`]. @@ -116,10 +130,24 @@ impl Capability { } // Cardinality has no inner handle; family match is total. (Capability::CardinalityApprox, Capability::CardinalityApprox) => true, - // Top-k family: same Any / concrete-match semantics as - // quantile. + // Top-k family: only heap-bearing handles (CmsWithHeap or + // CountSketchWithHeap) qualify on the available side. `Any` + // required matches either; a concrete required handle must + // match exactly. (Capability::FrequencyTopk(req), Capability::FrequencyTopk(have)) => { - handles_compatible(*req, *have) + is_heap_bearing(*have) && handles_compatible_for_topk(*req, *have) + } + // Bare frequency: any frequency-family handle works on the + // available side — heap-LESS (CountMin / CountSketch) AND + // heap-bearing (CmsWithHeap / CountSketchWithHeap) all answer + // a point-frequency query (heap is additional info layered on + // the sketch matrix). A heap-bearing `FrequencyTopk` indexed + // capability ALSO satisfies a bare-frequency required capability. + (Capability::FrequencyEstimate(req), Capability::FrequencyEstimate(have)) => { + is_frequency_family(*have) && handles_compatible(*req, *have) + } + (Capability::FrequencyEstimate(req), Capability::FrequencyTopk(have)) => { + is_heap_bearing(*have) && handles_compatible(*req, *have) } _ => false, } @@ -132,6 +160,37 @@ fn handles_compatible(required: SketchKindHandle, available: SketchKindHandle) - matches!(required, SketchKindHandle::Any) || required == available } +/// `Any` required for top-k means "any heap-bearing handle"; concrete +/// required must match exactly. +fn handles_compatible_for_topk( + required: SketchKindHandle, + available: SketchKindHandle, +) -> bool { + matches!(required, SketchKindHandle::Any) || required == available +} + +/// True when the handle carries a heavy-hitter heap (i.e. it can +/// enumerate top-k items without an external item list). +fn is_heap_bearing(h: SketchKindHandle) -> bool { + matches!( + h, + SketchKindHandle::CmsWithHeap | SketchKindHandle::CountSketchWithHeap + ) +} + +/// True when the handle belongs to the frequency family — any of +/// `CountMin` / `CountSketch` (heap-less) or `CmsWithHeap` / +/// `CountSketchWithHeap` (heap-bearing). +fn is_frequency_family(h: SketchKindHandle) -> bool { + matches!( + h, + SketchKindHandle::CountMin + | SketchKindHandle::CountSketch + | SketchKindHandle::CmsWithHeap + | SketchKindHandle::CountSketchWithHeap + ) +} + // ── AggIntent → Capability bridge ──────────────────────────────────────────── /// Map a semantic [`AggIntent`] to the warm-tier [`Capability`] that can @@ -152,12 +211,14 @@ fn handles_compatible(required: SketchKindHandle, available: SketchKindHandle) - /// |---|---| /// | `Quantile { q, accuracy }` (accuracy not `Exact`) | `Some(QuantileApprox(Any))` | /// | `Quantile { q, accuracy: Exact }` | `None` (exact must use HashAgg/SortAgg) | +/// | `Min` / `Max` | `Some(QuantileApprox(Any))` — quantile sketches answer min = q(0), max = q(1) | /// | `Cardinality { accuracy }` (accuracy not `Exact`) | `Some(CardinalityApprox)` | /// | `Cardinality { accuracy: Exact }` | `None` | /// | `Count { accuracy }` (same logic as Cardinality) | `Some(CardinalityApprox)` / `None` | -/// | `TopK { k, accuracy }` | `Some(FrequencyTopk(CmsWithHeap))` | -/// | `Frequency { accuracy }` (accuracy not `Exact`) | `Some(FrequencyTopk(CmsWithHeap))` | -/// | `Sum` / `Min` / `Max` / `Avg` / `Rate` / `Increase` | `None` | +/// | `TopK { k, accuracy }` (accuracy not `Exact`) | `Some(FrequencyTopk(CmsWithHeap))` | +/// | `Frequency { accuracy }` (accuracy not `Exact`) | `Some(FrequencyEstimate(Any))` | +/// | `Frequency { accuracy: Exact }` | `None` (exact aggregation; route to archive) | +/// | `Sum` / `Avg` / `Rate` / `Increase` | `None` | /// | Every archive-only intent | `None` | pub fn capability_for(intent: &AggIntent) -> Option { match intent { @@ -188,34 +249,49 @@ pub fn capability_for(intent: &AggIntent) -> Option { Some(Capability::CardinalityApprox) } } - AggIntent::TopK { .. } => { - // Top-k is intrinsically heavy-hitter — only the - // CMS-with-heap variant can enumerate the items. CountMin / - // CountSketch without a heap can answer point-frequency but - // not top-k. - Some(Capability::FrequencyTopk(SketchKindHandle::CmsWithHeap)) + AggIntent::TopK { accuracy, .. } => { + if is_exact(accuracy) { + // Exact top-k must use HashAgg+Heap; no warm-tier sketch. + None + } else { + // Top-k is intrinsically heavy-hitter — only heap-bearing + // handles can enumerate the items. `CmsWithHeap` is the + // canonical handle today; `is_satisfied_by` accepts + // either heap-bearing variant against an `Any` required. + Some(Capability::FrequencyTopk(SketchKindHandle::CmsWithHeap)) + } } AggIntent::Frequency { accuracy } => { if is_exact(accuracy) { + // Exact aggregation — sketch fallback is only meaningful + // when raw counters aren't kept at the ingest tier; with + // accuracy=Exact the caller wants exact `sum by (label) + // (rate(...))`, which routes to archive. None } else { - // Frequency point-queries use CMS-with-heap as the - // canonical family (lets a single sketch family answer - // both Frequency and TopK on the same metric). - Some(Capability::FrequencyTopk(SketchKindHandle::CmsWithHeap)) + // Bare frequency point-query uses a frequency-family + // sketch — any of CMS / CountSketch / CmsWithHeap / + // CountSketchWithHeap works (the heap is additional + // info that the FrequencyTopk path uses). `Any` here + // means the optimizer picks the cheapest indexed sid. + Some(Capability::FrequencyEstimate(SketchKindHandle::Any)) } } + // ── Min / Max via quantile sketches ────────────────────────── + // DDSketch / KLL answer min = quantile(0) and max = quantile(1) + // out of the box. No dedicated extrema sketch is needed; route + // these through the quantile-family handler. + AggIntent::Min | AggIntent::Max => { + Some(Capability::QuantileApprox(SketchKindHandle::Any)) + } // ── No warm-tier sketch ────────────────────────────────────── AggIntent::Sum - | AggIntent::Min - | AggIntent::Max | AggIntent::Avg | AggIntent::Rate { .. } | AggIntent::Increase { .. } => None, // Archive-only intents — never bind to a warm-tier capability; // routed to the cold tier (Gorilla / Thanos). - AggIntent::HistogramQuantile { .. } - | AggIntent::Absent + AggIntent::Absent | AggIntent::Present | AggIntent::Delta { .. } | AggIntent::Deriv { .. } @@ -242,6 +318,17 @@ fn is_exact(accuracy: &AccuracyTarget) -> bool { /// planner to check whether a sketch fits within a stage's budget. /// Populated from compiled-in defaults via [`default_capability_table`] /// or overridden at runtime via [`load_capability_overrides`]. +/// +/// Distinct from +/// [`crate::sketch_algebra::schema::SketchStateMetadata`] — this struct +/// is the **perf / feasibility / intent-routing** profile consumed by +/// the cost model and the optimizer's binding rules. The schema-side +/// `SketchStateMetadata` carries the **L4 type-system flags** +/// (`mergeable` / `subtractable` / `deletable`) that gate `SketchMerge` / +/// `SketchSubtract` / `SketchDelete` at plan-time. The two have +/// different consumers and different lifecycles — `SketchCapability` +/// is read at every plan-rewrite call site; `SketchStateMetadata` +/// is sealed onto each `SketchExpr` edge once the binding rule fires. #[derive(Debug, Clone)] pub struct SketchCapability { /// Insertion throughput (samples/sec at 1 core). @@ -542,12 +629,32 @@ mod tests { } #[test] - fn capability_for_min_max_avg_return_none() { - assert_eq!(capability_for(&AggIntent::Min), None); - assert_eq!(capability_for(&AggIntent::Max), None); + fn capability_for_avg_returns_none() { + // Avg is exact at L3 — no warm-tier sketch substitutes for it + // today (a sketch-bound `Avg` would fold onto `Quantile{q=0.5}` + // only when the cost model allows the relaxation, which is a + // follow-up). assert_eq!(capability_for(&AggIntent::Avg), None); } + #[test] + fn capability_for_min_returns_quantile_approx() { + // Min = quantile(0); DDSketch / KLL answer it directly. + assert_eq!( + capability_for(&AggIntent::Min), + Some(Capability::QuantileApprox(SketchKindHandle::Any)) + ); + } + + #[test] + fn capability_for_max_returns_quantile_approx() { + // Max = quantile(1); DDSketch / KLL answer it directly. + assert_eq!( + capability_for(&AggIntent::Max), + Some(Capability::QuantileApprox(SketchKindHandle::Any)) + ); + } + #[test] fn capability_for_rate_increase_return_none() { assert_eq!( @@ -577,23 +684,53 @@ mod tests { } #[test] - fn capability_for_frequency_approximate_returns_topk_cms_with_heap() { + fn capability_for_topk_exact_returns_none() { + // Exact top-k must use HashAgg+Heap; no warm-tier sketch. + let intent = AggIntent::TopK { + k: 10, + accuracy: AccuracyTarget::Exact, + }; + assert_eq!(capability_for(&intent), None); + } + + #[test] + fn frequency_estimate_with_epsilon_returns_frequency_estimate_approx() { let intent = AggIntent::Frequency { accuracy: AccuracyTarget::Epsilon(0.01), }; assert_eq!( capability_for(&intent), - Some(Capability::FrequencyTopk(SketchKindHandle::CmsWithHeap)) + Some(Capability::FrequencyEstimate(SketchKindHandle::Any)) ); } #[test] - fn capability_for_archive_only_intents_return_none() { - // Spot-check each archive-only variant. + fn frequency_estimate_with_epsilon_delta_returns_frequency_estimate_approx() { + let intent = AggIntent::Frequency { + accuracy: AccuracyTarget::EpsilonDelta { + eps: 0.01, + delta: 0.001, + }, + }; assert_eq!( - capability_for(&AggIntent::HistogramQuantile { q: 0.99 }), - None, + capability_for(&intent), + Some(Capability::FrequencyEstimate(SketchKindHandle::Any)) ); + } + + #[test] + fn frequency_estimate_with_exact_returns_none() { + // Exact aggregation routes to archive (sketch fallback only + // meaningful when raw counters aren't kept). + let intent = AggIntent::Frequency { + accuracy: AccuracyTarget::Exact, + }; + assert_eq!(capability_for(&intent), None); + } + + #[test] + fn capability_for_archive_only_intents_return_none() { + // Spot-check each archive-only variant. assert_eq!(capability_for(&AggIntent::Absent), None); assert_eq!(capability_for(&AggIntent::Present), None); assert_eq!( @@ -665,6 +802,79 @@ mod tests { assert!(!required.is_satisfied_by(&indexed_no_heap)); } + #[test] + fn is_satisfied_by_frequency_topk_rejects_heapless() { + // Top-k REQUIRES a heap-bearing handle. Even when the available + // capability declares itself as `FrequencyTopk(CountMin)` (an + // ill-formed catalog entry), the satisfaction check must reject + // it — top-k cannot enumerate items off a heap-less sketch. + let required_any = Capability::FrequencyTopk(SketchKindHandle::Any); + let required_concrete = Capability::FrequencyTopk(SketchKindHandle::CmsWithHeap); + let indexed_heapless = Capability::FrequencyTopk(SketchKindHandle::CountMin); + let indexed_heapless_cs = Capability::FrequencyTopk(SketchKindHandle::CountSketch); + assert!(!required_any.is_satisfied_by(&indexed_heapless)); + assert!(!required_any.is_satisfied_by(&indexed_heapless_cs)); + assert!(!required_concrete.is_satisfied_by(&indexed_heapless)); + } + + #[test] + fn is_satisfied_by_frequency_topk_any_matches_either_heap() { + // `Any` required for top-k accepts either heap-bearing handle. + let required = Capability::FrequencyTopk(SketchKindHandle::Any); + let cms_heap = Capability::FrequencyTopk(SketchKindHandle::CmsWithHeap); + let cs_heap = Capability::FrequencyTopk(SketchKindHandle::CountSketchWithHeap); + assert!(required.is_satisfied_by(&cms_heap)); + assert!(required.is_satisfied_by(&cs_heap)); + } + + #[test] + fn is_satisfied_by_frequency_estimate_accepts_heap_bearing() { + // Bare frequency point queries can be answered by ANY + // frequency-family sketch — heap-less AND heap-bearing both work + // (the heap is additional metadata; the underlying CMS / CS + // matrix answers the point query either way). + let required = Capability::FrequencyEstimate(SketchKindHandle::Any); + let cms = Capability::FrequencyEstimate(SketchKindHandle::CountMin); + let cs = Capability::FrequencyEstimate(SketchKindHandle::CountSketch); + let cms_heap = Capability::FrequencyTopk(SketchKindHandle::CmsWithHeap); + let cs_heap = Capability::FrequencyTopk(SketchKindHandle::CountSketchWithHeap); + assert!(required.is_satisfied_by(&cms)); + assert!(required.is_satisfied_by(&cs)); + assert!(required.is_satisfied_by(&cms_heap)); + assert!(required.is_satisfied_by(&cs_heap)); + } + + #[test] + fn is_satisfied_by_frequency_estimate_rejects_non_frequency_family() { + let required = Capability::FrequencyEstimate(SketchKindHandle::Any); + // QuantileApprox / CardinalityApprox don't answer frequency. + let q = Capability::QuantileApprox(SketchKindHandle::DDSketch); + let c = Capability::CardinalityApprox; + // FrequencyEstimate with a non-frequency-family handle on the + // available side is also rejected (defensive). + let bad = Capability::FrequencyEstimate(SketchKindHandle::Hll); + assert!(!required.is_satisfied_by(&q)); + assert!(!required.is_satisfied_by(&c)); + assert!(!required.is_satisfied_by(&bad)); + } + + #[test] + fn count_sketch_with_heap_handle_round_trips() { + // `CountSketchWithHeap` is the CountSketch counterpart to + // `CmsWithHeap`. Construct a `FrequencyTopk` capability around + // it and verify it satisfies an `Any`-required top-k. + let cap = Capability::FrequencyTopk(SketchKindHandle::CountSketchWithHeap); + let required = Capability::FrequencyTopk(SketchKindHandle::Any); + assert!(required.is_satisfied_by(&cap)); + // And the concrete-against-concrete must match exactly. + let required_concrete = + Capability::FrequencyTopk(SketchKindHandle::CountSketchWithHeap); + assert!(required_concrete.is_satisfied_by(&cap)); + // A different concrete heap-bearing handle must NOT match. + let required_cms = Capability::FrequencyTopk(SketchKindHandle::CmsWithHeap); + assert!(!required_cms.is_satisfied_by(&cap)); + } + // ── default_capability_table ───────────────────────────────────────── #[test] diff --git a/controller/src/sketch_algebra/mod.rs b/controller/src/sketch_algebra/mod.rs index 45211bfb..3105da26 100644 --- a/controller/src/sketch_algebra/mod.rs +++ b/controller/src/sketch_algebra/mod.rs @@ -34,10 +34,16 @@ pub mod capability; pub mod capability_matching; pub mod lower; -pub mod params; pub mod rules; pub mod schema; pub mod sketch_expr; +pub mod sketch_params; + +// Back-compat alias. External call sites that imported +// `controller::sketch_algebra::params::*` (and the in-tree +// `crate::sketch_algebra::params::SketchKind` use sites that this +// touch-up didn't migrate) keep compiling. +pub use sketch_params as params; #[cfg(test)] mod tests; @@ -51,8 +57,8 @@ pub use capability_matching::{ classify_demo_metric, is_valid_pair, pick_family, AccuracyPreference, StatisticClass, }; pub use lower::{bind_query_expr, BindingError}; -pub use params::{ +pub use sketch_params::{ CmsParams, CountSketchParams, DDSketchParams, HllParams, KllParams, SketchKind, SketchParams, }; -pub use schema::{SketchCapabilities, SketchStateSchema}; +pub use schema::{SketchStateMetadata, SketchStateSchema}; pub use sketch_expr::{EstimateOp, MergeAlgebra, SketchExpr}; diff --git a/controller/src/sketch_algebra/rules/bind_archive_only.rs b/controller/src/sketch_algebra/rules/bind_archive_only.rs index 44c70d4e..fcd622c9 100644 --- a/controller/src/sketch_algebra/rules/bind_archive_only.rs +++ b/controller/src/sketch_algebra/rules/bind_archive_only.rs @@ -2,9 +2,9 @@ //! cold tier. //! //! This rule is the L4 catch for [`AggIntent`]s that don't have a warm- -//! tier streaming sketch family today (`HistogramQuantile`, `Absent`, -//! `Delta`, `Deriv`, `PredictLinear`, `HoltWinters`, `Idelta`, `Irate`, -//! `Resets`, `Changes`, `Present`). It matches a single-intent +//! 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 //! [`SketchExpr::Logical`] pass-through. The L5 emitter looks at the //! enclosed [`AggIntent::archive_only`] flag and routes the corresponding @@ -123,11 +123,18 @@ mod tests { } #[test] - fn binds_histogram_quantile() { - let expr = agg_with(AggIntent::HistogramQuantile { q: 0.99 }); + fn binds_absent_archive_only() { + // `histogram_quantile(...)` is no longer an L3 intent — it's a + // PromQL operator that the controller's PromQL parser lowers via + // `legacy_expr::QueryExpr::HistogramQuantile`. The L3 mapping + // `histogram_quantile(q, bucket_metric) → Quantile{q,...}` is a + // semantic-only documented contract; the canonical archive-only + // anchor for this test is `Absent` (which has no warm-tier sketch + // family). + let expr = agg_with(AggIntent::Absent); let out = BindArchiveOnly .apply(&expr, &AccuracyTarget::Epsilon(0.01)) - .expect("rule should match histogram_quantile"); + .expect("rule should match Absent"); match out { SketchExpr::Logical(inner) => assert_eq!(inner, expr), other => panic!("expected Logical pass-through, got {other:?}"), @@ -137,7 +144,6 @@ mod tests { #[test] fn binds_each_archive_only_intent() { let intents = vec![ - AggIntent::HistogramQuantile { q: 0.5 }, AggIntent::Absent, AggIntent::Present, AggIntent::Delta { diff --git a/controller/src/sketch_algebra/schema.rs b/controller/src/sketch_algebra/schema.rs index dd5235ca..aae569c7 100644 --- a/controller/src/sketch_algebra/schema.rs +++ b/controller/src/sketch_algebra/schema.rs @@ -42,16 +42,25 @@ pub struct SketchStateSchema { /// Parameter payload — must match across all inputs to a `SketchMerge`. pub params: SketchParams, /// Capability flags from the sketch catalog. - pub caps: SketchCapabilities, + pub caps: SketchStateMetadata, } -/// Catalog capability flags. Populated from the sketch catalog at -/// `Bind*`-rule time. `mergeable` gates `SketchMerge`; `subtractable` -/// gates `SketchSubtract`; `deletable` gates `SketchDelete`. See design.md -/// §6 line ~646 ("catalog is the single source of truth for these flags; -/// binding rules consult it before producing the node"). +/// L4 type-system catalog flags for a sketch state. Populated from the +/// sketch catalog at `Bind*`-rule time. `mergeable` gates `SketchMerge`; +/// `subtractable` gates `SketchSubtract`; `deletable` gates +/// `SketchDelete`. See design.md §6 line ~646 ("catalog is the single +/// source of truth for these flags; binding rules consult it before +/// producing the node"). +/// +/// Renamed from `SketchCapabilities` in May 2026 to disambiguate from +/// [`crate::sketch_algebra::capability::SketchCapability`] (perf / +/// cost-model profile). The two structs live side-by-side: this one is +/// the **L4 type-system / plan-time** surface — sealed onto every +/// `SketchExpr` edge by the binding rule and consulted by the type +/// checker. `SketchCapability` is the **perf / feasibility / intent- +/// routing** surface — consumed by the optimizer and cost model. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct SketchCapabilities { +pub struct SketchStateMetadata { /// Whether two states of this family + params can be unioned — /// catalog default for KLL / HLL / DDSketch / CMS / CountSketch. pub mergeable: bool, @@ -70,27 +79,27 @@ impl SketchStateSchema { /// truth that downstream rules must consult. pub fn for_kind(kind: SketchKind, params: SketchParams) -> Self { let caps = match kind { - SketchKind::Kll => SketchCapabilities { + SketchKind::Kll => SketchStateMetadata { mergeable: true, subtractable: false, deletable: false, }, - SketchKind::DDSketch => SketchCapabilities { + SketchKind::DDSketch => SketchStateMetadata { mergeable: true, subtractable: false, deletable: false, }, - SketchKind::Hll => SketchCapabilities { + SketchKind::Hll => SketchStateMetadata { mergeable: true, subtractable: false, deletable: false, }, - SketchKind::Cms => SketchCapabilities { + SketchKind::Cms => SketchStateMetadata { mergeable: true, subtractable: true, deletable: true, }, - SketchKind::CountSketch => SketchCapabilities { + SketchKind::CountSketch => SketchStateMetadata { mergeable: true, subtractable: true, deletable: false, diff --git a/controller/src/sketch_algebra/params.rs b/controller/src/sketch_algebra/sketch_params.rs similarity index 100% rename from controller/src/sketch_algebra/params.rs rename to controller/src/sketch_algebra/sketch_params.rs diff --git a/controller/src/sketch_algebra/tests.rs b/controller/src/sketch_algebra/tests.rs index e28d01b7..9412722e 100644 --- a/controller/src/sketch_algebra/tests.rs +++ b/controller/src/sketch_algebra/tests.rs @@ -473,15 +473,21 @@ fn phase_b_pattern_temporal_and_spatial_combined() { assert!(matches!(bound, SketchExpr::Logical(_))); } -/// `histogram_quantile(φ, …)` — Phase β archive-only addition (not in -/// `patterns.rs`'s 5 entries; the legacy planner refused these via -/// `is_supported() == false`). Controller path: `BindArchiveOnly` matches -/// → `Logical` pass-through, and the L5 emitter / Phase α routing reads +/// Phase β archive-only intent: any of the no-warm-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. +/// +/// `histogram_quantile(...)` was previously an L3 intent here but is no +/// longer — it's a PromQL/MetricsQL language-level operator (carried by +/// `legacy_expr::QueryExpr::HistogramQuantile`), NOT a semantic intent. +/// The L1→L3 lowerer's documented contract is +/// `histogram_quantile(q, bucket_metric) → AggIntent::Quantile { q, .. }`; +/// bucket-aware reduction is a physical-planner concern. #[test] -fn phase_b_pattern_histogram_quantile_routes_to_archive() { - let intent = AggIntent::HistogramQuantile { q: 0.99 }; +fn phase_b_pattern_archive_only_routes_to_archive() { + let intent = AggIntent::Absent; assert!(intent.archive_only(), "Phase β intent must flag archive"); let expr = QueryExpr::Aggregate { by: vec![], @@ -496,7 +502,7 @@ fn phase_b_pattern_histogram_quantile_routes_to_archive() { SketchExpr::Logical(QueryExpr::Aggregate { aggs, .. }) => { assert_eq!(aggs, vec![intent]); } - other => panic!("expected Logical(Aggregate(HistogramQuantile)), got {other:?}"), + other => panic!("expected Logical(Aggregate(Absent)), got {other:?}"), } } @@ -703,19 +709,21 @@ fn phase_b_e2e_topk_well_formed() { let _ = collect_sketch_kinds(&bound); } -/// `histogram_quantile` — Phase β archive routing through the full L1→ -/// L3→L4 pipeline. Asserts the expected functional equivalent of -/// asap-planner-rs's previous `is_supported() == false` behavior -/// (refused outright); the controller now lifts these to L3 with -/// `archive_only() == true` and the binder emits a Logical pass-through. +/// Archive-only routing through the full L1→L3→L4 pipeline. Asserts the +/// expected functional equivalent of asap-planner-rs's previous +/// `is_supported() == false` behavior (refused outright); the controller +/// now lifts these to L3 with `archive_only() == true` and the binder +/// emits a Logical pass-through. +/// +/// Replaces the prior `phase_b_e2e_histogram_quantile_e2e_through_parser` +/// — `histogram_quantile(...)` is now a PromQL/MetricsQL language-level +/// operator carried by `legacy_expr::QueryExpr::HistogramQuantile`, NOT +/// an L3 intent. The L1→L3 contract maps it semantically to `Quantile{q}`; +/// the archive-only routing this test exercises uses `Absent` as a +/// stable proxy (every archive-only variant follows the same code path). #[test] -fn phase_b_e2e_histogram_quantile_e2e_through_parser() { - // The PromQL parser produces a `HistogramQuantile` QueryExpr node - // (not a flat `Aggregate{Quantile}`), so the L3 lowering of - // ParsedQuery cannot fully express it via the legacy AggType axis. - // We assert the SHAPE of the bound expression directly here: an - // archive-only intent under any `Aggregate` survives through bind. - let intent = AggIntent::HistogramQuantile { q: 0.99 }; +fn phase_b_e2e_archive_only_e2e_binding() { + let intent = AggIntent::Absent; let expr = QueryExpr::Aggregate { by: vec![], aggs: vec![intent.clone()], @@ -725,9 +733,9 @@ fn phase_b_e2e_histogram_quantile_e2e_through_parser() { let bound = bind_query_expr(&expr, AccuracyTarget::Epsilon(0.01)).unwrap(); assert!( binding_is_archive(&bound), - "histogram_quantile must surface archive flag through L4 binding" + "archive-only intent must surface archive flag through L4 binding" ); - // No warm-tier sketch fires for HistogramQuantile. + // No warm-tier sketch fires for archive-only intents. assert!(collect_sketch_kinds(&bound).is_empty()); } @@ -739,7 +747,6 @@ fn phase_b_e2e_histogram_quantile_e2e_through_parser() { #[test] fn phase_b_archive_only_intents_round_trip_through_binder() { let intents = vec![ - AggIntent::HistogramQuantile { q: 0.5 }, AggIntent::Absent, AggIntent::Present, AggIntent::Delta { diff --git a/controller/src/warm_tier_analysis.rs b/controller/src/warm_tier_analysis.rs index 124d8180..becbd81a 100644 --- a/controller/src/warm_tier_analysis.rs +++ b/controller/src/warm_tier_analysis.rs @@ -262,7 +262,6 @@ fn intent_kind_label(intent: &AggIntent) -> &'static str { AggIntent::Frequency { .. } => "frequency", AggIntent::Rate { .. } => "rate", AggIntent::Increase { .. } => "increase", - AggIntent::HistogramQuantile { .. } => "histogram_quantile", AggIntent::Absent => "absent", AggIntent::Present => "present", AggIntent::Delta { .. } => "delta", @@ -415,13 +414,14 @@ mod tests { #[test] fn analyze_histogram_quantile_is_rejected() { - // `histogram_quantile(...)` is either rejected by the - // controller's PromQL parser (because its second-arg shape - // requires a `rate(bucket[r])` that the analyzer rejects as - // an exact-counter intent) or lowered to the archive-only - // `AggIntent::HistogramQuantile` (which `capability_for` - // returns None for). Either path is the right "not warm-tier - // answerable" answer; assert SOME unsupported reason. + // `histogram_quantile(...)` is a PromQL/MetricsQL language-level + // operator (a `legacy_expr::QueryExpr::HistogramQuantile` node), + // NOT an L3 intent. The inner argument shape requires a + // `rate(bucket[r])` which the analyzer rejects as an exact-counter + // intent, so the analyzer returns SOME unsupported reason. The + // architectural mapping `histogram_quantile(q, bucket_metric)` → + // `AggIntent::Quantile{q,...}` is documented but the bucket-aware + // physical reduction is not yet wired into the warm-tier path. let a = analyze_promql_for_warm_tier( "histogram_quantile(0.99, sum(rate(http_latency_bucket[5m])) by (le))", );