From 37455d945b105bf24177b7aff94071f03d658cbf Mon Sep 17 00:00:00 2001 From: zz_y Date: Tue, 28 Jul 2026 15:55:52 -0600 Subject: [PATCH] refactor(sketch,plan): rename sketch/sketch_input to summary/summary_input; merge Implementation::Sketch/ExactAccumulator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both were flagged as follow-ups during PR #169's review of the L4 doc's Interface section (not yet merged): - `SummaryExpr`'s `sketch`/`sketch_input` fields predated this codebase's "summary" umbrella term (an approximate sketch is one kind of summary, alongside an exact accumulator) — renamed to `summary`/`summary_input` to match. - `Implementation::Sketch{kind,params}` and `::ExactAccumulator{kind,params}` carried identical shapes; the split existed only so `bind_summary_agg` could tell whether to wrap a `SummaryEstimate` readout. Collapsed into one `Implementation::Summary{kind,params}`, with a new `SummaryKind::is_exact()` recovering the same fact from `kind` alone. Co-Authored-By: Claude Sonnet 5 --- crates/e2e/tests/l4_binding.rs | 20 +++---- crates/plan/src/bind.rs | 101 +++++++++++++++++---------------- crates/plan/src/boundary.rs | 46 ++++++++------- crates/sketch/src/exec.rs | 32 +++++------ crates/sketch/src/expr.rs | 16 +++--- crates/sketch/src/sketch.rs | 52 +++++++++++++++++ 6 files changed, 164 insertions(+), 103 deletions(-) diff --git a/crates/e2e/tests/l4_binding.rs b/crates/e2e/tests/l4_binding.rs index 33b4515f..255b8e08 100644 --- a/crates/e2e/tests/l4_binding.rs +++ b/crates/e2e/tests/l4_binding.rs @@ -46,7 +46,7 @@ fn promql_quantile_of_rate_binds_kll_over_rate_accumulator() { // Root: the sketch readout, back to a plain row shape. let SummaryExpr::SummaryEstimate { - sketch_input, + summary_input, query, } = &root.expr else { @@ -66,15 +66,15 @@ fn promql_quantile_of_rate_binds_kll_over_rate_accumulator() { // same empty `by: []` (issue #163). let SummaryExpr::SummaryAgg { child, - sketch, + summary, params, col, reduction, - } = &sketch_input.expr + } = &summary_input.expr else { - panic!("expected SummaryAgg, got {:?}", sketch_input.expr); + panic!("expected SummaryAgg, got {:?}", summary_input.expr); }; - assert_eq!(sketch, &SummaryKind::Kll); + assert_eq!(summary, &SummaryKind::Kll); assert_eq!(params, &SummaryParams::Kll { k: 200 }); assert_eq!(col, &ColumnRef::SampleValue); assert_eq!( @@ -83,7 +83,7 @@ fn promql_quantile_of_rate_binds_kll_over_rate_accumulator() { "global quantile — no group keys, full reduction" ); assert_eq!( - dtype(&sketch_input.schema, "quantile_0_99"), + dtype(&summary_input.schema, "quantile_0_99"), &L4DataType::Sketch(SummaryKind::Kll, SummaryParams::Kll { k: 200 }) ); @@ -92,7 +92,7 @@ fn promql_quantile_of_rate_binds_kll_over_rate_accumulator() { // grouping concept at all — every entity stays its own summary. let SummaryExpr::SummaryAgg { child: leaf, - sketch, + summary, params, reduction, .. @@ -100,7 +100,7 @@ fn promql_quantile_of_rate_binds_kll_over_rate_accumulator() { else { panic!("expected inner SummaryAgg for rate, got {:?}", child.expr); }; - assert_eq!(sketch, &SummaryKind::Rate); + assert_eq!(summary, &SummaryKind::Rate); assert_eq!(params, &SummaryParams::Rate); assert_eq!(reduction, &Reduction::PerEntity); assert_eq!( @@ -140,7 +140,7 @@ fn promql_exact_workload_binds_accumulators_not_sketches() { .expect("lowering failed"); let root = implement_tree(&l3).expect("binding failed"); let SummaryExpr::SummaryAgg { - sketch, + summary, params, reduction, .. @@ -148,7 +148,7 @@ fn promql_exact_workload_binds_accumulators_not_sketches() { else { panic!("expected SummaryAgg, got {:?}", root.expr); }; - assert_eq!(sketch, &SummaryKind::Sum); + assert_eq!(summary, &SummaryKind::Sum); assert_eq!(params, &SummaryParams::Sum); assert_eq!( reduction, diff --git a/crates/plan/src/bind.rs b/crates/plan/src/bind.rs index 6e2cb0a1..78e167d7 100644 --- a/crates/plan/src/bind.rs +++ b/crates/plan/src/bind.rs @@ -95,15 +95,11 @@ pub fn implement_tree_in_with( // nodes and HAVING stay logical — see the module docs.) if let ([intent], None) = (aggs.as_slice(), having) { match implementation_for_with(intent, cost_model) { - Implementation::Sketch { kind, params } => { + Implementation::Summary { kind, params } => { + let estimate = !kind.is_exact(); return bind_summary_agg( - expr, reduction, intent, child, kind, params, scope, true, cost_model, - ) - } - Implementation::ExactAccumulator { kind, params } => { - return bind_summary_agg( - expr, reduction, intent, child, kind, params, scope, false, cost_model, - ) + expr, reduction, intent, child, kind, params, scope, estimate, cost_model, + ); } Implementation::PassThrough => {} } @@ -154,7 +150,7 @@ fn bind_summary_agg( let agg = Rc::new(L4Node { expr: SummaryExpr::SummaryAgg { child: implement_tree_in_with(child, scope, cost_model)?, - sketch: kind, + summary: kind, params, col, reduction: reduction.clone(), @@ -166,7 +162,7 @@ fn bind_summary_agg( // row shape again (the `Sketch(…)` type does not propagate). Some(query) => Ok(Rc::new(L4Node { expr: SummaryExpr::SummaryEstimate { - sketch_input: agg, + summary_input: agg, query, }, schema: lift(&out_schema), @@ -324,7 +320,7 @@ mod tests { let root = implement_tree(&q).unwrap(); let SummaryExpr::SummaryEstimate { - sketch_input, + summary_input, query, } = &root.expr else { @@ -343,21 +339,21 @@ mod tests { let SummaryExpr::SummaryAgg { child, - sketch, + summary, params, col, reduction, - } = &sketch_input.expr + } = &summary_input.expr else { - panic!("expected SummaryAgg, got {:?}", sketch_input.expr); + panic!("expected SummaryAgg, got {:?}", summary_input.expr); }; - assert_eq!(sketch, &SummaryKind::Kll); + assert_eq!(summary, &SummaryKind::Kll); assert_eq!(params, &SummaryParams::Kll { k: 200 }); assert_eq!(col, &ColumnRef::SampleValue); assert_eq!(reduction, &Reduction::by(vec![2])); // SummaryAgg edge: the state column carries the committed (kind, params). assert_eq!( - field(&sketch_input.schema, "quantile_0_99").dtype, + field(&summary_input.schema, "quantile_0_99").dtype, L4DataType::Sketch(SummaryKind::Kll, SummaryParams::Kll { k: 200 }) ); assert!(matches!(child.expr, SummaryExpr::Logical(ref e) @@ -390,23 +386,26 @@ mod tests { // Default: KLL (see `quantile_binds_kll_wrapped_in_estimate` above). let default_root = implement_tree(&q).unwrap(); - let SummaryExpr::SummaryEstimate { sketch_input, .. } = &default_root.expr else { + let SummaryExpr::SummaryEstimate { summary_input, .. } = &default_root.expr else { panic!("expected SummaryEstimate root, got {:?}", default_root.expr); }; - let SummaryExpr::SummaryAgg { sketch, .. } = &sketch_input.expr else { - panic!("expected SummaryAgg, got {:?}", sketch_input.expr); + let SummaryExpr::SummaryAgg { summary, .. } = &summary_input.expr else { + panic!("expected SummaryAgg, got {:?}", summary_input.expr); }; - assert_eq!(sketch, &SummaryKind::Kll); + assert_eq!(summary, &SummaryKind::Kll); // With `PreferDDSketch`: DDSketch instead, same query. let custom_root = implement_tree_with(&q, &PreferDDSketch).unwrap(); - let SummaryExpr::SummaryEstimate { sketch_input, .. } = &custom_root.expr else { + let SummaryExpr::SummaryEstimate { summary_input, .. } = &custom_root.expr else { panic!("expected SummaryEstimate root, got {:?}", custom_root.expr); }; - let SummaryExpr::SummaryAgg { sketch, params, .. } = &sketch_input.expr else { - panic!("expected SummaryAgg, got {:?}", sketch_input.expr); + let SummaryExpr::SummaryAgg { + summary, params, .. + } = &summary_input.expr + else { + panic!("expected SummaryAgg, got {:?}", summary_input.expr); }; - assert_eq!(sketch, &SummaryKind::DDSketch); + assert_eq!(summary, &SummaryKind::DDSketch); assert_eq!(params, &SummaryParams::DDSketch { alpha: 0.01 }); } @@ -432,7 +431,7 @@ mod tests { _payload: &serde_json::Value, ) -> crate::boundary::Implementation { if ext_kind == "frequency" { - crate::boundary::Implementation::Sketch { + crate::boundary::Implementation::Summary { kind: SummaryKind::CountSketch, params: SummaryParams::CountSketch { width: 256, @@ -483,7 +482,7 @@ mod tests { let root = implement_tree_with(&q, &FrequencyCostModel).unwrap(); let SummaryExpr::SummaryEstimate { - sketch_input, + summary_input, query, } = &root.expr else { @@ -495,10 +494,13 @@ mod tests { if k == "item" && v == "checkout" )); - let SummaryExpr::SummaryAgg { sketch, params, .. } = &sketch_input.expr else { - panic!("expected SummaryAgg, got {:?}", sketch_input.expr); + let SummaryExpr::SummaryAgg { + summary, params, .. + } = &summary_input.expr + else { + panic!("expected SummaryAgg, got {:?}", summary_input.expr); }; - assert_eq!(sketch, &SummaryKind::CountSketch); + assert_eq!(summary, &SummaryKind::CountSketch); assert_eq!( params, &SummaryParams::CountSketch { @@ -512,13 +514,16 @@ mod tests { fn exact_sum_binds_accumulator_without_estimate() { let q = agg(vec![2], AggIntent::Sum { col: None }, metric_scan(&["job"])); let root = implement_tree(&q).unwrap(); - let SummaryExpr::SummaryAgg { sketch, params, .. } = &root.expr else { + let SummaryExpr::SummaryAgg { + summary, params, .. + } = &root.expr + else { panic!( "expected bare SummaryAgg (no estimate), got {:?}", root.expr ); }; - assert_eq!(sketch, &SummaryKind::Sum); + assert_eq!(summary, &SummaryKind::Sum); assert_eq!(params, &SummaryParams::Sum); assert_eq!( field(&root.schema, "sum").dtype, @@ -538,10 +543,10 @@ mod tests { }, ); let root = implement_tree(&q).unwrap(); - let SummaryExpr::SummaryAgg { sketch, .. } = &root.expr else { + let SummaryExpr::SummaryAgg { summary, .. } = &root.expr else { panic!("expected SummaryAgg, got {:?}", root.expr); }; - assert_eq!(sketch, &SummaryKind::Rate); + assert_eq!(summary, &SummaryKind::Rate); assert_eq!( root.schema .fields @@ -572,11 +577,11 @@ mod tests { }, ); let root = implement_tree(&q).unwrap(); - let SummaryExpr::SummaryEstimate { sketch_input, .. } = &root.expr else { + let SummaryExpr::SummaryEstimate { summary_input, .. } = &root.expr else { panic!("expected estimate root, got {:?}", root.expr); }; - let SummaryExpr::SummaryAgg { reduction, .. } = &sketch_input.expr else { - panic!("expected SummaryAgg, got {:?}", sketch_input.expr); + let SummaryExpr::SummaryAgg { reduction, .. } = &summary_input.expr else { + panic!("expected SummaryAgg, got {:?}", summary_input.expr); }; assert_eq!(reduction, &Reduction::PerEntity); } @@ -594,11 +599,11 @@ mod tests { }; let q = agg(vec![], intent, metric_scan(&["job"])); let root = implement_tree(&q).unwrap(); - let SummaryExpr::SummaryEstimate { sketch_input, .. } = &root.expr else { + let SummaryExpr::SummaryEstimate { summary_input, .. } = &root.expr else { panic!("expected estimate root, got {:?}", root.expr); }; - let SummaryExpr::SummaryAgg { reduction, .. } = &sketch_input.expr else { - panic!("expected SummaryAgg, got {:?}", sketch_input.expr); + let SummaryExpr::SummaryAgg { reduction, .. } = &summary_input.expr else { + panic!("expected SummaryAgg, got {:?}", summary_input.expr); }; assert_eq!(reduction, &Reduction::by(vec![])); } @@ -611,15 +616,15 @@ mod tests { let outer = agg(vec![], default_quantile(0.9), inner); let root = implement_tree(&outer).unwrap(); - let SummaryExpr::SummaryEstimate { sketch_input, .. } = &root.expr else { + let SummaryExpr::SummaryEstimate { summary_input, .. } = &root.expr else { panic!("expected estimate root, got {:?}", root.expr); }; - let SummaryExpr::SummaryAgg { child, sketch, .. } = &sketch_input.expr else { - panic!("expected outer SummaryAgg, got {:?}", sketch_input.expr); + let SummaryExpr::SummaryAgg { child, summary, .. } = &summary_input.expr else { + panic!("expected outer SummaryAgg, got {:?}", summary_input.expr); }; - assert_eq!(sketch, &SummaryKind::Kll); + assert_eq!(summary, &SummaryKind::Kll); let SummaryExpr::SummaryAgg { - sketch: inner_kind, + summary: inner_kind, child: leaf, .. } = &child.expr @@ -659,7 +664,7 @@ mod tests { fn find_summary_col(node: &L4Node) -> Option { match &node.expr { SummaryExpr::SummaryAgg { col, .. } => Some(col.clone()), - SummaryExpr::SummaryEstimate { sketch_input, .. } => find_summary_col(sketch_input), + SummaryExpr::SummaryEstimate { summary_input, .. } => find_summary_col(summary_input), _ => None, } } @@ -739,7 +744,7 @@ mod tests { ); let root = implement_tree(&q).unwrap(); let SummaryExpr::SummaryEstimate { - sketch_input, + summary_input, query, } = &root.expr else { @@ -747,9 +752,9 @@ mod tests { }; assert!(matches!(query, SketchQuery::TopK { k: 5 })); assert!(matches!( - &sketch_input.expr, + &summary_input.expr, SummaryExpr::SummaryAgg { - sketch: SummaryKind::CmsWithHeap, + summary: SummaryKind::CmsWithHeap, .. } )); diff --git a/crates/plan/src/boundary.rs b/crates/plan/src/boundary.rs index c5020c16..3db21f68 100644 --- a/crates/plan/src/boundary.rs +++ b/crates/plan/src/boundary.rs @@ -33,15 +33,15 @@ use crate::cost_model::{CostModel, DefaultCostModel}; /// How an [`AggIntent`] is realised at L4. #[derive(Debug, Clone, PartialEq)] pub enum Implementation { - /// An approximate sketch, sized to the intent's [`AccuracyTarget`]. - Sketch { - kind: SummaryKind, - params: SummaryParams, - }, - /// An exact **mergeable** accumulator (partial state ≡ the value itself: - /// `Sum` / `Count` / `MinMax` / `Rate` / `Increase`). Still a summary — - /// it pre-aggregates and merges across stages — just with zero error. - ExactAccumulator { + /// A summary — either an approximate sketch sized to the intent's + /// [`AccuracyTarget`], or an exact **mergeable** accumulator (partial + /// state ≡ the value itself: `Sum` / `Count` / `MinMax` / `Rate` / + /// `Increase`). `kind.is_exact()` tells the two apart; binding needs + /// that fact to decide whether a `SummaryEstimate` readout is needed + /// afterward (approximate) or the built state *is* the answer already + /// (exact — no estimate step). Either way this is still a summary — it + /// pre-aggregates and merges across stages. + Summary { kind: SummaryKind, params: SummaryParams, }, @@ -69,7 +69,7 @@ pub enum Implementation { /// crate can settle on its own. Two real, reasonable answers already /// diverge outside this crate: /// -/// - A **pure sketch-algebra** answer would say a `Sketch{kind: Kll, ..}` +/// - A **pure sketch-algebra** answer would say a `Summary{kind: Kll, ..}` /// requirement is satisfied by an available `DDSketch` (both quantile /// sketches), and that a heap-bearing top-k sketch also answers a bare /// frequency point-query (the heap is additional info on the same @@ -220,7 +220,11 @@ fn accumulator(intent: &AggIntent, kind: SummaryKind, params: SummaryParams) -> agg_is_mergeable(intent), "accumulator for non-mergeable {intent:?}" ); - Implementation::ExactAccumulator { kind, params } + debug_assert!( + kind.is_exact(), + "accumulator() called with a non-exact kind {kind:?}" + ); + Implementation::Summary { kind, params } } /// Bind the preferred candidate summary, with parameters sized to the @@ -245,7 +249,7 @@ fn bind_summary_with( .next() .expect("approximate intent has at least one candidate summary"); let params = cost_model.size_params(kind.clone(), intent, eps, delta); - Implementation::Sketch { kind, params } + Implementation::Summary { kind, params } } /// `asap-plan`'s built-in `SummaryParams` sizing, keyed off the resolved @@ -383,8 +387,8 @@ mod tests { fn cat(intent: &AggIntent) -> Cat { match implementation_for(intent) { - Implementation::Sketch { kind, .. } => Cat::Sketch(kind), - Implementation::ExactAccumulator { kind, .. } => Cat::Acc(kind), + Implementation::Summary { kind, .. } if kind.is_exact() => Cat::Acc(kind), + Implementation::Summary { kind, .. } => Cat::Sketch(kind), Implementation::PassThrough => Cat::Pass, } } @@ -544,7 +548,7 @@ mod tests { let approx = default_quantile(0.99); // ε = 0.01 assert_eq!( implementation_for(&approx), - Implementation::Sketch { + Implementation::Summary { kind: SummaryKind::Kll, params: SummaryParams::Kll { k: 200 }, // design.md worked example } @@ -557,7 +561,7 @@ mod tests { }; assert_eq!( implementation_for(&looser), - Implementation::Sketch { + Implementation::Summary { kind: SummaryKind::Kll, params: SummaryParams::Kll { k: 40 }, // ⌈2/0.05⌉ } @@ -570,7 +574,7 @@ mod tests { // sizing must invert it back exactly. assert_eq!( implementation_for(&default_cardinality()), - Implementation::Sketch { + Implementation::Summary { kind: SummaryKind::Hll, params: SummaryParams::Hll { precision: 14 }, } @@ -587,7 +591,7 @@ mod tests { }; assert_eq!( implementation_for(&intent), - Implementation::Sketch { + Implementation::Summary { kind: SummaryKind::Cms, params: SummaryParams::Cms { width: 2719, @@ -601,7 +605,7 @@ mod tests { }; assert_eq!( implementation_for(&intent), - Implementation::Sketch { + Implementation::Summary { kind: SummaryKind::Cms, params: SummaryParams::Cms { width: 2719, @@ -618,7 +622,7 @@ mod tests { accuracy: eps(0.01), }; match implementation_for(&intent) { - Implementation::Sketch { + Implementation::Summary { kind: SummaryKind::CmsWithHeap, params: SummaryParams::CmsWithHeap { @@ -670,7 +674,7 @@ mod tests { }; assert_eq!( implementation_for(&intent), - Implementation::Sketch { + Implementation::Summary { kind: SummaryKind::Kll, params: SummaryParams::Kll { k: 65_535 }, } diff --git a/crates/sketch/src/exec.rs b/crates/sketch/src/exec.rs index bdbf8aad..5dc7f20a 100644 --- a/crates/sketch/src/exec.rs +++ b/crates/sketch/src/exec.rs @@ -36,7 +36,7 @@ pub trait SummaryExecutor { /// Resolve a `SummaryAgg` leaf to matching materialized-instance /// handles, each tagged with the group it belongs to. Must only /// return handles whose `(SummaryKind, SummaryParams)` is exactly - /// `(sketch, params)`. + /// `(summary, params)`. /// /// `reduction` (the same [`Reduction`] the L3 `Aggregate` node this /// was bound from carried) tells you which of two shapes to produce — @@ -55,7 +55,7 @@ pub trait SummaryExecutor { #[allow(clippy::type_complexity)] fn find_candidates( &self, - sketch: &SummaryKind, + summary: &SummaryKind, params: &SummaryParams, col: &ColumnRef, reduction: &Reduction, @@ -130,12 +130,12 @@ pub fn execute( SummaryExpr::SummaryAgg { child, - sketch, + summary, params, col, reduction, } => { - let tagged = exec.find_candidates(sketch, params, col, reduction, child)?; + let tagged = exec.find_candidates(summary, params, col, reduction, child)?; if tagged.is_empty() { return Err(ExecError::NoCandidates); } @@ -155,16 +155,16 @@ pub fn execute( .map(|h| exec.fetch_state(h)) .collect::, _>>()?; let state = fold_states(states, exec)?; - out.push((key, state, sketch.clone(), params.clone())); + out.push((key, state, summary.clone(), params.clone())); } Ok(ExecOutcome::State(out)) } SummaryExpr::SummaryEstimate { - sketch_input, + summary_input, query, } => { - let groups = expect_state(execute(sketch_input, exec)?)?; + let groups = expect_state(execute(summary_input, exec)?)?; let mut out = Vec::with_capacity(groups.len()); for (key, state, _kind, _params) in groups { out.push((key, exec.readout(&state, query)?)); @@ -288,12 +288,12 @@ mod tests { /// ungrouped cross-series reduction) — the shape every pre-#163 test in /// this module exercises. Use [`agg_node_with_reduction`] to exercise /// `Reduction` itself. - fn agg_node(sketch: SummaryKind, params: SummaryParams, child: Rc) -> Rc { - agg_node_with_reduction(sketch, params, child, Reduction::by(vec![])) + fn agg_node(summary: SummaryKind, params: SummaryParams, child: Rc) -> Rc { + agg_node_with_reduction(summary, params, child, Reduction::by(vec![])) } fn agg_node_with_reduction( - sketch: SummaryKind, + summary: SummaryKind, params: SummaryParams, child: Rc, reduction: Reduction, @@ -301,7 +301,7 @@ mod tests { Rc::new(L4Node { expr: SummaryExpr::SummaryAgg { child, - sketch, + summary, params, col: ColumnRef::SampleValue, reduction, @@ -310,10 +310,10 @@ mod tests { }) } - fn estimate_node(sketch_input: Rc, query: SketchQuery) -> Rc { + fn estimate_node(summary_input: Rc, query: SketchQuery) -> Rc { Rc::new(L4Node { expr: SummaryExpr::SummaryEstimate { - sketch_input, + summary_input, query, }, schema: lift(vec!["value"]), @@ -333,7 +333,7 @@ mod tests { /// default), and `merge_states` is `sum`, so tests can assert on /// concrete numbers without any real sketch-math dependency. /// - /// `find_candidates` intentionally ignores `sketch`/`params`/ + /// `find_candidates` intentionally ignores `summary`/`params`/ /// `reduction`/`child` and returns every registered handle tagged with /// its own registered group — real deployments filter on those; these /// tests only exercise `execute`'s own tree-walking and grouping/merge @@ -381,7 +381,7 @@ mod tests { fn find_candidates( &self, - _sketch: &SummaryKind, + _summary: &SummaryKind, _params: &SummaryParams, _col: &ColumnRef, _reduction: &Reduction, @@ -659,7 +659,7 @@ mod tests { fn find_candidates( &self, - _sketch: &SummaryKind, + _summary: &SummaryKind, _params: &SummaryParams, _col: &ColumnRef, reduction: &Reduction, diff --git a/crates/sketch/src/expr.rs b/crates/sketch/src/expr.rs index 596f0618..e5885ac1 100644 --- a/crates/sketch/src/expr.rs +++ b/crates/sketch/src/expr.rs @@ -34,13 +34,13 @@ pub enum SummaryExpr { /// with all fields as `L4DataType::Primitive`. Logical(Box), - /// Sketch aggregation. L4 chose `sketch` + `params` from the catalog + /// Summary aggregation. L4 chose `summary` + `params` from the catalog /// for `AggIntent` under `DeploymentConstraints`. - /// Output schema: grouping columns (verbatim) + one `Sketch(sketch, - /// params)` field carrying partial sketch state per group. + /// Output schema: grouping columns (verbatim) + one `Sketch(summary, + /// params)` field carrying partial summary state per group. SummaryAgg { child: Rc, - sketch: SummaryKind, + summary: SummaryKind, params: SummaryParams, /// The column being summarised (fed into the sketch). col: ColumnRef, @@ -63,7 +63,7 @@ pub enum SummaryExpr { outer: Rc, inner: Rc, key: ColumnRef, - sketch: SummaryKind, + summary: SummaryKind, params: SummaryParams, }, @@ -77,16 +77,16 @@ pub enum SummaryExpr { /// filter). Catalog flag `deletable` must be true. Output schema = /// input schema unchanged in type (same `Sketch(s, p)` field). SummaryDelete { - sketch_input: Rc, + summary_input: Rc, key: ColumnRef, }, - /// Read out a query result from a built sketch. The `Sketch(…)` field + /// Read out a query result from a built summary. The `Sketch(…)` field /// type does *not* propagate downstream of an estimate — the output /// schema is a regular row-shaped schema (Float64 for quantile, Int64 /// for count/cardinality, `[(key, count)]` for top-k). SummaryEstimate { - sketch_input: Rc, + summary_input: Rc, query: SketchQuery, }, diff --git a/crates/sketch/src/sketch.rs b/crates/sketch/src/sketch.rs index 931e6642..4dff9ab1 100644 --- a/crates/sketch/src/sketch.rs +++ b/crates/sketch/src/sketch.rs @@ -48,6 +48,21 @@ pub enum SummaryKind { CountSketchWithHeap, } +impl SummaryKind { + /// Is this family an exact accumulator (zero approximation error) rather + /// than an approximate sketch? The partial state built for an exact kind + /// *is* the answer — no `SummaryEstimate` readout is needed to get a + /// value out of it. Used by `asap-plan::boundary::Implementation::Summary` + /// to recover, from `kind` alone, the same fact the now-collapsed + /// `Sketch`/`ExactAccumulator` variant tags used to carry directly. + pub fn is_exact(&self) -> bool { + matches!( + self, + Self::Sum | Self::Count | Self::MinMax | Self::Increase | Self::Rate + ) + } +} + // ── Sketch parameters ───────────────────────────────────────────────────────── /// Concrete, catalog-validated parameters for a specific summary instance. @@ -122,3 +137,40 @@ pub enum SketchQuery { /// Top-k most frequent (key, count) pairs. TopK { k: usize }, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn is_exact_matches_the_enum_declaration_split() { + for kind in [ + SummaryKind::Sum, + SummaryKind::Count, + SummaryKind::MinMax, + SummaryKind::Increase, + SummaryKind::Rate, + ] { + assert!( + kind.is_exact(), + "{kind:?} is declared as an exact accumulator" + ); + } + for kind in [ + SummaryKind::Kll, + SummaryKind::Cms, + SummaryKind::Hll, + SummaryKind::DDSketch, + SummaryKind::CmsWithHeap, + SummaryKind::Kmv, + SummaryKind::Theta, + SummaryKind::CountSketch, + SummaryKind::CountSketchWithHeap, + ] { + assert!( + !kind.is_exact(), + "{kind:?} is declared as an approximate sketch" + ); + } + } +}