feat(sketch_db): fold sketch_kind/sketch_config into agg_kind - #154
Merged
Merged
Conversation
…chInstanceMetadata
Phase 5 M2.3.2 — replaces the M2-era `sketch_kind: SketchKindHandle`
+ `sketch_config: SketchConfig` field pair on `SketchInstanceMetadata`
with a single `agg_kind: AggKind`. Same struct can now describe
either a sketch-backed sid (`AggKind::Sketch { kind, config }`) or a
precompute-backed sid (`AggKind::Precompute { agg_type,
parameters_canonical }`) — the latter lights up in M2.3.3+.
`capability: Option<Capability>` and `accuracy: Option<AccuracyBound>`
because precomputes answer exact statistics and have no warm-tier
approximation profile. The warm-tier reducer (sketch-only) handles a
`None` capability as a defensive `UnsupportedCapability` rather than
a panic.
`sketch_kind()` and `sketch_config()` accessor methods return
`Option<_>` so existing sketch-only consumers (`sketch_reducer.rs`)
keep working with a thin `.expect("only handles sketches")` —
upstream classification already filters precompute sids out of the
reducer's caller list.
8 ingest + test construction sites updated. 798/800 lib tests pass
(the 2 pre-existing `schema_timeline_dispatch_tests` failures
predate this PR).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Jul 22, 2026
ASAPController#146 (CostModel::size_params) and #154 (bind::logical visibility) are both merged. Re-pin from the feat/costmodel-size-params-hook branch tip to main's current commit now that both land there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Jul 22, 2026
…#408) * feat(control_plane): adopt asap_sketch::L4Node for the L4 IR (Step B) Retires control_plane's own locally-defined, flat PhysicalExpr L4 algebra (Logical/SketchAgg/SketchEstimate/SketchMerge/ExactAgg) in favor of ASAPController's canonical L4 IR, asap_sketch::{SummaryExpr, L4Node} -- the same move Step 3 of the enum-unification made for SketchKind -> SummaryKind, one layer up. PhysicalExpr is now a thin L5 wrapper: `Committed(L4Plan)` for the common case, plus the two Phase eps.1 placement variants. L4Plan keeps only what asap_sketch genuinely doesn't have -- named LetBinding/Ref fan-in sharing (asap_sketch's own DAG sharing is structural, via Rc, but this crate's rule-firing walk still needs a name to thread a bound value across sibling calls). The 7 bind_kll_quantile/bind_ddsketch_quantile/bind_hll_cardinality/ bind_cms_count/bind_cms_topk/bind_exact_agg/bind_archive_only Rule structs are retired -- their selection/sizing policy (KLL's k rungs, DDSketch-over-KLL priority, TopK recall tiers, wire-cost tie-breaks) is preserved verbatim in the new ControlPlaneCostModel, plugged into asap_plan::bind::implement_tree_in_with via the CostModel trait (rank_candidates + the new size_params hook, ASAPController#146) instead of a bespoke dispatcher -- so schema derivation, col/by computation, and DAG construction are asap_plan::bind's, not a forked copy. Three node shapes get a small local pre-pass in sketch_algebra::lower before delegating, because asap_plan::boundary::implementation_for actively binds them to something this deployment's data plane can't (or deliberately shouldn't) serve: - AggIntent::Count{accuracy: Exact} would bind SummaryKind::Count, which has no data-plane accumulator (PR #200/#201 already established this is wrong -- reverted, stays on archive). - AggIntent::Rate would bind its own SummaryKind::Rate; this deployment represents Rate as an Increase accumulator (rate = increase / window, a query-time division, not a separate accumulator). - AggIntent::Extension (Frequency) and TopK{accuracy: Exact} both decline to bind at all (asap_plan's Extension/exact-TopK coverage gaps -- filed upstream as ASAPController#150 and #151); this is a real, accepted behavior change from the retired bind_cms_count/bind_cms_topk rules, not a bug -- see the updated tests in optimizer/rules/mod.rs and emit/mod.rs. Also fixes a real bug surfaced by the migration: PhysicalExpr now carries Rc<L4Node> (asap_sketch's own DAG-sharing mechanism), so holding a bound PhysicalExpr across an .await point made handle_plan's generated Future !Send, breaking axum::Handler. Scoped the Rc-bearing computation into a synchronous block that resolves down to Send-safe StageConfig output before the first await. Pins ASAPQuery-backend's ASAPController dependency to 12482fd77945ab5771a021c6112750fd9284f8ed (ASAPController PR #146, CostModel::size_params + bind::logical). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore: re-pin ASAPController to main (post #146/#154 merge) ASAPController#146 (CostModel::size_params) and #154 (bind::logical visibility) are both merged. Re-pin from the feat/costmodel-size-params-hook branch tip to main's current commit now that both land there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 5 M2.3.2 — replaces the M2-era
sketch_kind+sketch_configfield pair onSketchInstanceMetadatawith a singleagg_kind: AggKind. Same struct can now describe either a sketch-backed sid (AggKind::Sketch { kind, config }) or a precompute-backed sid (AggKind::Precompute { agg_type, parameters_canonical }— wired up in M2.3.3+).capability: Option<Capability>andaccuracy: Option<AccuracyBound>(None for exact precomputes).sketch_kind()/sketch_config()accessor methods returnOption<_>. The warm-tier reducer (sketch-only) keeps working with a thin.expect("only handles sketches")— upstreaminstances_matchingalready filters precompute sids out.Option<Capability>handling insketch_reducer::require_capabilitysurfaces aNoneasUnsupportedCapabilityrather than panicking.Test plan
cargo test -p data_plane --lib --no-fail-fast— 798/800 pass (2 pre-existingschema_timeline_dispatch_testsfailures unrelated).cargo check --workspaceclean.data_plane/tests/e2e_modified_otlp_sketch_path.rs— pre-existing struct-field-removed errors documented in the prior session summary; not in scope.🤖 Generated with Claude Code