Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 29 additions & 24 deletions control_plane/src/sketch_algebra/capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,19 @@ pub fn capability_for(intent: &AggIntent) -> Option<Capability> {
// accuracy is non-Exact and we hand it to the cardinality
// sketch path.
//
// PR-6 follow-up: exact count = sum-of-1s, which is
// served by the `AggregationType::Sum` exact-precompute
// operator at the ASAP tier. Returning that capability
// lets the analyzer route `count_over_time` to a ASAP-tier
// ExactAgg sid instead of falling through to archive.
// Exact count routes to archive (`None`). The PR #200/#201
// follow-up flipped this to `ExactAgg(Sum)` on the theory
// "count = sum-of-1s" — but the data plane has no count
// accumulator. `SumAccumulator` only tracks `sum: f64` and
// its `query` returns `self.sum` for BOTH `Statistic::Sum`
// and `Statistic::Count`, so a `count_over_time` query
// matched against a `Sum` policy returns the sum of the
// sample VALUES, not the count of samples. Reverted here
// until a real `SumCountAccumulator` lands (the
// temporal/spatial-split work) — archive counts correctly
// in the meantime.
if is_exact(accuracy) {
Some(Capability::ExactAgg(AggregationType::Sum))
None
} else {
Some(Capability::CardinalityApprox)
}
Expand Down Expand Up @@ -691,18 +697,18 @@ mod tests {
}

#[test]
fn capability_for_count_exact_routes_to_exact_agg_sum() {
// PR-6 follow-up: `count_over_time` lowers to
// `Count{accuracy:Exact}`; count = sum-of-1s, so the ASAP-tier
// ExactAgg path uses `AggregationType::Sum`. Pre-follow-up
// this returned `None` and the analyzer routed to archive.
fn capability_for_count_exact_routes_to_archive() {
// `count_over_time` lowers to `Count{accuracy:Exact}`. The
// PR #200/#201 follow-up briefly routed this to
// `ExactAgg(Sum)`, but the data plane has no count
// accumulator — `SumAccumulator` returns its `sum` for both
// `Statistic::Sum` and `Statistic::Count`, so the result was
// sum-of-values, not sample-count. Reverted to `None` (archive
// routing) until a real `SumCountAccumulator` lands.
let intent = AggIntent::Count {
accuracy: AccuracyTarget::Exact,
};
assert_eq!(
capability_for(&intent),
Some(Capability::ExactAgg(AggregationType::Sum))
);
assert_eq!(capability_for(&intent), None);
}

#[test]
Expand Down Expand Up @@ -1026,12 +1032,10 @@ mod tests {
// ── capability_for: ExactAgg dormancy ────────────────────────────────

#[test]
fn pr_6_follow_up_flipped_sum_rate_increase_count_exact() {
// PR 6 first landed `Capability::ExactAgg` dormant — variant
// wired into `is_satisfied_by` but `capability_for` still
// returned `None` for Sum / Rate / Increase / Count{Exact}.
// This test locks in the follow-up that flipped those four
// intents to route through ASAP-tier ExactAgg state.
fn exact_agg_routing_covers_sum_rate_increase_only() {
// `Capability::ExactAgg` routing covers the three intents the
// data plane has a real accumulator for: `Sum` (SumAccumulator)
// and `Rate` / `Increase` (IncreaseAccumulator).
assert_eq!(
capability_for(&AggIntent::Sum),
Some(Capability::ExactAgg(AggregationType::Sum))
Expand All @@ -1048,14 +1052,15 @@ mod tests {
}),
Some(Capability::ExactAgg(AggregationType::Increase))
);
// `Count{Exact}` (count_over_time) and `Avg` both need a real
// count accumulator that doesn't exist yet — they route to
// archive until `SumCountAccumulator` lands.
assert_eq!(
capability_for(&AggIntent::Count {
accuracy: AccuracyTarget::Exact,
}),
Some(Capability::ExactAgg(AggregationType::Sum))
None
);
// Avg stays on archive — needs cross-policy join (Sum + Count)
// that the L4 binder doesn't yet emit. Tracked as follow-up.
assert_eq!(capability_for(&AggIntent::Avg), None);
}

Expand Down
59 changes: 30 additions & 29 deletions control_plane/src/sketch_algebra/rules/bind_exact_agg.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! `BindExactAgg` — emits `PhysicalExpr::ExactAgg` for the four exact-aggregation
//! intents that the PR 6 follow-up flipped to ASAP-tier routing.
//! `BindExactAgg` — emits `PhysicalExpr::ExactAgg` for the exact-aggregation
//! intents the ASAP tier can serve from a precompute accumulator.
//!
//! ## Coverage
//!
Expand All @@ -8,24 +8,25 @@
//! | `Sum` | `agg_type: AggregationType::Sum` |
//! | `Rate { .. }` | `agg_type: AggregationType::Increase` |
//! | `Increase { .. }` | `agg_type: AggregationType::Increase` |
//! | `Count { accuracy: Exact }` | `agg_type: AggregationType::Sum` |
//!
//! `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.
//!
//! `Count{accuracy:Exact}` maps to `Sum` because `count_over_time` is
//! exactly the sum of presence indicators (each sample contributes 1).
//! There's no dedicated `Count` `AggregationType` variant; `Sum`
//! covers the shape.
//!
//! ## What this rule does NOT bind
//!
//! - `AggIntent::Avg` — needs cross-policy join (Sum / Count), no single
//! `AggregationType` covers it. Stays on archive until the L4 binder
//! gains a join rule.
//! - `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
Expand Down Expand Up @@ -111,18 +112,10 @@ impl Rule for BindExactAgg {
AggregationType::Increase
}
}
AggIntent::Count {
accuracy: AccuracyTarget::Exact,
} => {
// count_over_time = sum-of-1s, so it lowers through the
// Sum/MultipleSum accumulator family — same as
// `AggIntent::Sum` above.
if keyed {
AggregationType::MultipleSum
} else {
AggregationType::Sum
}
}
// `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,
};

Expand Down Expand Up @@ -197,13 +190,17 @@ mod tests {
}

#[test]
fn binds_count_exact_to_exact_agg_sum() {
check_binds(
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,
},
AggregationType::Sum,
"test_metric",
);
assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none());
}

#[test]
Expand Down Expand Up @@ -282,13 +279,17 @@ mod tests {
}

#[test]
fn keyed_count_exact_binds_to_multiple_sum() {
check_keyed_binds(
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,
},
AggregationType::MultipleSum,
"test_metric",
vec![0],
);
assert!(BindExactAgg.apply(&expr, &AccuracyTarget::Exact).is_none());
}

#[test]
Expand Down