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
3 changes: 2 additions & 1 deletion asap-query-engine/src/drivers/query/servers/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4691,7 +4691,8 @@ struct PrecomputeJobRequest {
/// POST /api/v1/precompute
///
/// The controller creates PrecomputeJobs when a query's upper sub-tree
/// (e.g., TopK, HistogramQuantile) requires evaluation on merged sketches.
/// (e.g., TopK or a histogram-quantile-shaped Aggregate{Quantile(φ)})
/// requires evaluation on merged sketches.
/// This endpoint receives that job and runs it against the SimpleMapStore.
async fn handle_precompute_job(
State(state): State<AppState>,
Expand Down
6 changes: 4 additions & 2 deletions controller/src/deployment_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,10 @@ mod tests {
let id = DeploymentModelId::asaplifecycle();
assert!(reg.contains(&id));
let m = reg.lookup(&id).expect("asaplifecycle must be registered");
// The default rule set carries the 12 engine rules.
assert_eq!(m.rules.len(), 12, "asaplifecycle should ship the 12 engine rules");
// The default rule set carries the 11 engine rules. (R6
// HistogramQuantileFusion was retired in Step γ5 — see
// `optimizer::engine` module note.)
assert_eq!(m.rules.len(), 11, "asaplifecycle should ship the 11 engine rules");
// Emitter set carries the three demo emitters.
assert!(m.emitters.has("opamp_edge_yaml"));
assert!(m.emitters.has("streaming_config_json"));
Expand Down
15 changes: 8 additions & 7 deletions controller/src/intent_algebra/agg_intent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ pub enum AggIntent {
// captures the intent so the routing decision is layered above intent.
//
// 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
// /MetricsQL language-level operator. Per Step γ5 of the legacy_expr
// migration, the PromQL parser substitutes it directly into a plain
// `Aggregate { Quantile(φ) }` (which lowers 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
Expand Down Expand Up @@ -176,9 +176,10 @@ impl AggIntent {
///
/// 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, .. }`.
/// it's a PromQL/MetricsQL language-level operator. Per Step γ5,
/// the PromQL parser substitutes it into a plain
/// `Aggregate { Quantile(φ) }`, which lowers to
/// `AggIntent::Quantile { q, .. }`.
pub fn archive_only(&self) -> bool {
matches!(
self,
Expand Down
2 changes: 1 addition & 1 deletion controller/src/intent_algebra/aggregate_bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
//!
//! Many `Aggregate.input` subtrees still hold legacy types (other not-
//! yet-migrated variants — `SketchAgg`, `WindowedAgg`, `TopK`,
//! `HistogramQuantile`, `PromQLSubquery`). The canonical
//! `PromQLSubquery`). The canonical
//! `QueryExpr::Aggregate.child: Box<QueryExpr>` field expects a CANONICAL
//! `QueryExpr` — so until Steps γ2-γ7 migrate those variants, we cannot
//! build a fully-canonical `Aggregate` rooted at a legacy parser's emit
Expand Down
27 changes: 5 additions & 22 deletions controller/src/intent_algebra/legacy_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,12 +492,11 @@ pub enum QueryExpr {

// ── PromQL-specific operators ─────────────────────────────────────────

/// `histogram_quantile(φ, <buckets>)` — converts an HLL / histogram
/// sketch into a quantile estimate.
HistogramQuantile {
phi: f64,
input: Box<QueryExpr>,
},
// Note: `histogram_quantile(φ, <buckets>)` is no longer a `QueryExpr`
// variant. Per Step γ5 of the legacy_expr migration, the PromQL parser
// substitutes the call with a plain `Aggregate { Quantile(φ) }` so
// downstream code (lowerer, optimizer, physical planner) sees a single
// canonical Quantile intent.

/// PromQL sub-query syntax: `<expr>[range:resolution]`.
PromQLSubquery {
Expand Down Expand Up @@ -784,7 +783,6 @@ impl QueryExpr {
| QueryExpr::TopK { input, .. }
| QueryExpr::Sort { input, .. }
| QueryExpr::Limit { input, .. }
| QueryExpr::HistogramQuantile { input, .. }
| QueryExpr::PromQLSubquery { input, .. } => input.walk(f),

QueryExpr::Aggregate { input, .. } => input.walk(f),
Expand Down Expand Up @@ -832,7 +830,6 @@ impl QueryExpr {
| QueryExpr::Sort { input, .. }
| QueryExpr::Limit { input, .. }
| QueryExpr::Aggregate { input, .. }
| QueryExpr::HistogramQuantile { input, .. }
| QueryExpr::PromQLSubquery { input, .. } => input.source_name(),
QueryExpr::Merge { inputs } => inputs.first()?.source_name(),
QueryExpr::Join { left, .. }
Expand Down Expand Up @@ -1094,20 +1091,6 @@ mod tests {
}
}

#[test]
fn histogram_quantile_node() {
let expr = QueryExpr::HistogramQuantile {
phi: 0.95,
input: Box::new(QueryExpr::Source(SourceSpec { name: "hist".into() })),
};
match expr {
QueryExpr::HistogramQuantile { phi, .. } => {
assert!((phi - 0.95).abs() < 1e-9);
}
_ => panic!(),
}
}

#[test]
fn promql_subquery_node() {
let expr = QueryExpr::PromQLSubquery {
Expand Down
20 changes: 0 additions & 20 deletions controller/src/intent_algebra/legacy_lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,6 @@ pub fn lower_to_sketch_algebra_with_schema(
offset,
input: Box::new(lower_to_sketch_algebra_with_schema(*input, parent_schema)),
},
QueryExpr::HistogramQuantile { phi, input } => QueryExpr::HistogramQuantile {
phi,
input: Box::new(lower_to_sketch_algebra_with_schema(*input, parent_schema)),
},
QueryExpr::PromQLSubquery { range, resolution, input } => QueryExpr::PromQLSubquery {
range,
resolution,
Expand Down Expand Up @@ -544,22 +540,6 @@ mod tests {
}
}

#[test]
fn histogram_quantile_passthrough() {
let expr = QueryExpr::HistogramQuantile {
phi: 0.95,
input: Box::new(make_agg(AggFunc::Quantile(0.95), src("m"))),
};
let lowered = lower_to_sketch_algebra(expr);
match &lowered {
QueryExpr::HistogramQuantile { phi, input } => {
assert!((phi - 0.95).abs() < 1e-9);
assert!(matches!(input.as_ref(), QueryExpr::SketchAgg { .. }));
}
other => panic!("expected HistogramQuantile, got {other:?}"),
}
}

#[test]
fn rate_lowered_to_sum() {
let expr = make_agg(AggFunc::Rate, src("m"));
Expand Down
5 changes: 3 additions & 2 deletions controller/src/language_logical_plan/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
//!
//! The `intent_algebra` (L3) layer (Phase B, separate worktree) is
//! responsible for normalising this language-specific tree into the
//! language-orthogonal `QueryExpr` shape (e.g. dropping
//! `HistogramQuantile`, `PromQLSubquery`).
//! language-orthogonal `QueryExpr` shape (e.g. dropping `PromQLSubquery`).
//! `histogram_quantile(...)` is no longer a legacy variant — Step γ5
//! substitutes it at the parser level into a plain `Aggregate{Quantile(φ)}`.

use std::collections::HashMap;
use std::time::Duration;
Expand Down
142 changes: 20 additions & 122 deletions controller/src/optimizer/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
//! | R3 | `HLLDedupElim` | Eliminate `Dedup` before HLL (HLL is inherently distinct) |
//! | R4 | `FilterWindowSwap` | Swap `Filter` below `Window` to reduce window input size |
//! | R5 | `TopKFusion` | Absorb `Limit` / `TopK` into a `CountSketch` agg |
//! | R6 | `HistogramQuantileFusion` | Recognise `HistogramQuantile(φ, Agg(DDSketch))` and mark |
//! | R6 | _(retired)_ | `HistogramQuantileFusion` retired in Step γ5 — `histogram_quantile` is now a parser-level substitution |
//! | R7 | `SubqueryDecorrelation` | Hoist correlated `ScalarSubquery` to a `LetBinding` |
//! | R8 | `CommonSubexprElim` | Extract identical sub-trees into `LetBinding`s |
//! | R9 | `HydraConversion` | Convert multi-key `Partition + Agg` into `Hydra` sketch |
Expand Down Expand Up @@ -229,7 +229,7 @@ impl CostModel for DefaultCostModel {
| QueryExpr::Window { .. } => &dc.agent,
QueryExpr::Partition { .. } | QueryExpr::Merge { .. }
| QueryExpr::Distinct { .. } => &dc.backend_collector,
QueryExpr::TopK { .. } | QueryExpr::HistogramQuantile { .. }
QueryExpr::TopK { .. }
| QueryExpr::BinaryOp { .. } | QueryExpr::PromQLSubquery { .. } => &dc.backend_db,
QueryExpr::Aggregate { .. } => &dc.original_db,
_ => &dc.agent,
Expand Down Expand Up @@ -492,76 +492,15 @@ impl RewriteRule for TopKFusion {
}
}

// ── R6: HistogramQuantileFusion ───────────────────────────────────────────────

/// Recognise `HistogramQuantile(φ, SketchAgg(DDSketch([φ]), …))` and
/// simplify to a single annotated node that the allocator handles as one
/// DDSketch query.
///
/// `HistogramQuantile(φ, SketchAgg(DDSketch(qs), col, e))`
/// where `qs` contains `φ`
/// → `HistogramQuantile(φ, SketchAgg(DDSketch(qs), col, e))` [marked fused]
///
/// In practice we just ensure the quantile is in the DDSketch's quantile
/// list so the allocator emits a single sketch with the right φ.
pub struct HistogramQuantileFusion;

impl RewriteRule for HistogramQuantileFusion {
fn name(&self) -> &'static str { "HistogramQuantileFusion" }

fn try_rewrite(&self, expr: QueryExpr, _model: &dyn CostModel, _parent_schema: &Schema) -> Option<QueryExpr> {
// Canonical Quantile is single-φ post Step α — multi-φ fan-out
// happens at construction time, so the "merge phi into the
// existing quantile list" branch is now a "if phis match, keep
// structure; else build a Merge of two SketchAgg siblings". For
// this PR we keep the structural marker (identity rewrite) and
// defer the Merge-aware fusion to Step γ; the original rule was
// primarily a structural marker anyway.
match expr {
QueryExpr::HistogramQuantile { phi, input } => {
match *input {
QueryExpr::SketchAgg {
op: AggIntent::Quantile { q, accuracy },
col,
input: inner,
} => {
if (q - phi).abs() < f64::EPSILON {
// Quantile already matches φ — keep shape.
Some(QueryExpr::HistogramQuantile {
phi,
input: Box::new(QueryExpr::SketchAgg {
op: AggIntent::Quantile { q, accuracy },
col,
input: inner,
}),
})
} else {
// φ ≠ q: build a Merge of two single-φ
// SketchAgg siblings (F1 fan-out for the
// multi-φ case) and re-wrap.
let new_q = QueryExpr::SketchAgg {
op: AggIntent::Quantile { q: phi, accuracy: accuracy.clone() },
col: col.clone(),
input: inner.clone(),
};
let old_q = QueryExpr::SketchAgg {
op: AggIntent::Quantile { q, accuracy },
col,
input: inner,
};
Some(QueryExpr::HistogramQuantile {
phi,
input: Box::new(QueryExpr::Merge { inputs: vec![new_q, old_q] }),
})
}
}
other => Some(QueryExpr::HistogramQuantile { phi, input: Box::new(other) }),
}
}
_ => None,
}
}
}
// ── R6 (retired): HistogramQuantileFusion ─────────────────────────────────────
//
// Per Step γ5 of the legacy_expr migration, `histogram_quantile(φ, …)` is
// substituted at the PromQL parser level into a plain
// `Aggregate{Quantile(φ)}`. The fusion rule that used to merge a
// `HistogramQuantile` wrapper with an inner DDSketch is no longer needed —
// the parser emits a single `Aggregate{Quantile(φ)}` directly, and the
// L1→L3 lowerer turns it into a single `SketchAgg{Quantile(φ, ...)}` with
// no wrapper to fuse.

// ── R7: SubqueryDecorrelation ─────────────────────────────────────────────────

Expand Down Expand Up @@ -1003,10 +942,6 @@ impl QueryOptimizer {
let (new_input, c) = recurse!(input);
(QueryExpr::Limit { n, offset, input: new_input }, c)
}
QueryExpr::HistogramQuantile { phi, input } => {
let (new_input, c) = recurse!(input);
(QueryExpr::HistogramQuantile { phi, input: new_input }, c)
}
QueryExpr::PromQLSubquery { range, resolution, input } => {
let (new_input, c) = recurse!(input);
(QueryExpr::PromQLSubquery { range, resolution, input: new_input }, c)
Expand Down Expand Up @@ -1051,7 +986,9 @@ fn default_rules() -> Vec<Box<dyn RewriteRule>> {
Box::new(WindowMerge),
Box::new(PartitionElim),
Box::new(TopKFusion),
Box::new(HistogramQuantileFusion),
// R6 (HistogramQuantileFusion) retired in Step γ5 — see the
// module-level note above; histogram_quantile is now a parser-
// level substitution into plain `Aggregate{Quantile(φ)}`.
Box::new(MergeLifting),
Box::new(SetOpFusion),
Box::new(HydraConversion),
Expand All @@ -1075,7 +1012,7 @@ pub fn default_rules_as_optimizer_rules()
Box::new(WindowMerge),
Box::new(PartitionElim),
Box::new(TopKFusion),
Box::new(HistogramQuantileFusion),
// R6 (HistogramQuantileFusion) retired in Step γ5.
Box::new(MergeLifting),
Box::new(SetOpFusion),
Box::new(HydraConversion),
Expand Down Expand Up @@ -1121,10 +1058,6 @@ impl OptimizerRule for TopKFusion {
fn name(&self) -> &'static str { <Self as RewriteRule>::name(self) }
fn category(&self) -> RuleCategory { RuleCategory::Fusion }
}
impl OptimizerRule for HistogramQuantileFusion {
fn name(&self) -> &'static str { <Self as RewriteRule>::name(self) }
fn category(&self) -> RuleCategory { RuleCategory::Fusion }
}
impl OptimizerRule for MergeLifting {
fn name(&self) -> &'static str { <Self as RewriteRule>::name(self) }
fn category(&self) -> RuleCategory { RuleCategory::PushDown }
Expand Down Expand Up @@ -1256,46 +1189,11 @@ mod tests {
);
}

// ── R6: HistogramQuantileFusion ───────────────────────────────────────────

#[test]
fn r6_fans_out_to_merge_when_phi_differs() {
use crate::types_v2::AccuracyTarget;
// Step α: canonical Quantile is single-φ; R6 emits a Merge of
// two single-φ SketchAgg siblings when φ doesn't match the
// existing intent's q. Apply R6 directly so this test is
// independent of downstream rules (CommonSubexprElim, etc.).
let expr = QueryExpr::HistogramQuantile {
phi: 0.95,
input: Box::new(QueryExpr::SketchAgg {
op: AggIntent::Quantile { q: 0.5, accuracy: AccuracyTarget::Epsilon(0.01) },
col: ColumnRef::SampleValue,
input: Box::new(src("latency")),
}),
};
let rule = HistogramQuantileFusion;
let model = DefaultCostModel { raw_bytes_per_sec: 100_000.0, deployment: None };
let schema = infer_schema_for_root(&expr);
let result = rule.try_rewrite(expr, &model, &schema).expect("R6 should fire");
match &result {
QueryExpr::HistogramQuantile { input, .. } => {
match input.as_ref() {
QueryExpr::Merge { inputs } => {
let mut qs: Vec<f64> = inputs.iter().filter_map(|i| match i {
QueryExpr::SketchAgg { op: AggIntent::Quantile { q, .. }, .. } => {
Some(*q)
}
_ => None,
}).collect();
qs.sort_by(|a, b| a.partial_cmp(b).unwrap());
assert_eq!(qs, vec![0.5, 0.95]);
}
other => panic!("expected Merge of SketchAgg siblings, got {other:?}"),
}
}
other => panic!("unexpected {other:?}"),
}
}
// ── R6 (retired) ──────────────────────────────────────────────────────────
// HistogramQuantileFusion was removed in Step γ5 — `histogram_quantile`
// is now substituted at the parser level into a plain
// `Aggregate{Quantile(φ)}`, so the wrapper-fusion rule has no input
// shape to match. See the module-level note for details.

// ── R10: WindowMerge ──────────────────────────────────────────────────────

Expand Down
4 changes: 2 additions & 2 deletions controller/src/optimizer/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub trait OptimizerRule: Send + Sync {
mod tests {
use super::*;
use crate::optimizer::engine::{
CommonSubexprElim, FilterWindowSwap, HLLDedupElim, HistogramQuantileFusion,
CommonSubexprElim, FilterWindowSwap, HLLDedupElim,
HydraConversion, MergeLifting, PartitionElim, PredicatePushDown, SetOpFusion,
SubqueryDecorrelation, TopKFusion, WindowMerge,
};
Expand All @@ -89,7 +89,7 @@ mod tests {
Box::new(WindowMerge),
Box::new(PartitionElim),
Box::new(TopKFusion),
Box::new(HistogramQuantileFusion),
// R6 (HistogramQuantileFusion) retired in Step γ5.
Box::new(MergeLifting),
Box::new(SetOpFusion),
Box::new(HydraConversion),
Expand Down
Loading