From eec9474bc8b9c8dbfa9f26767740ed5e0b8ffe3c Mon Sep 17 00:00:00 2001 From: zz_y Date: Tue, 21 Jul 2026 16:14:13 -0600 Subject: [PATCH] feat(plan): extend CostModel with a size_params hook `CostModel::rank_candidates` already lets a deployment reorder which `SummaryKind` answers an intent, but `SummaryParams` sizing (k / width / depth / precision) was still hardcoded in `boundary::bind_summary_with`, unreachable by any cost model. A deployment with its own accuracy-bound math (or a downstream catalog that only recognizes discrete parameter rungs) had no extension point short of forking `implementation_for_with`. Add `CostModel::size_params`, defaulting to the now-extracted `boundary::default_size_params` (byte-for-byte the same formulas `bind_summary_with` used inline before) so `DefaultCostModel` and every existing caller keep today's behavior unchanged. A deployment can now override ranking, sizing, or both, independently. Co-Authored-By: Claude Sonnet 5 --- crates/plan/src/boundary.rs | 25 +++++++++-- crates/plan/src/cost_model.rs | 82 ++++++++++++++++++++++++++++++++++- 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/crates/plan/src/boundary.rs b/crates/plan/src/boundary.rs index 231ce01f..a66a6cd7 100644 --- a/crates/plan/src/boundary.rs +++ b/crates/plan/src/boundary.rs @@ -242,7 +242,27 @@ fn bind_summary_with( .into_iter() .next() .expect("approximate intent has at least one candidate summary"); - let params = match kind { + let params = cost_model.size_params(kind.clone(), intent, eps, delta); + Implementation::Sketch { kind, params } +} + +/// `asap-plan`'s built-in `SummaryParams` sizing, keyed off the resolved +/// `(eps, delta)` accuracy budget. [`CostModel::size_params`]'s default +/// body — factored out to a free function so a deployment's own +/// `CostModel` impl can still delegate to it for the candidates it +/// doesn't want to resize itself. +/// +/// Each formula inverts the sketch family's standard error bound to the +/// smallest parameter satisfying the target, clamped to the family's sane +/// range. A non-positive ε saturates to the clamp maximum (tightest +/// allowed). +pub fn default_size_params( + kind: SummaryKind, + intent: &AggIntent, + eps: f64, + delta: f64, +) -> SummaryParams { + match kind { SummaryKind::Kll => SummaryParams::Kll { k: kll_k(eps) }, SummaryKind::Cms => SummaryParams::Cms { width: cms_width(eps), @@ -294,8 +314,7 @@ fn bind_summary_with( | SummaryKind::MinMax | SummaryKind::Increase | SummaryKind::Rate => unreachable!("exact accumulators are not sketch candidates"), - }; - Implementation::Sketch { kind, params } + } } // ── Parameter sizing ────────────────────────────────────────────────────────── diff --git a/crates/plan/src/cost_model.rs b/crates/plan/src/cost_model.rs index d4ea6a8e..773e45c3 100644 --- a/crates/plan/src/cost_model.rs +++ b/crates/plan/src/cost_model.rs @@ -29,7 +29,7 @@ //! byte. use asap_ir::intent_algebra::agg_intent::AggIntent; -use asap_sketch::SummaryKind; +use asap_sketch::{SummaryKind, SummaryParams}; /// Ranks the candidate summary families for one [`AggIntent`], best choice /// first. @@ -54,10 +54,32 @@ pub trait CostModel { /// `implementation_for_with` treats that the same as `candidates` /// having been empty to begin with. fn rank_candidates(&self, intent: &AggIntent, candidates: &[SummaryKind]) -> Vec; + + /// Size [`SummaryParams`] for `kind` (one of the candidates + /// [`rank_candidates`](Self::rank_candidates) put first) under the + /// resolved `(eps, delta)` accuracy budget. + /// + /// Splitting sizing out from candidate selection lets a deployment own + /// its own parameter-sizing math (e.g. an empirically-tuned table, or + /// discrete rungs required by a downstream catalog) without forking + /// [`boundary::implementation_for_with`] — the same "one extension + /// point" rationale as `rank_candidates`, one level deeper. Default: + /// [`boundary::default_size_params`], `asap-plan`'s built-in formulas + /// (unchanged) — a deployment that only needs to reorder candidates, + /// not resize them, can leave this method unimplemented. + fn size_params( + &self, + kind: SummaryKind, + intent: &AggIntent, + eps: f64, + delta: f64, + ) -> SummaryParams { + crate::boundary::default_size_params(kind, intent, eps, delta) + } } /// The default cost model: preserves [`summary_candidates`]'s built-in static -/// order unchanged. +/// order and [`boundary::default_size_params`]'s built-in sizing unchanged. /// /// [`summary_candidates`]: crate::boundary::summary_candidates pub struct DefaultCostModel; @@ -105,4 +127,60 @@ mod tests { let ranked = AlwaysPreferLast.rank_candidates(&intent, candidates); assert_eq!(ranked.first(), candidates.last()); } + + /// A deployment that only overrides `rank_candidates` keeps + /// `asap-plan`'s built-in sizing via the trait's default `size_params` + /// body — the split is opt-in per method, not all-or-nothing. + #[test] + fn size_params_default_body_matches_default_size_params() { + let intent = default_cardinality(); + assert_eq!( + AlwaysPreferLast.size_params(SummaryKind::Hll, &intent, 0.01, 0.01), + crate::boundary::default_size_params(SummaryKind::Hll, &intent, 0.01, 0.01), + ); + } + + /// A deployment CAN override `size_params` independently of + /// `rank_candidates` — e.g. to size against a catalog-constrained set + /// of discrete parameter rungs instead of `asap-plan`'s continuous + /// formulas. + struct DiscreteKllRungs; + + impl CostModel for DiscreteKllRungs { + fn rank_candidates(&self, _intent: &AggIntent, candidates: &[SummaryKind]) -> Vec { + candidates.to_vec() + } + + fn size_params( + &self, + kind: SummaryKind, + intent: &AggIntent, + eps: f64, + delta: f64, + ) -> SummaryParams { + match kind { + SummaryKind::Kll => { + let k = if eps >= 0.01 { 200 } else { 2048 }; + SummaryParams::Kll { k } + } + other => crate::boundary::default_size_params(other, intent, eps, delta), + } + } + } + + #[test] + fn custom_cost_model_can_override_sizing_independently_of_ranking() { + use asap_ir::intent_algebra::agg_intent::default_quantile; + + let intent = default_quantile(0.99); + assert_eq!( + DiscreteKllRungs.size_params(SummaryKind::Kll, &intent, 0.001, 0.01), + SummaryParams::Kll { k: 2048 }, + ); + // Untouched kinds still fall through to the default formula. + assert_eq!( + DiscreteKllRungs.size_params(SummaryKind::Hll, &intent, 0.01, 0.01), + crate::boundary::default_size_params(SummaryKind::Hll, &intent, 0.01, 0.01), + ); + } }