Repository navigation
feat(query-engine): add DDSketch aggregation type - #792
Merged
Merged
Conversation
Add a DDSketch quantile aggregation backed by asap_sketchlib::DDSketch, the implementation sketch-bench measures for the planner's cost table. - AggregationType::DDSketch (aliases: DDSketchAccumulator, ddsketch, dd), single-population, served for Statistic::Quantile by capability matching. - DDSketchAccumulator: update, quantile query, merge, msgpack/JSON output. Non-positive values are dropped, as DDSketch indexes positive values only. - Accumulator factory updater; config validation requires an `alpha` parameter in (0, 1). - Planner: DDSketch parameter building (default alpha 0.01, overridable via sketch_parameters.DDSketch.alpha) and sketch properties. The MILP optimizer skips DDSketch until it has a parameter grid and cost rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol
marked this pull request as ready for review
October 6, 2026 19:10
…gatives asap_sketchlib#141 gives DDSketch a zero bucket and a negative store, as ClickHouse's quantilesDD has. At the old pin, zero and negative samples were dropped, so quantiles over data containing them were computed over the positive part only. No release includes the fix yet, so the git pin moves. The accumulator's doc comment and drop test now describe the kept values. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each job now has two instances feeding one DDSketch, sent in time order since the watermark is per group, plus ten zeros per instance. The zeros move the true p90 from 90·scale to 88·scale, outside the alpha bound, so the test fails if zeros are dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Move the DDSketch e2e test off port 19421, which the legacy-feature topk range test also binds. - State the accuracy bound as |x̂ - x| <= alpha·|x|; (1+alpha)/(1-alpha) is the bucket width, not the guarantee. - Count the negative store in memory_usage_bytes. - Drop issue numbers from code comments, per AGENTS.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
merge_accumulators_batch had no DDSketch arm, so merging N buckets went through merge_with, which clones the running sketch at every step. DDSketchAccumulator::merge_multiple clones the first sketch once and merges the rest into it in place, as the KLL and CMS arms do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
milindsrivastava1997
approved these changes
Oct 6, 2026
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
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
Adds a
DDSketchquantile aggregation to the ASAPQuery data plane and query engine. This is the DDSketch slice of #787 (and theDDSketchaggregation type from #762); CountSketch and UnivMon are not included.The accumulator wraps
asap_sketchlib::DDSketch, the implementation sketch-bench measures, so the optimizer's DD cost rows describe what ASAPQuery runs. It is needed for the data-plane vs ClickHouse evaluation (#789), which compares DDSketch on both sides (quantilesDD(α)in ClickHouse).AggregationType::DDSketch(aliasesDDSketchAccumulator,ddsketch,dd). It is single-population likeDatasketchesKLL, and capability matching servesStatistic::Quantilefrom it.DDSketchAccumulatorhandles update, quantile query, merge, and msgpack/JSON serialization. A mismatchedalphaor a different accumulator type is rejected on merge. Negative values go to a mirrored store and zeros to a zero bucket, as in ClickHouse'squantilesDD; non-finite values are dropped. An empty sketch returnsNaN.DDSketchaggregation requiresparameters.alphain (0, 1) and fails validation otherwise (InvalidAlpha).DDSketch::newpanics outside that range, so the check runs before the accumulator is built.asap_sketchlibmoves from94d76f6to5158bf2for the zero bucket and negative store (fix(ddsketch): retain negative observations and zeros asap_sketchlib#141), which no release includes yet. No other sketch's tests change.{alpha}with a default of 0.01, overridable viasketch_parameters.DDSketch.alpha, and adds the sketch's properties (mergeable, not subtractable).Not included
DatasketchesKLL(map_statistic_to_precompute_operator). Choosing DD is part of--planner milp(Optimizer: move sketch-bench #129 MILP into asap-planner-rs (--planner milp) #753).DDSketch(OPTIMIZER_SKIPPED_AGG_TYPES) until it has a parameter grid andatomic_costsmapping (Add optimizer aggregation families and capability/SLA metadata #762). Without the skip it would propose DD candidates with noalpha.HashMaporder puts first (already true of KLL vs HydraKLL). Breaking ties byaggregation_idis a follow-up.Tests
merge_with); mismatched α or type is rejected; serialization round-trips.config_is_keyedagrees with the updater.alphaand is rejected without one.DDSketchconfig.asap_sketchlibmoves from94d76f6to5158bf2for the zero bucket and negative store (fix(ddsketch): retain negative observations and zeros asap_sketchlib#141), which no release includes yet. No other sketch's tests change.e2e_grouped_quantile_over_ddsketch_is_within_alpha): Remote Write → precompute → one DDSketch per group →quantile by (job) (0.9, …). Each job has two instances and twenty zeros; each group's estimate is within α of the true p90, which the zeros move outside the bound if they were dropped.cargo testforasap_types,promql_utilities,asap_plannerandquery_engine_rustpasses;cargo test --workspacepasses (1,187 tests).cargo clippy --workspace --all-targets --all-features --locked -- -D warningsandcargo fmt --checkare clean.Part of #787.
🤖 Generated with Claude Code