From 67f0b42f549692b1506d35c72d7a75edda619979 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:10:15 +0000 Subject: [PATCH 1/4] feat(plan-selection): declare non-negative gauges so quantile sketches compose through avg (#772) - SourceFacts::values_non_negative declares every sample of a metric non-negative, for a gauge as well as a counter (default false, omitted from JSON when false). - Plan selection binds the sign of a metric's samples (values_non_negative[m]) to proven when the metric is declared a counter or declared non-negative. - A quantile estimate over a metric's raw samples now carries that metric's sign statistic, so a per-series DDSketch's relative error composes through sum and avg once the sign is declared. - Test: {avg,sum,max} by (region) (quantile_over_time(0.99, x[100s])) with x a gauge, with and without the declaration. - Docs: the new field in the library API and the input/output workflow. --- .../src/per_computation_candidates.rs | 23 +++++- crates/plan-selection/src/lib.rs | 76 ++++++++++++++++++- crates/types/src/workload/mod.rs | 10 +++ .../architecture/input-output-workflow.md | 2 + docs/develop_docs/library-api.md | 4 +- 5 files changed, 109 insertions(+), 6 deletions(-) diff --git a/crates/logical-asap-optimization/src/per_computation_candidates.rs b/crates/logical-asap-optimization/src/per_computation_candidates.rs index 59a2f0953..df7c7fc94 100644 --- a/crates/logical-asap-optimization/src/per_computation_candidates.rs +++ b/crates/logical-asap-optimization/src/per_computation_candidates.rs @@ -1660,8 +1660,10 @@ fn fold_stats(reduction: &Reduction, input: &Rc) -> Option Option { match &node.operator { Operator::ASAP(ASAPOp::FinalizeExactAccumulator { child }) => match child.asap()? { @@ -1673,6 +1675,23 @@ fn estimates_samples_of(node: &OperatorNode) -> Option { } => estimates_samples_of(child), _ => None, }, + Operator::ASAP(ASAPOp::SummaryEstimate { + summary_input, + query: SketchStatistic::Quantile { .. }, + }) => match summary_input.asap()? { + ASAPOp::SummaryAgg { + input: + SummaryUpdate { + item: None, + weight: SummaryInputExpr::Column(ColumnRef::SampleValue), + .. + }, + filter: None, + child, + .. + } => estimates_samples_of(child), + _ => None, + }, Operator::ASAP(ASAPOp::SummaryEstimate { summary_input, .. }) => { match summary_input.asap()? { ASAPOp::SummaryAgg { diff --git a/crates/plan-selection/src/lib.rs b/crates/plan-selection/src/lib.rs index ead610339..a106201d5 100644 --- a/crates/plan-selection/src/lib.rs +++ b/crates/plan-selection/src/lib.rs @@ -1723,6 +1723,17 @@ pub fn confirmed_weights(domain: &WeightDomain, data: &DataWorkload) -> WeightDo domain.confirmed(|metric| declared_counter(data, metric)) } +/// Whether `data` declares every sample of `metric` non-negative: the +/// metric is declared a counter, or a metric (a gauge, say) with +/// [`SourceFacts::values_non_negative`](asap_types::workload::SourceFacts::values_non_negative). +fn declared_non_negative(data: &DataWorkload, metric: &str) -> bool { + declared_counter(data, metric) + || data + .sources + .get(metric) + .is_some_and(|facts| facts.values_non_negative) +} + /// Whether `data` declares `metric` a counter. A declared type does not /// expire, so its evidence is not checked. fn declared_counter(data: &DataWorkload, metric: &str) -> bool { @@ -1844,9 +1855,10 @@ fn bind_statistics( /// metric's samples, from its declared type. fn workload_statistic(statistic: &str, series: Option, data: &DataWorkload) -> Option { // The sign of one metric's samples: proven (1) when the metric is - // declared a counter; no type is inferred from its name. + // declared a counter or declared non-negative; nothing is inferred from + // its name. if let Some(metric) = parse_samples_sign_statistic(statistic) { - return declared_counter(data, metric).then_some(1.0); + return declared_non_negative(data, metric).then_some(1.0); } if statistic == INPUT_ROW_COUNT { return series.map(|rows| rows as f64); @@ -5218,6 +5230,66 @@ mod tests { } } + /// The candidates of `query` that build a DDSketch. + fn ddsketch_candidates(candidates: &[PhysicalCandidate]) -> Vec { + candidates + .iter() + .filter(|c| { + c.dag.nodes.iter().any(|node| { + matches!(&node.payload, + Payload::ASAP(ASAPOp::SummaryAgg { family: FieldDataType::Sketch(kind, _), .. }) + if kind.algorithm() == &SketchAlgorithm::DDSketch) + }) + }) + .map(|c| c.id.clone()) + .collect() + } + + /// A per-series DDSketch quantile is within a relative error, which + /// carries through `avg` and `sum` only over non-negative values, and + /// through `max` whatever their sign. With `x` declared a gauge whose + /// samples are non-negative, the DDSketch candidate of + /// `avg by (region) (quantile_over_time(0.99, x[100s]))` (and of its `sum`) + /// is valid and accepted with a relative-error guarantee; without the + /// declaration it is rejected. `max` is valid either way (issue #772). + #[test] + fn a_non_negative_gauge_carries_a_ddsketch_relative_bound_through_avg() { + let target = AccuracyTarget::max_error(0.01) + .failure_probability(0.01) + .accuracy_metric(AccuracyMetric::Relative); + for (outer, needs_sign) in [("avg", true), ("sum", true), ("max", false)] { + let query = format!("{outer} by (region) (quantile_over_time(0.99, x[100s]))"); + let candidates = all_candidates(&query, &target); + let ddsketch = ddsketch_candidates(&candidates); + assert!(!ddsketch.is_empty(), "{query}"); + for values_non_negative in [true, false] { + let mut data = with_metric_types(data(), &[("x", MetricType::Gauge)]); + data.sources.get_mut("x").unwrap().values_non_negative = values_non_negative; + let selection = select_plan( + &candidates, + &[entry_with(target.clone(), repeating(10_000))], + &data, + PlanningModels::builtin(), + ) + .unwrap(); + for id in &ddsketch { + let accepted = selection.accepted.get(id); + if values_non_negative || !needs_sign { + let [accepted] = accepted.map(Vec::as_slice).unwrap_or_default() else { + panic!("{query}, {id} not valid: {:?}", selection.rejected); + }; + assert_eq!(accepted.accepted_metric, Some(AccuracyMetric::Relative)); + assert!(!accepted.accepted_regardless_of_guarantee); + } else { + assert!(accepted.is_none(), "{query}, {id}: {accepted:?}"); + let rejection = selection.rejected.iter().find(|r| &r.id == id).unwrap(); + assert!(rejection.cause.failed_a_check(), "{rejection:?}"); + } + } + } + } + } + /// A candidate that ties the selected one is rejected for losing on /// enumeration order, not as costlier. #[test] diff --git a/crates/types/src/workload/mod.rs b/crates/types/src/workload/mod.rs index 4b5092f26..57faa75c1 100644 --- a/crates/types/src/workload/mod.rs +++ b/crates/types/src/workload/mod.rs @@ -510,6 +510,14 @@ pub struct SourceFacts { /// undeclared metric's samples may take any value. #[serde(default)] pub metric_type: Option, + /// Whether every sample of the metric is non-negative (default `false`: + /// unknown). A counter's samples always are; this declares it for a + /// gauge, such as a latency or a queue length. The deployment must + /// ensure it. Plan selection reads it as the sign of the metric's + /// samples, which a relative-error bound needs to carry through a + /// `sum` or `avg`. Omitted from JSON when `false`. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub values_non_negative: bool, /// PromQL and MetricsQL only: how often each series of the metric /// receives a sample (default: unknown). A bare instant selector over /// the metric reads the latest sample within it; lowering such a @@ -1168,6 +1176,7 @@ mod tests { SourceFacts { series_count: Some(10_000), metric_type: Some(MetricType::Counter), + values_non_negative: true, sample_interval: Some(DurationMs(15_000)), stream_norms: Some(StreamNorms { l1: Some(1e9), @@ -1187,6 +1196,7 @@ mod tests { "sources": {"requests_total": { "series_count": 10000, "metric_type": "counter", + "values_non_negative": true, "sample_interval": 15000, "stream_norms": {"l1": 1e9, "l2": null}, "max_distinct_per_evaluation": 100 diff --git a/docs/design_docs/architecture/input-output-workflow.md b/docs/design_docs/architecture/input-output-workflow.md index f2785ca72..afad31083 100644 --- a/docs/design_docs/architecture/input-output-workflow.md +++ b/docs/design_docs/architecture/input-output-workflow.md @@ -196,6 +196,7 @@ enum DataArrival { struct SourceFacts { series_count: Option, metric_type: Option, + values_non_negative: bool, sample_interval: Option, stream_norms: Option, max_distinct_per_evaluation: Option, @@ -208,6 +209,7 @@ struct SourceFacts { | `rows_per_second` | With `Ingesting`/`Mixed` | Rows arriving per second over all sources, used to price scans and continuous maintenance. It must be finite and non-negative. | | `series_count` | Optional | Series in the source. Plan selection bounds a query's number of values `G`, and its input row count, by the series counts of the sources it reads (each branch of an `or` counted); unknown, they stay unbound. | | `metric_type` | Optional | Declared Prometheus type; a counter proves samples non-negative. Never inferred from a name. | +| `values_non_negative` | Optional; default `false` | Declares every sample non-negative, for a gauge as well as a counter. It is the sign a relative-error bound needs to carry through `sum` and `avg` (a per-series DDSketch quantile under `avg by`, say); without it, and for a gauge, that bound is unknown. | | `sample_interval` | PromQL and MetricsQL; nonzero | How often each series of the metric is sampled. Bare instant selectors over it use it as their selection horizon; without it, such a query fails to lower. | | `stream_norms`, `max_distinct_per_evaluation` | Optional | Bounds that Count-Min/Count Sketch and HLL/UnivMon guarantees are bound with. | diff --git a/docs/develop_docs/library-api.md b/docs/develop_docs/library-api.md index d8a12f20f..0d3401e67 100644 --- a/docs/develop_docs/library-api.md +++ b/docs/develop_docs/library-api.md @@ -290,8 +290,8 @@ query's window comes from its text (range and offset). These facts are separate: `DataWorkload` holds `arrival` (`DataArrival`: `AtRest`, `Ingesting` or `Mixed` with `rows_per_second`, or `Unknown`) and `sources`, a map from PromQL metric or SQL table name to `SourceFacts`: `series_count`, -`metric_type`, `sample_interval` (PromQL and MetricsQL), `stream_norms` and -`max_distinct_per_evaluation`. Every fact is declared: the planner has no +`metric_type`, `values_non_negative`, `sample_interval` (PromQL and +MetricsQL), `stream_norms` and `max_distinct_per_evaluation`. Every fact is declared: the planner has no clock, so a caller drops facts that no longer hold before planning. Plan selection bounds a query's number of values `G`, and its input row count, by the `series_count` of the sources it reads, counting each branch of an From 61b1165887eed2d4d2084502aa74ae172dfdde11 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:24:59 +0000 Subject: [PATCH 2/4] fix(logical): an offset keeps a metric's sample sign (#773 review) samples_of_metric now passes through an offset-only TimeShift: it moves the window, not the samples, so their sign is the metric's. With it, topk(10, sum by (job) (rate(data_total[5m] offset 1h))) over a declared counter keeps its non-negative Count-Min weights, and a per-series DDSketch quantile over `x[100s] offset 1h` of a non-negative gauge keeps its relative bound through avg. An `@` anchor still ends it. Tests: the offset forms of both acceptance tests. --- .../src/per_computation_candidates.rs | 8 ++++--- crates/plan-selection/src/lib.rs | 21 ++++++++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/logical-asap-optimization/src/per_computation_candidates.rs b/crates/logical-asap-optimization/src/per_computation_candidates.rs index df7c7fc94..62956e608 100644 --- a/crates/logical-asap-optimization/src/per_computation_candidates.rs +++ b/crates/logical-asap-optimization/src/per_computation_candidates.rs @@ -940,9 +940,10 @@ pub(crate) fn partitions_over( } /// The metric whose raw samples, or plain sums of them, are every value -/// `node` outputs, or `None`. Only a time-series scan, a time range over it -/// and a plain `sum` keep that; any other operator (rate and arithmetic -/// included) ends it. The metric's type is not read here: whether its +/// `node` outputs, or `None`. Only a time-series scan, a time range over it, +/// an `offset`-only time shift (it moves the window, not the samples) and a +/// plain `sum` keep that; any other operator (rate and arithmetic included) +/// ends it. The metric's type is not read here: whether its /// samples are non-negative is plan selection's question /// ([`WeightDomain::SamplesOfMetric`]). pub(crate) fn samples_of_metric(node: &OperatorNode) -> Option { @@ -952,6 +953,7 @@ pub(crate) fn samples_of_metric(node: &OperatorNode) -> Option { .. } => Some(metric.clone()), NonASAPOp::TimeRange { child, .. } => samples_of_metric(child), + NonASAPOp::TimeShift { shift, child } if shift.at.is_none() => samples_of_metric(child), NonASAPOp::Aggregate { measures, child, .. } if matches!(measures.as_slice(), [AggIntent::Sum { col: None }]) => { diff --git a/crates/plan-selection/src/lib.rs b/crates/plan-selection/src/lib.rs index a106201d5..0fa74d040 100644 --- a/crates/plan-selection/src/lib.rs +++ b/crates/plan-selection/src/lib.rs @@ -3875,9 +3875,14 @@ mod tests { let target = AccuracyTarget::max_error(0.01) .failure_probability(0.001) .accuracy_metric(AccuracyMetric::ABSOLUTE_STREAM_L1); - for function in ["increase", "rate", "irate"] { - let query = format!("topk(10, sum by (job) ({function}(data_total[5m])))"); - let candidates = all_candidates(&query, &target); + // An `offset` moves the window, not the samples' sign. + for query in [ + "topk(10, sum by (job) (increase(data_total[5m])))", + "topk(10, sum by (job) (rate(data_total[5m])))", + "topk(10, sum by (job) (irate(data_total[5m])))", + "topk(10, sum by (job) (rate(data_total[5m] offset 1h)))", + ] { + let candidates = all_candidates(query, &target); let count_min: Vec<&str> = candidates .iter() .filter(|c| count_min_builds(c).contains(&SketchAlgorithm::CmsWithHeap)) @@ -5257,8 +5262,14 @@ mod tests { let target = AccuracyTarget::max_error(0.01) .failure_probability(0.01) .accuracy_metric(AccuracyMetric::Relative); - for (outer, needs_sign) in [("avg", true), ("sum", true), ("max", false)] { - let query = format!("{outer} by (region) (quantile_over_time(0.99, x[100s]))"); + // An `offset` moves the window, not the samples' sign. + for (outer, window, needs_sign) in [ + ("avg", "x[100s]", true), + ("sum", "x[100s]", true), + ("max", "x[100s]", false), + ("avg", "x[100s] offset 1h", true), + ] { + let query = format!("{outer} by (region) (quantile_over_time(0.99, {window}))"); let candidates = all_candidates(&query, &target); let ddsketch = ddsketch_candidates(&candidates); assert!(!ddsketch.is_empty(), "{query}"); From 186d0dd665c00443afd17d54e1cedc6e6473e47c Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:24:59 +0000 Subject: [PATCH 3/4] fix(promql): topk over a per-series irate is a heavy-hitter ranking (#773 review) `topk(k, irate(m[w]))` lowered to an exact sort and limit while `topk(k, rate(m[w]))` got the heavy-hitter TopK intent. irate now takes the same direct path as rate and increase (and the matching direct_counter_ranking check). Tests: the lowering of topk(3, rate|increase|irate(...)) and a plan-selection check that the direct irate ranking plans. --- crates/frontend-promql/src/promql.rs | 12 ++++++++++-- .../tests/promql_conformance.rs | 17 +++++++++++++++++ crates/plan-selection/src/lib.rs | 19 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/crates/frontend-promql/src/promql.rs b/crates/frontend-promql/src/promql.rs index 89775fc81..53cfdf9c1 100644 --- a/crates/frontend-promql/src/promql.rs +++ b/crates/frontend-promql/src/promql.rs @@ -805,7 +805,10 @@ fn build_over_sub_dag(outer: Outer, keys: Vec, child: Unresolved) -> ); let direct_counter_ranking = matches!(&child, Unresolved::Aggregate { measures, reduction: Reduction::PerEntity, .. - } if matches!(measures.as_slice(), [AggIntent::Rate | AggIntent::Increase])); + } if matches!( + measures.as_slice(), + [AggIntent::Rate | AggIntent::Increase | AggIntent::IRate] + )); if descending && (weighted_counter_ranking || direct_counter_ranking) { return Ok(outer_aggregate( keys, @@ -1708,7 +1711,12 @@ fn build(inner: Inner, keys: Vec, outer: Outer) -> Result // Preserve the counter-value ranking intent. Physical candidates // may rebuild a heap over finalized rates or use exact Sort/Limit; // neither is allowed to sum raw counter samples as ranking weights. - if descending && matches!(inner.func, Some(InnerFunc::Rate | InnerFunc::Increase)) { + if descending + && matches!( + inner.func, + Some(InnerFunc::Rate | InnerFunc::Increase | InnerFunc::IRate) + ) + { let intent = inner_intent(inner.func.as_ref().expect("counter function")); let ranked = windowed_aggregate(inner, vec![], intent)?; return Ok(Unresolved::Aggregate { diff --git a/crates/frontend-promql/tests/promql_conformance.rs b/crates/frontend-promql/tests/promql_conformance.rs index a259a2931..8faca896c 100644 --- a/crates/frontend-promql/tests/promql_conformance.rs +++ b/crates/frontend-promql/tests/promql_conformance.rs @@ -832,6 +832,23 @@ fn topk_over_nested_sum_preserves_weighted_topk_accuracy() { assert!(has(&qe, |i| matches!(i, AggIntent::TopK { .. }))); } +#[test] +fn topk_over_a_per_series_counter_derivative_is_a_heavy_hitter_ranking() { + // `topk(k, rate|increase|irate(m[w]))` ranks each series' derivative: a + // heavy-hitter TopK intent directly over the per-series derivative, for + // `irate` as for `rate` and `increase`. + for function in ["rate", "increase", "irate"] { + let qe = ok(&format!("topk(3, {function}(node_cpu_seconds_total[5m]))")); + let NonASAPOp::Aggregate { measures, .. } = qe.expect_non_asap() else { + panic!("{function}: expected a TopK aggregate, got {qe:?}"); + }; + assert!( + matches!(measures.as_slice(), [AggIntent::TopK { k: 3, .. }]), + "{function}: {measures:?}" + ); + } +} + #[test] fn outer_aggregate_over_nested_aggregate_nests() { // `max(sum by (job) (rate(m[5m])))` — an outer cross-series reduction over a diff --git a/crates/plan-selection/src/lib.rs b/crates/plan-selection/src/lib.rs index 0fa74d040..f91fd4d3e 100644 --- a/crates/plan-selection/src/lib.rs +++ b/crates/plan-selection/src/lib.rs @@ -5301,6 +5301,25 @@ mod tests { } } + /// `topk(k, irate(m[w]))`, now a heavy-hitter ranking as `rate`'s is, + /// still plans: Stages 1-3 compose its candidates and select one. + #[test] + fn a_direct_irate_ranking_plans() { + let target = AccuracyTarget::max_error(0.01).failure_probability(0.001); + for function in ["rate", "irate"] { + let query = format!("topk(10, {function}(data_total[5m]))"); + let candidates = all_candidates(&query, &target); + let data = with_metric_types(data(), &[("data_total", MetricType::Counter)]); + let selection = select_plan( + &candidates, + &[entry_with(target.clone(), repeating(10_000))], + &data, + PlanningModels::builtin(), + ); + assert!(selection.is_ok(), "{query}: {selection:?}"); + } + } + /// A candidate that ties the selected one is rejected for losing on /// enumeration order, not as costlier. #[test] From f62062dd8de0b7eff11cef06ec46a042368c692b Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:20:41 +0000 Subject: [PATCH 4/4] fix(logical): a quantile with phi outside [0, 1] gets no sketch and no sample sign (#772 review) PromQL answers quantile_over_time(phi, ...) with phi < 0 or phi > 1 as -Inf or +Inf, which no sketch estimates and the executor's DDSketch and KLL refuse at run time. - Pass 1 prunes every sketch candidate of such a quantile with the new PruneReason::QuantileOutsideUnitInterval, so only the exact plan is offered and selected (this also fixes bare `quantile_over_time(2, x)` and `max by (...) (quantile_over_time(-1, x))`, which selected a sketch plan before). - estimates_samples_of follows a quantile estimate only for phi in [0, 1], so such an estimate never carries its samples' sign. Tests: the four reviewer probes plan exactly, also over a gauge declared non-negative; the sign rule for phi = -1 and 2. --- .../src/per_computation_candidates.rs | 57 ++++++++++++++++++- crates/plan-selection/src/lib.rs | 42 ++++++++++++++ .../asap-aware-mapping-contracts.md | 3 +- 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/crates/logical-asap-optimization/src/per_computation_candidates.rs b/crates/logical-asap-optimization/src/per_computation_candidates.rs index 62956e608..e7241ea32 100644 --- a/crates/logical-asap-optimization/src/per_computation_candidates.rs +++ b/crates/logical-asap-optimization/src/per_computation_candidates.rs @@ -174,6 +174,10 @@ pub enum PruneReason { /// answer the measure's readout from it /// ([`DeploymentCapabilities::supports`]). UnsupportedByDeployment, + /// The quantile's φ is outside [0, 1], where PromQL answers -Inf or + /// +Inf: no sketch estimates that, and the executor's sketches refuse + /// such a φ, so only the exact answer remains. + QuantileOutsideUnitInterval, } /// Unranked candidates of one measure and the candidates pruned for it. @@ -481,7 +485,14 @@ pub fn computation_candidates_for_intent( } } let summaries = summary_candidates(intent).iter().cloned(); - if target.is_exact() { + if matches!(intent, AggIntent::Quantile { q, .. } if !(0.0..=1.0).contains(q)) { + choices.pruned.extend(summaries.map(|algorithm| { + ( + ComputationCandidate::Summary(algorithm), + PruneReason::QuantileOutsideUnitInterval, + ) + })); + } else if target.is_exact() { choices.pruned.extend(summaries.map(|algorithm| { ( ComputationCandidate::Summary(algorithm), @@ -1677,10 +1688,12 @@ fn estimates_samples_of(node: &OperatorNode) -> Option { } => estimates_samples_of(child), _ => None, }, + // Only a φ in [0, 1] picks one of the values; outside it the answer + // is -Inf or +Inf, whatever their sign. Operator::ASAP(ASAPOp::SummaryEstimate { summary_input, - query: SketchStatistic::Quantile { .. }, - }) => match summary_input.asap()? { + query: SketchStatistic::Quantile { q }, + }) if (0.0..=1.0).contains(q) => match summary_input.asap()? { ASAPOp::SummaryAgg { input: SummaryUpdate { @@ -4374,6 +4387,44 @@ mod tests { assert_eq!(inventory.combinations(), enumerated); } + /// A quantile estimate of a metric's samples carries their sign only for + /// φ in [0, 1]: outside it PromQL answers -Inf or +Inf. Pass 1 offers + /// no sketch for such a φ. + #[test] + fn a_quantile_estimate_shares_its_samples_sign_only_inside_the_unit_interval() { + let estimate_of = |query: &str| { + let inventory = inventory(query); + (0..inventory.targets.len()) + .flat_map(|t| composed_with(&inventory, t)) + .find_map(|(_, root)| { + OperatorNode::reachable(&root) + .into_iter() + .find(|n| matches!(n.asap(), Some(ASAPOp::SummaryEstimate { .. }))) + }) + }; + let inside = estimate_of("quantile_over_time(0.5, m[1m])").expect("a sketch estimate"); + assert_eq!(estimates_samples_of(&inside).as_deref(), Some("m")); + let Some(ASAPOp::SummaryEstimate { summary_input, .. }) = inside.asap() else { + unreachable!() + }; + for q in [-1.0, 2.0] { + let outside = OperatorNode::with_schema( + Operator::ASAP(ASAPOp::SummaryEstimate { + summary_input: Rc::clone(summary_input), + query: SketchStatistic::Quantile { q }, + }), + inside.schema.clone(), + ); + assert_eq!(estimates_samples_of(&outside), None, "q = {q}"); + } + for query in [ + "quantile_over_time(-1, m[1m])", + "quantile_over_time(2, m[1m])", + ] { + assert!(estimate_of(query).is_none(), "{query}: a sketch is offered"); + } + } + /// The metric whose raw samples, or plain sums of them, a composed node's /// values estimate: a whole-expression heap's estimates and an exact /// sum over the samples are; a rate or any other function is not. A diff --git a/crates/plan-selection/src/lib.rs b/crates/plan-selection/src/lib.rs index f91fd4d3e..448938a43 100644 --- a/crates/plan-selection/src/lib.rs +++ b/crates/plan-selection/src/lib.rs @@ -5301,6 +5301,48 @@ mod tests { } } + /// PromQL answers a quantile with φ outside [0, 1] as -Inf or +Inf, + /// which no sketch estimates and the executor's sketches refuse: such a + /// quantile gets no sketch candidate, so the exact plan is selected, and + /// a sketch never inherits its samples' sign for it, even over a gauge + /// declared non-negative. + #[test] + fn a_quantile_outside_the_unit_interval_is_answered_exactly() { + let target = AccuracyTarget::max_error(0.01) + .failure_probability(0.01) + .accuracy_metric(AccuracyMetric::Relative); + for query in [ + "avg by (region) (quantile_over_time(-1, x[100s]))", + "avg by (region) (quantile_over_time(2, x[100s]))", + "quantile_over_time(2, x[100s])", + "max by (region) (quantile_over_time(-1, x[100s]))", + ] { + let candidates = all_candidates(query, &target); + assert!( + candidates.iter().all(|c| !has_summary_estimate(c)), + "{query}: a sketch candidate is offered" + ); + let mut data = with_metric_types(data(), &[("x", MetricType::Gauge)]); + data.sources.get_mut("x").unwrap().values_non_negative = true; + let selection = select_plan( + &candidates, + &[entry_with(target.clone(), repeating(10_000))], + &data, + PlanningModels::builtin(), + ) + .unwrap(); + let selected = candidates + .iter() + .find(|c| c.id == selection.selected) + .unwrap(); + assert!( + !has_summary_estimate(selected), + "{query}: {}", + selection.selected + ); + } + } + /// `topk(k, irate(m[w]))`, now a heavy-hitter ranking as `rate`'s is, /// still plans: Stages 1-3 compose its candidates and select one. #[test] diff --git a/docs/develop_docs/asap-aware-mapping-contracts.md b/docs/develop_docs/asap-aware-mapping-contracts.md index c7e6ce164..81051a8ae 100644 --- a/docs/develop_docs/asap-aware-mapping-contracts.md +++ b/docs/develop_docs/asap-aware-mapping-contracts.md @@ -41,7 +41,8 @@ asap-physical-asap-optimization -p asap-plan-selection -p asap-types compares cost. - **Pruning.** A candidate that cannot meet the target is moved to the target's pruned list with a `PruneReason` (`RequiresExact`, - `NullableAvgInput`, `UnsupportedByDeployment`); it is never silently + `NullableAvgInput`, `UnsupportedByDeployment`, + `QuantileOutsideUnitInterval`); it is never silently dropped. Pass 1 prunes only what it can prove; accuracy that depends on data facts is left to Stage 3. - **Fused computations.** `FusedComputation { absorbs, candidates, pruned }`