diff --git a/crates/asap-aware-mapping/src/accuracy.rs b/crates/asap-aware-mapping/src/accuracy.rs index a132eb39..1590d734 100644 --- a/crates/asap-aware-mapping/src/accuracy.rs +++ b/crates/asap-aware-mapping/src/accuracy.rs @@ -541,7 +541,7 @@ impl DefaultAccuracyModel { } let mut provenance = Vec::new(); let count = row_count(stats, &mut provenance); - let exact_local = ResultGuarantee::exact("ExactAggregate(MinMax)"); + let exact_local = ResultGuarantee::exact("ExactAggregate(Max)"); provenance.extend(composed_provenance( op, inputs, diff --git a/crates/asap-aware-mapping/src/function_rules.rs b/crates/asap-aware-mapping/src/function_rules.rs index e5c5f7ba..0b96e018 100644 --- a/crates/asap-aware-mapping/src/function_rules.rs +++ b/crates/asap-aware-mapping/src/function_rules.rs @@ -21,7 +21,7 @@ pub(crate) fn function_rules(intent: &AggIntent) -> Option { ), AggIntent::Max { .. } => ( CompositionOperator::ExactExtremum, - Some((ExactKind::MinMax, ExactParams::MinMax)), + Some((ExactKind::Max, ExactParams::Max)), ), AggIntent::Avg { .. } => (CompositionOperator::ExactAverage, None), AggIntent::Rate => ( diff --git a/crates/asap-aware-mapping/src/replacement.rs b/crates/asap-aware-mapping/src/replacement.rs index deef4095..d2ddb2a1 100644 --- a/crates/asap-aware-mapping/src/replacement.rs +++ b/crates/asap-aware-mapping/src/replacement.rs @@ -632,7 +632,7 @@ pub trait ReplacementStrategy { #[derive(Debug, Clone, PartialEq)] pub enum Implementation { /// An exact **mergeable** accumulator (partial state ≡ the value - /// itself: `Sum` / `Count` / `MinMax` / `Rate` / `Increase`). The + /// itself: `Sum` / `Count` / `Min` / `Max` / `Rate` / `Increase`). The /// built state *is* the answer already — no `SummaryEstimate` readout /// step. ExactAggregate { @@ -5921,7 +5921,7 @@ mod tests { // exact mergeable accumulators (A::Sum { col: None }, Acc(E::Sum)), (A::Min { col: None }, Acc(E::Min)), - (A::Max { col: None }, Acc(E::MinMax)), + (A::Max { col: None }, Acc(E::Max)), (A::Rate, Acc(E::Rate)), (A::IRate, Acc(E::IRate)), (A::Increase, Acc(E::Increase)), diff --git a/crates/asap-aware-mapping/src/rollup.rs b/crates/asap-aware-mapping/src/rollup.rs index a5abae8a..351515dd 100644 --- a/crates/asap-aware-mapping/src/rollup.rs +++ b/crates/asap-aware-mapping/src/rollup.rs @@ -143,7 +143,7 @@ fn bindable_grouped_aggregate( /// reasoning behind each arm. `None` for any intent this module does not /// (yet) know a correct combinator for — including every intent /// `agg_is_mergeable` permits but this module doesn't specifically handle -/// (`Rate`, and everything outside the `Sum`/`Count`/`MinMax`/`Increase` +/// (`Rate`, and everything outside the `Sum`/`Count`/`Min`/`Max`/`Increase` /// vocabulary `agg_is_mergeable`'s own doc names) — so `is_legal_rollup_source` /// (which calls this) is *strictly narrower* than `agg_is_mergeable` alone, /// deliberately: `agg_is_mergeable` answers "does *some* partial-state merge diff --git a/crates/frontend-promql/tests/promql_lowering.rs b/crates/frontend-promql/tests/promql_lowering.rs index 128783f9..fb2af49b 100644 --- a/crates/frontend-promql/tests/promql_lowering.rs +++ b/crates/frontend-promql/tests/promql_lowering.rs @@ -380,6 +380,55 @@ fn count_over_rate_keeps_both_levels() { )); } +#[test] +fn count_over_distinct_over_time_preserves_both_aggregates() { + // One series with window samples [1, 2] produces one distinct-count + // result (value 2). The outer count counts that one series, yielding 1. + for (query, reduction) in [ + ( + "count(distinct_over_time(unique_users[5m]))", + Reduction::by(vec![]), + ), + ( + "count by (job) (distinct_over_time(unique_users[5m]))", + Reduction::by(vec![2]), + ), + ] { + let tree = lower(query); + let QueryExpr::Aggregate { + measures, + reduction: actual, + child, + .. + } = &tree + else { + panic!("expected outer Count: {tree:?}"); + }; + assert!( + matches!(measures.as_slice(), [AggIntent::Count { .. }]), + "{query}: {tree:?}" + ); + assert_eq!(actual, &reduction, "{query}"); + let QueryExpr::Aggregate { + measures, + reduction, + child, + .. + } = child.as_ref() + else { + panic!("expected inner per-series Cardinality: {tree:?}"); + }; + assert!( + matches!(measures.as_slice(), [AggIntent::Cardinality { .. }]), + "{query}: {tree:?}" + ); + assert_eq!(reduction, &Reduction::PerEntity, "{query}"); + assert!( + matches!(child.as_ref(), QueryExpr::TimeRange { range, .. } if range.as_secs() == 300) + ); + } +} + // ── count / cardinality ─────────────────────────────────────────────────────── #[test] diff --git a/crates/integration-tests/tests/exact_composition.rs b/crates/integration-tests/tests/exact_composition.rs index 86b5ad1f..de138d48 100644 --- a/crates/integration-tests/tests/exact_composition.rs +++ b/crates/integration-tests/tests/exact_composition.rs @@ -261,7 +261,7 @@ fn every_exact_accumulator_is_finalized_before_an_outer_sketch() { AggIntent::Max { col: None }, Rc::new(metric_scan(&["zone"])), ), - ExactKind::MinMax, + ExactKind::Max, ), ( per_entity( @@ -616,8 +616,8 @@ fn readout_under_maintenance_is_rejected_at_construction() { expr: SummaryExpr::SummaryAgg { child: post, family: SummaryFamilyType::ExactAggregate( - ExactKind::MinMax, - asap_types::post_asap::ExactParams::MinMax, + ExactKind::Max, + asap_types::post_asap::ExactParams::Max, ), input: SummaryUpdate::column(asap_types::pre_asap::ColumnRef::SampleValue), reduction: Reduction::by(vec![]), diff --git a/crates/types/src/post_asap/schema.rs b/crates/types/src/post_asap/schema.rs index a61abcef..ffef2a05 100644 --- a/crates/types/src/post_asap/schema.rs +++ b/crates/types/src/post_asap/schema.rs @@ -24,7 +24,7 @@ pub enum SummaryFamilyType { /// pre-ASAP `DataType` (`Int64`/`Float64`/`Utf8`/`Bool`/`Timestamp`), /// passed through unchanged from a pre-ASAP edge. Plain(DataType), - /// Exact, mergeable accumulator state (`Sum`/`Count`/`MinMax`/`Rate`/ + /// Exact, mergeable accumulator state (`Sum`/`Count`/`Min`/`Max`/`Rate`/ /// `Increase`) — the partial state *is* the value; no readout needed. ExactAggregate(ExactKind, ExactParams), /// Approximate sketch state (KLL/CMS/HLL/…), read out via a diff --git a/crates/types/src/post_asap/sketch.rs b/crates/types/src/post_asap/sketch.rs index 6a14e126..9d870a6c 100644 --- a/crates/types/src/post_asap/sketch.rs +++ b/crates/types/src/post_asap/sketch.rs @@ -13,10 +13,10 @@ pub enum ExactKind { Sum, /// Exact count accumulator (mergeable by addition). Count, - /// Exact min/max accumulator (mergeable by comparison). - MinMax, - /// Exact minimum, distinct from the legacy maximum accumulator. + /// Exact minimum accumulator (mergeable by comparison). Min, + /// Exact maximum accumulator (mergeable by comparison). + Max, /// Exact increase accumulator (counter-reset-aware delta). Increase, /// Rate accumulator (increase / time window duration). @@ -33,9 +33,8 @@ pub enum ExactKind { pub enum ExactParams { Sum, Count, - MinMax, - /// Exact minimum, distinct from the legacy maximum accumulator. Min, + Max, Increase, Rate, IRate,