From 5b74c79cb64fa38e0da88e2dc9890c0478cc47a0 Mon Sep 17 00:00:00 2001 From: zz_y Date: Mon, 20 Jul 2026 10:42:33 -0600 Subject: [PATCH] chore(promql_utilities): delete five dead types and the logics.rs planner shim Stage 2 of the promql_utilities/sketch_algebra retirement plan. Every item deleted here had zero non-test callers anywhere in the workspace (control_plane, data_plane, asap_types) -- confirmed by a full-repo call-site audit, not just a local grep. - QueryPatternType, PromQLFunction, AggregationOperator: defined, tested, never imported anywhere else. PromQLFunction duplicated work that already happened twice over independently (ASAPController's frontend-promql parses the same function names at L1; data_plane's own backend_storage_routing.rs built its own QueryShape matcher for the same strings). - QueryTreatmentType: its one live dependent was a cross-check test in asap_types::capability_matching, superseded by that crate's own AccuracyTarget (Exact/Approximate) which already covers the need. - logics.rs (map_statistic_to_precompute_operator, does_precompute_operator_support_subpopulations, get_is_collapsable): dead in production. The docstring's claimed caller, IntermediateAggConfig, doesn't exist anywhere in the Rust codebase -- a retired Python-planner relic. Deleted the whole file; asap_types::capability_matching:: compatible_agg_types is now the sole source of truth for (Statistic, AggregationType) compatibility, no second table to keep in agreement with. - Rewrote/removed the capability_canonical_map_agreement test (its entire premise -- two independently-maintained tables that must agree -- no longer applies now that only one table exists) and fixed the stale doc-comment cross-references to the deleted function. No behavior change anywhere. cargo build --workspace clean; cargo test -p promql_utilities -p asap_types -p control_plane all green (828 passed, 1 known pre-existing unrelated failure). Co-Authored-By: Claude Sonnet 5 --- crates/asap_types/src/capability_matching.rs | 85 +----- .../src/query_logics/enums.rs | 206 -------------- .../src/query_logics/logics.rs | 257 ------------------ .../promql_utilities/src/query_logics/mod.rs | 2 - 4 files changed, 10 insertions(+), 540 deletions(-) delete mode 100644 crates/promql_utilities/src/query_logics/logics.rs diff --git a/crates/asap_types/src/capability_matching.rs b/crates/asap_types/src/capability_matching.rs index 79855f5f..1a6627cf 100644 --- a/crates/asap_types/src/capability_matching.rs +++ b/crates/asap_types/src/capability_matching.rs @@ -117,14 +117,14 @@ pub enum AccuracyTarget { /// Returns the aggregation types that can serve this statistic. /// -/// This list is the **superset of compatibility**: every `AggregationType` -/// that the planner's canonical map (`promql_utilities::query_logics::logics:: -/// map_statistic_to_precompute_operator`) may legally produce for this -/// statistic — across both `Exact` and `Approximate` treatment types — must -/// appear here. The agreement is enforced by -/// `capability_canonical_map_agreement` in the test module: any future -/// divergence between this table and `map_statistic_to_precompute_operator` -/// will be caught at test-time. +/// This list is the **superset of compatibility** and, as of the +/// `promql_utilities` retirement, the **single source of truth** for it — +/// there used to be a second, independently-maintained table +/// (`promql_utilities::query_logics::logics::map_statistic_to_precompute_operator`, +/// the planner's own canonical map) that this one had to agree with, +/// checked by a `capability_canonical_map_agreement` test. That table was +/// dead code (a Python-planner relic — nothing in Rust ever called it +/// except that one test) and was deleted; this is now the only table. /// /// The runtime caller (`find_compatible_aggregation`) has no /// `QueryTreatmentType` to consult — `QueryRequirements` is treatment-agnostic @@ -134,8 +134,7 @@ pub enum AccuracyTarget { /// `aggregation_priority` (largest window size wins). pub fn compatible_agg_types(stat: Statistic) -> &'static [AggregationType] { match stat { - // Sum: exact via Sum / MultipleSum; approximate via CountMinSketch - // (the canonical approximator picked by `map_statistic_to_precompute_operator`). + // Sum: exact via Sum / MultipleSum; approximate via CountMinSketch. // Pre-fix this list omitted CountMinSketch, so a `sum_over_time(...)` // query against a CMS-only config fell through capability matching // and onto the cold tier. @@ -1057,69 +1056,6 @@ mod tests { assert!(result.is_none()); } - // ----------------------------------------------------------------------- - // Source-of-truth agreement check. - // - // `compatible_agg_types(Statistic)` (this file) and - // `promql_utilities::query_logics::logics::map_statistic_to_precompute_operator` - // are two views onto the same `(Statistic, AggregationType)` capability - // table. The planner emits configs from the canonical map; capability - // matching dispatches queries against the compat list. They MUST agree — - // every canonical map output for a given Statistic must be a member of - // `compatible_agg_types(Statistic)` — or queries the planner configured - // will silently fall through capability matching to the cold-tier - // fallback. - // - // This test enumerates every supported `(Statistic, QueryTreatmentType)` - // pair, calls the canonical map, and asserts membership. Any future edit - // on either side that breaks the agreement fails the build. - // ----------------------------------------------------------------------- - - #[test] - fn capability_canonical_map_agreement() { - use promql_utilities::query_logics::enums::QueryTreatmentType; - use promql_utilities::query_logics::logics::map_statistic_to_precompute_operator; - - // Listed exhaustively so adding a new `Statistic` variant fails to - // compile here (forcing the author to decide its compat membership). - let stats = [ - Statistic::Count, - Statistic::Sum, - Statistic::Cardinality, - Statistic::Increase, - Statistic::Rate, - Statistic::Min, - Statistic::Max, - Statistic::Quantile, - Statistic::Topk, - ]; - let treatments = [QueryTreatmentType::Exact, QueryTreatmentType::Approximate]; - - for &stat in &stats { - let compat = compatible_agg_types(stat); - for &treat in &treatments { - match map_statistic_to_precompute_operator(stat, treat) { - Ok((agg_type, _sub_type)) => { - assert!( - compat.contains(&agg_type), - "Divergence: map_statistic_to_precompute_operator({stat:?}, {treat:?}) \ - returns {agg_type:?}, but compatible_agg_types({stat:?}) = {compat:?} \ - does not list it. Either add {agg_type:?} to compatible_agg_types or \ - change the canonical map. See the docstring on \ - compatible_agg_types for the source-of-truth invariant.", - ); - } - Err(_) => { - // The canonical map declines this pair (e.g. - // Quantile-Exact, Cardinality, etc.). That's fine — - // capability_matching never sees a planner-emitted - // config for that pair, so there's nothing to agree on. - } - } - } - } - } - /// Pin the canonical-approximator picks driving the ASAP-tier query path /// (the "five sketch types" CMS / KLL / HLL / DDSketch / CountSketch /// canonical statistic table from PROGRESS.md). HLL / DDSketch / @@ -1345,8 +1281,7 @@ mod tests { assert_eq!(parse_storage_backend_engine_id("not_an_engine"), None); } - /// Source-of-truth agreement check, mirrors - /// `capability_canonical_map_agreement` for the storage axis. + /// Source-of-truth agreement check for the storage axis. /// /// For every `(Statistic, AccuracyTarget, StorageBackend)` triple /// the returned backend list must be non-empty and its head must diff --git a/crates/promql_utilities/src/query_logics/enums.rs b/crates/promql_utilities/src/query_logics/enums.rs index 088eb0fb..cdbcae26 100644 --- a/crates/promql_utilities/src/query_logics/enums.rs +++ b/crates/promql_utilities/src/query_logics/enums.rs @@ -3,40 +3,6 @@ use std::fmt; use std::str::FromStr; use tracing::debug; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum QueryPatternType { - OnlyTemporal, - OnlySpatial, - OneTemporalOneSpatial, -} - -impl std::fmt::Display for QueryPatternType { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - debug!("Formatting QueryPatternType: {:?}", self); - match self { - QueryPatternType::OnlyTemporal => write!(f, "only_temporal"), - QueryPatternType::OnlySpatial => write!(f, "only_spatial"), - QueryPatternType::OneTemporalOneSpatial => write!(f, "one_temporal_one_spatial"), - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum QueryTreatmentType { - Exact, - Approximate, -} - -impl std::fmt::Display for QueryTreatmentType { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - debug!("Formatting QueryTreatmentType: {:?}", self); - match self { - QueryTreatmentType::Exact => write!(f, "exact"), - QueryTreatmentType::Approximate => write!(f, "approximate"), - } - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum Statistic { Count, @@ -113,148 +79,6 @@ impl std::fmt::Display for QueryResultType { } } -/// A PromQL function that produces a vector-valued result. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum PromQLFunction { - Rate, - Increase, - SumOverTime, - CountOverTime, - AvgOverTime, - MinOverTime, - MaxOverTime, - QuantileOverTime, -} - -impl PromQLFunction { - pub fn as_str(self) -> &'static str { - match self { - PromQLFunction::Rate => "rate", - PromQLFunction::Increase => "increase", - PromQLFunction::SumOverTime => "sum_over_time", - PromQLFunction::CountOverTime => "count_over_time", - PromQLFunction::AvgOverTime => "avg_over_time", - PromQLFunction::MinOverTime => "min_over_time", - PromQLFunction::MaxOverTime => "max_over_time", - PromQLFunction::QuantileOverTime => "quantile_over_time", - } - } - - /// Returns `true` for functions whose result requires approximate pre-aggregation. - pub fn is_approximate(self) -> bool { - matches!( - self, - PromQLFunction::QuantileOverTime - | PromQLFunction::SumOverTime - | PromQLFunction::CountOverTime - | PromQLFunction::AvgOverTime - ) - } -} - -impl std::fmt::Display for PromQLFunction { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(self.as_str()) - } -} - -impl std::str::FromStr for PromQLFunction { - type Err = String; - fn from_str(s: &str) -> Result { - match s { - "rate" => Ok(PromQLFunction::Rate), - "increase" => Ok(PromQLFunction::Increase), - "sum_over_time" => Ok(PromQLFunction::SumOverTime), - "count_over_time" => Ok(PromQLFunction::CountOverTime), - "avg_over_time" => Ok(PromQLFunction::AvgOverTime), - "min_over_time" => Ok(PromQLFunction::MinOverTime), - "max_over_time" => Ok(PromQLFunction::MaxOverTime), - "quantile_over_time" => Ok(PromQLFunction::QuantileOverTime), - other => Err(format!("Unknown PromQL function: '{other}'")), - } - } -} - -/// A PromQL aggregation operator. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum AggregationOperator { - Sum, - Count, - Avg, - Quantile, - Min, - Max, - Topk, -} - -impl AggregationOperator { - pub fn as_str(self) -> &'static str { - match self { - AggregationOperator::Sum => "sum", - AggregationOperator::Count => "count", - AggregationOperator::Avg => "avg", - AggregationOperator::Quantile => "quantile", - AggregationOperator::Min => "min", - AggregationOperator::Max => "max", - AggregationOperator::Topk => "topk", - } - } - - /// Returns the `Statistic` values required to answer this operator. - /// `Avg` needs both `Sum` and `Count`. - pub fn to_statistics(self) -> Vec { - match self { - AggregationOperator::Avg => vec![Statistic::Sum, Statistic::Count], - AggregationOperator::Sum => vec![Statistic::Sum], - AggregationOperator::Count => vec![Statistic::Count], - AggregationOperator::Quantile => vec![Statistic::Quantile], - AggregationOperator::Min => vec![Statistic::Min], - AggregationOperator::Max => vec![Statistic::Max], - AggregationOperator::Topk => vec![Statistic::Topk], - } - } - - pub fn as_str_slice() -> &'static [&'static str] { - &["sum", "count", "avg", "quantile", "min", "max", "topk"] - } -} - -impl fmt::Display for AggregationOperator { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) - } -} - -impl FromStr for AggregationOperator { - type Err = String; - fn from_str(s: &str) -> Result { - match s.to_lowercase().as_str() { - "sum" => Ok(AggregationOperator::Sum), - "count" => Ok(AggregationOperator::Count), - "avg" => Ok(AggregationOperator::Avg), - "quantile" => Ok(AggregationOperator::Quantile), - "min" => Ok(AggregationOperator::Min), - "max" => Ok(AggregationOperator::Max), - "topk" => Ok(AggregationOperator::Topk), - other => Err(format!("Unknown aggregation operator: '{other}'")), - } - } -} - -impl AggregationOperator { - /// Returns `true` for operators whose result requires approximate pre-aggregation. - pub fn is_approximate(self) -> bool { - matches!( - self, - AggregationOperator::Quantile - | AggregationOperator::Sum - | AggregationOperator::Count - | AggregationOperator::Avg - | AggregationOperator::Topk - ) - } -} - /// Concrete aggregation/sketch type used in precompute configs and accumulator dispatch. /// /// `Display` outputs the canonical PascalCase name used in YAML/JSON configs. @@ -413,33 +237,3 @@ impl<'de> Deserialize<'de> for AggregationType { s.parse().map_err(serde::de::Error::custom) } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_query_treatment_type_display() { - assert_eq!(QueryTreatmentType::Exact.to_string(), "exact"); - assert_eq!(QueryTreatmentType::Approximate.to_string(), "approximate"); - } - - #[test] - fn test_query_treatment_type_serialization() { - let exact = QueryTreatmentType::Exact; - let approximate = QueryTreatmentType::Approximate; - - // Test that they can be serialized/deserialized - let exact_str = serde_json::to_string(&exact).unwrap(); - let approximate_str = serde_json::to_string(&approximate).unwrap(); - - assert_eq!(exact_str, "\"Exact\""); - assert_eq!(approximate_str, "\"Approximate\""); - - let exact_back: QueryTreatmentType = serde_json::from_str(&exact_str).unwrap(); - let approximate_back: QueryTreatmentType = serde_json::from_str(&approximate_str).unwrap(); - - assert_eq!(exact_back, QueryTreatmentType::Exact); - assert_eq!(approximate_back, QueryTreatmentType::Approximate); - } -} diff --git a/crates/promql_utilities/src/query_logics/logics.rs b/crates/promql_utilities/src/query_logics/logics.rs deleted file mode 100644 index 9772c18f..00000000 --- a/crates/promql_utilities/src/query_logics/logics.rs +++ /dev/null @@ -1,257 +0,0 @@ -use crate::query_logics::enums::{ - AggregationOperator, AggregationType, PromQLFunction, QueryTreatmentType, Statistic, -}; -use tracing::debug; - -/// Map statistic to precompute operator based on treatment type. -/// -/// This is the **canonical primary picker** consulted by the planner when -/// emitting `IntermediateAggConfig`s from a query plan. For each -/// `(Statistic, QueryTreatmentType)` pair it returns one canonical -/// `AggregationType` plus an optional `aggregation_sub_type` string. -/// -/// **Source-of-truth invariant:** every `AggregationType` returned here for a -/// given `Statistic` must also appear in -/// `asap_types::capability_matching::compatible_agg_types(Statistic)`. The -/// invariant is enforced by the `capability_canonical_map_agreement` test in -/// `asap_types::capability_matching::tests` and is the single guard against -/// divergence between the planner-side and matcher-side capability tables. -/// -/// This mirrors the Python implementation's logic. -pub fn map_statistic_to_precompute_operator( - statistic: Statistic, - treatment_type: QueryTreatmentType, -) -> Result<(AggregationType, String), String> { - debug!( - "Mapping statistic {:?} with treatment type {:?} to precompute operator", - statistic, treatment_type - ); - match statistic { - Statistic::Quantile => { - if treatment_type == QueryTreatmentType::Exact { - Err("Statistic Quantile cannot be computed exactly".to_string()) - } else { - Ok((AggregationType::DatasketchesKLL, "".to_string())) - //Ok((AggregationType::HydraKLL, "".to_string())) - } - } - Statistic::Min | Statistic::Max => { - // Min/Max are always served by `MultipleMinMax` regardless of - // treatment type. The previous Approximate branch routed to - // `DatasketchesKLL`, but `DatasketchesKLLAccumulator::query` only - // implements `Statistic::Quantile` — KLL exposes no min/max query - // surface — so that branch produced configs whose runtime path - // would error. `MultipleMinMax` is exact and cheap; an - // approximate KLL-backed variant is reserved for a future KLL - // extension that exposes `min_value` / `max_value` through - // `query_statistic` (tracked alongside the accumulator-library - // work, not in this PR). - let _ = treatment_type; - Ok(( - AggregationType::MultipleMinMax, - statistic.to_string().to_lowercase(), - )) - } - Statistic::Sum | Statistic::Count => { - if treatment_type == QueryTreatmentType::Approximate { - Ok(( - AggregationType::CountMinSketch, - statistic.to_string().to_lowercase(), - )) - } else { - Ok(( - AggregationType::MultipleSum, - statistic.to_string().to_lowercase(), - )) - } - } - Statistic::Rate | Statistic::Increase => { - Ok((AggregationType::MultipleIncrease, "".to_string())) - } - Statistic::Topk => Ok((AggregationType::CountMinSketchWithHeap, "topk".to_string())), - _ => Err(format!("Statistic {statistic:?} not supported")), - } -} - -/// Check if a precompute operator supports subpopulations (multiple keys) -pub fn does_precompute_operator_support_subpopulations( - statistic: Statistic, - precompute_operator: AggregationType, -) -> bool { - debug!( - "Checking if precompute operator '{}' supports subpopulations for statistic {:?}", - precompute_operator, statistic - ); - match precompute_operator { - // Single-key operators - AggregationType::Increase - | AggregationType::MinMax - | AggregationType::Sum - | AggregationType::DatasketchesKLL => false, - - // Multi-key operators - AggregationType::MultipleIncrease - | AggregationType::MultipleMinMax - | AggregationType::MultipleSum - | AggregationType::HydraKLL => true, - - // CountMinSketch supports subpopulations only for certain statistics - AggregationType::CountMinSketch => matches!(statistic, Statistic::Sum | Statistic::Count), - - // CountMinSketchWithHeap is only supported for Topk — does not support subpopulations - AggregationType::CountMinSketchWithHeap if matches!(statistic, Statistic::Topk) => false, - - // CountSketch is the signed-counter equivalent of CMS — same - // subpopulation shape for Sum/Count statistics. The - // heap-bearing variant covers Topk like its CMS counterpart. - AggregationType::CountSketch => matches!(statistic, Statistic::Sum | Statistic::Count), - AggregationType::CountSketchWithHeap if matches!(statistic, Statistic::Topk) => false, - - // Default: not supported - _ => panic!("Unexpected precompute operator: {}", precompute_operator), - } -} - -/// Check if temporal and spatial aggregations are collapsible. -/// Based on Python implementation in promql_utilities/query_logics/logics.py -pub fn get_is_collapsable( - temporal_aggregation: PromQLFunction, - spatial_aggregation: AggregationOperator, -) -> bool { - debug!( - "Checking if temporal aggregation '{}' and spatial aggregation '{}' are collapsable", - temporal_aggregation, spatial_aggregation - ); - match spatial_aggregation { - AggregationOperator::Sum => matches!( - temporal_aggregation, - // Note: Increase and Rate are commented out in the Python reference - PromQLFunction::SumOverTime | PromQLFunction::CountOverTime - ), - AggregationOperator::Min => temporal_aggregation == PromQLFunction::MinOverTime, - AggregationOperator::Max => temporal_aggregation == PromQLFunction::MaxOverTime, - _ => false, - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_map_statistic_to_precompute_operator() { - // Test exact sum - let result = - map_statistic_to_precompute_operator(Statistic::Sum, QueryTreatmentType::Exact) - .unwrap(); - assert_eq!(result, (AggregationType::MultipleSum, "sum".to_string())); - - // Test approximate sum - let result = - map_statistic_to_precompute_operator(Statistic::Sum, QueryTreatmentType::Approximate) - .unwrap(); - assert_eq!(result, (AggregationType::CountMinSketch, "sum".to_string())); - - // Test exact quantile (should fail) - let result = - map_statistic_to_precompute_operator(Statistic::Quantile, QueryTreatmentType::Exact); - assert!(result.is_err()); - - // Test approximate quantile - let result = map_statistic_to_precompute_operator( - Statistic::Quantile, - QueryTreatmentType::Approximate, - ) - .unwrap(); - assert_eq!(result, (AggregationType::DatasketchesKLL, "".to_string())); - //assert_eq!(result, (AggregationType::HydraKLL, "".to_string())); - } - - #[test] - fn test_does_precompute_operator_support_subpopulations() { - // Test MultipleSum supports subpopulations - assert!(does_precompute_operator_support_subpopulations( - Statistic::Sum, - AggregationType::MultipleSum, - )); - - // Test DatasketchesKLL does not support subpopulations - assert!(!does_precompute_operator_support_subpopulations( - Statistic::Quantile, - AggregationType::DatasketchesKLL, - )); - - // Test HydraKLL supports subpopulations - assert!(does_precompute_operator_support_subpopulations( - Statistic::Quantile, - AggregationType::HydraKLL, - )); - - // Test CountMinSketch with valid statistic - assert!(does_precompute_operator_support_subpopulations( - Statistic::Sum, - AggregationType::CountMinSketch, - )); - - // Sibling CountSketch path — must not panic, matches - // CountMinSketch's Sum/Count subpopulation shape. - assert!(does_precompute_operator_support_subpopulations( - Statistic::Sum, - AggregationType::CountSketch, - )); - assert!(does_precompute_operator_support_subpopulations( - Statistic::Count, - AggregationType::CountSketch, - )); - - // Heap-bearing variants on Topk — both return false (heap - // is per-policy, not subpopulation-keyed) and must not panic. - assert!(!does_precompute_operator_support_subpopulations( - Statistic::Topk, - AggregationType::CountMinSketchWithHeap, - )); - assert!(!does_precompute_operator_support_subpopulations( - Statistic::Topk, - AggregationType::CountSketchWithHeap, - )); - } - - #[test] - fn test_topk_maps_to_count_min_sketch_with_heap() { - let result = - map_statistic_to_precompute_operator(Statistic::Topk, QueryTreatmentType::Approximate) - .unwrap(); - assert_eq!( - result, - (AggregationType::CountMinSketchWithHeap, "topk".to_string()) - ); - } - - #[test] - fn test_get_is_collapsable() { - assert!(get_is_collapsable( - PromQLFunction::SumOverTime, - AggregationOperator::Sum - )); - assert!(get_is_collapsable( - PromQLFunction::CountOverTime, - AggregationOperator::Sum - )); - assert!(get_is_collapsable( - PromQLFunction::MinOverTime, - AggregationOperator::Min - )); - assert!(get_is_collapsable( - PromQLFunction::MaxOverTime, - AggregationOperator::Max - )); - assert!(!get_is_collapsable( - PromQLFunction::MinOverTime, - AggregationOperator::Sum - )); - assert!(!get_is_collapsable( - PromQLFunction::Rate, - AggregationOperator::Sum - )); - } -} diff --git a/crates/promql_utilities/src/query_logics/mod.rs b/crates/promql_utilities/src/query_logics/mod.rs index 8ca2b7df..7071c4c3 100644 --- a/crates/promql_utilities/src/query_logics/mod.rs +++ b/crates/promql_utilities/src/query_logics/mod.rs @@ -1,5 +1,3 @@ pub mod enums; -pub mod logics; pub use enums::*; -pub use logics::*;