Repository navigation
fix: read sketch capabilities and state bytes from asap_sketchlib (#953, #946, #947) - #989
Merged
Merged
Conversation
Contributor
|
@GordonYuanyc I pushed one commit on top of yours ( sketchlib side (merged as ProjectASAP/asap_sketchlib#254,
This PR
Tests
The PR body's What / How / interfaces / Verification / decisions sections are updated. Please take a look, especially the derived- 🤖 Generated with Claude Code |
zzylol
reviewed
Oct 28, 2026
…ly-capability table `summary_candidates` was a hand-written intent → family list that repeated the family-capability table in the other direction (#953). It now takes the intent's statistic and keeps, in `FAMILY_PREFERENCE` order, the families whose table row answers it, less `UNIMPLEMENTED_FAMILIES` (Theta) and less UnivMon for a distinct-tuple count. Candidate lists and their order are unchanged. - The intent → statistic mapping is written once, `sketch_statistic`; Pass 1's `statistic` wraps it with its existing error. - `local_guarantee` returns `None` for a sketch read for a statistic its table row does not answer. Plain values and exact aggregates are not checked: the table lists no readouts for them. - Two estimator tests read CMS for a distinct count and Count Sketch for a total, which the table does not answer; they now read a statistic each family answers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main after release 0.4.0 carries #245–#248, which the old pin took from a branch, plus #249 (`memory_bytes` / `estimated_memory_bytes` across the sketches; KMV builds without `experimental`) and #251 (the public capability table). The executor drops the `experimental` feature: KMV was the only gated type it used. #251 makes `Kll::merge` panic when k or m differ. The KLL kernel checked only k, so a state adopted with another m would reach the panic; it now checks both and returns an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…from asap_sketchlib (#953) The family-capability table kept its own copy of what each sketch family answers and how it merges. A sketch family's row is now asap_sketchlib's (`SketchFamily::capability`, #251), mapped from `SketchAlgorithm` and `HydraKind` and read in the planner's vocabulary at query time: the seven statistics `StatisticKind` names, `NoGuarantee` as `Approximate`. The planner keeps the rows the library has none for: the exact accumulators and Theta. `readouts` becomes a method; its readers follow. Cells that change: - Count-Min and UnivMon totals are `Exact`, not `AsIfBuiltOverUnion`. Every consumer accepts both alike. - HydraKll merges disjoint inputs, its quantile as if built over the union: sketchlib's Hydra merge now requires the same k and m and returns an error otherwise. Nothing builds a HydraKll yet: Pass 1 offers no Hydra and the executor binds only HydraCms. - UnivMon lists its readouts in sketchlib's order, so its entry in the executor's supported summaries does too. sketchlib also lists DDSketch's total, the heap sketches' item frequency and HydraCms's total, which nothing here reads yet. `NOT_READ_YET` keeps them out, so Pass 1's candidates, the IR's estimate validation and the executor's supported summaries are unchanged; a test checks sketchlib still lists each entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…unts (#947) A DDSketch state was reserved at the trait's flat 4096 bytes whatever it held, and its updater counted the bins in use; a UnivMon state by a hand formula (16 bytes per counter, 256 per heap slot). Both now read asap_sketchlib's count (#249): - DDSketch: `memory_bytes()`, the bins its stores allocate; the updater charges the same. - UnivMon: its own fields, `UnivMon::estimated_memory_bytes` for its shape with keys that own no bytes, and the existing bound on the variable-length keys its heaps retain. `memory_bytes()` walks every heap, and a build charges memory after every row, so the charge stays constant time. KLL, KMV, Count-Min, Count Sketch (with or without a heap) and HLL already charge what their sketch holds plus their own fields; Hydra has no sketchlib count. They are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ates (#946, #947) Plan selection priced Count-Min, Count Sketch (with or without a heap), DDSketch, HLL, KMV and UnivMon states from hand-copied sketchlib internals: 24 bytes per top-k heap slot (a slot holds 150–225 plus its key), 8 per UnivMon counter, DDSketch's bucket chunks. Each is now asap_sketchlib's `estimated_memory_bytes` (#249) for the type the executor builds (`f64` counters, `Vector2D` storage), given what `sketch_shape` assumes the state holds: - Count-Min, Count Sketch, HLL: exact. - KMV: the rows the state folds as its distinct items; exact only at that many distinct inputs, an upper bound below. - DDSketch: sketchlib's worst case over insertion orders and merges, for the rows it folds over the assumed nine decades, with negative values unless proven non-negative: 37,360 bytes at α = 0.01 with rows unknown, where the old model gave 18,576. - Count-Min / Count Sketch with heap: a full heap of keys of `ASSUMED_HEAP_KEY_BYTES`, an assumption of three 16-byte labels in the executor's tuple-key encoding (63 bytes). It applies to these heaps only. - UnivMon: every layer's heap full of keys of `ASSUMED_VALUE_KEY_BYTES`, one 16-byte label value with the executor's type tag (17 bytes): Pass 1 feeds UnivMon one column, which a distinct count over a label makes Utf8. KLL keeps the planner's own formula, unchanged: sketchlib's estimate also counts a CDF cache the executor never builds. Theta keeps its `k` hashes. An α outside (0, 1), where sketchlib's DDSketch estimate panics, is priced as an empty sketch. A test builds each family the executor builds per group through its factory, feeds it rows shaped as the pricing assumes, and compares the executor's charge with the priced bytes plus each accumulator's own named fields. The heap families' shapes are dominated by their heaps. - Count-Min, Count Sketch, HLL and KMV: equal. - KLL: equal up to k rows. Past k only a loose guard holds, at most sketchlib's 2 (k - 1) items of compaction scratch over the price; the formula can price below the charge there, a known gap until a sketchlib estimate without the CDF cache can price KLL. - DDSketch, the heap sketches and UnivMon: at least the charge, and at most 1.5 times it once the state is full. UnivMon's price also bounds the bytes its sketch holds, by at most 1.6 times: its estimate includes digest-index growth under churn that the test's labels do not reach. HydraCms is left out: plan selection prices an independent Count-Min per group, the executor one shared grid. Plan selection's tests gain the executor as a dev-dependency for the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…xecutor support declaration - SketchAlgorithm, StatisticKind and MergedError are asap_sketchlib's (exhaustive, serde); the Planner's copies, the Readouts enum, NOT_READ_YET and the conversion matches are gone. FamilyCapability's sketch rows are the library's rows, unchanged; the exact accumulators keep Planner rows. - Theta is deleted (no library sketch), with UNIMPLEMENTED_FAMILIES. - The executor declares what it evaluates in one positive list, EVALUATED_READOUTS, replacing EVALUATED_FAMILIES and UNEVALUATED_READOUTS; a test checks every entry is in the table, so a sketchlib bump cannot widen the declared support. - The four hard-coded heap-family TopK checks read answers(TopK); the pcc.rs statistic() wrapper is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n comment Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol
force-pushed
the
fix/sketchlib-table-and-memory
branch
from
October 29, 2026 03:10
af848d9 to
1ba3c44
Compare
This was referenced Oct 29, 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.
Closes #953. Closes #946. Part of #947 (HydraCms and KLL past k remain; see Limitations). Part of #1012 (removes the parallel capability vocabulary and the two "not read yet" lists).
Why
Two kinds of sketch facts were hand-written in the Planner, and they drifted from each other and from asap_sketchlib.
summary_candidates(intent → families). fix(types,executor): one family-capability table decides which statistic a family answers #938 had to edit both; Executor has no KMV kernel although Pass 1 offers KMV for distinct counts #697 is an earlier drift. The team then asked for the table itself to live in sketchlib ("make it one table in sketchlib"). asap_sketchlib asap-aware-mapping: generalize summary/CSE selection into a ReplacementStrategy trait with multiple candidates #251 added it. Reading it through a second vocabulary still left duplication:SketchAlgorithm,StatisticKindandMergedErrorparalleled sketchlib's enums. The conversions ended in_ => return None, so a family or statistic sketchlib added would have been skipped silently.NOT_READ_YET(types) andUNEVALUATED_READOUTS(executor).TopKHeapslot holds 150–225 B plus the key.memory_bytes/estimated_memory_bytesfor these families.What
21b6357(QueryExpr: merge BinaryOp into Compare/Arithmetic (Instance 2 of #220) #245–feat(dag-viewer): lane-based Compare/Union DAG-merge view #249, asap-aware-mapping: generalize summary/CSE selection into a ReplacementStrategy trait with multiple candidates #251, and asap-aware-mapping: group-by-lattice roll-up reuse (fine-to-coarse) as a ReplacementStrategy #254, which renamed the capability vocabulary to the Planner's names). The executor no longer needs theexperimentalfeature, since KMV left it.SketchAlgorithm,StatisticKindandMergedErrorare now asap_sketchlib's types, re-exported fromasap-types. These enums are exhaustive, so a family or statistic added in sketchlib is a compile error in the Planner until it is handled.FamilyCapability's sketch rows are sketchlib's rows (src/capability.rs), read unchanged. The exact accumulators keep Planner rows.answers(TopK).EVALUATED_READOUTSlists what it evaluates per family. It replacesEVALUATED_FAMILIES+UNEVALUATED_READOUTSin the executor andNOT_READ_YETin types.estimated_memory_bytesfor the type the executor builds.minstead of reaching sketchlib's new panic.For #953, Pass 1 offers the same candidates in the same order with the same prune reasons. Memory prices change for the heap families, UnivMon, DDSketch and KMV (see Before / After).
How
FamilyCapability { merge_inputs, readouts }.of_per_instance_sketch(algorithm)returnsalgorithm.capability()'s readouts unchanged.The merge rule goes through an exhaustive
From<asap_sketchlib::MergeInputRule>. The Planner'sMergeInputRuleaddsDisjointInTimefor the exact counter accumulators.A Hydra kind maps to sketchlib's
HydraCms/HydraCountSketch/HydraKllrow.The rows the Planner reads, at
21b6357. Merge: D = Disjoint, O = MayOverlap. Error: U = AsIfBuiltOverUnion, E = Exact, A = Approximate.KllDDSketchCmsCountSketchCmsWithHeap/CountSketchWithHeapUnivMonHll/KmvHydraCmsHydraCountSketchHydraKllsummary_candidates(intent)maps the intent to its statistic and keeps the families inFAMILY_PREFERENCEwhose row answers it. It drops UnivMon for a multi-column distinct count.FAMILY_PREFERENCEis the families the Planner sizes.size_paramslists sketchlib's 19 other families by name (no_arm), so a new sketchlib family fails to compile there until it is sized or listed.sketch_shapecalls each family'sestimated_memory_byteswith the shape the state is assumed to reach. KLL keeps the Planner's own formula; see Architectural decisions.supported_summary_families()returnsEVALUATED_READOUTS.validate_sketch_evaluationchecks the table row and that list. A test checks that each declared readout is in the family's table row and that the factory and kernels answer exactly what is declared.DDSketchAccumulator::approx_memory_bytesreturnsmemory_bytes(). UnivMon charges sketchlib'sestimated_memory_bytesplus its own fields and the key bytes it retains. The other families already charged sketchlib'smemory_bytes().Key code interfaces added / modified
asap_sketchlibsrc/capability.rsat21b6357. This is the one capability table. It comes from ProjectASAP/asap_sketchlib#251 and #254; the Planner reads its types directly.crates/types/src/ir/schema/crates/executor/src/capability.rscrates/logical-asap-optimization/src/realization.rscrates/plan-selection/src/lib.rscrates/executor/src/summary_kernels/End to end, with numbers from the tests below:
Major algorithm changes
Pass 1 candidate selection (#953).
matchfrom intent to a hand-written family list.sketch_statistic(intent);FAMILY_PREFERENCEbyFamilyCapability::of_per_instance_sketch(f).answers(statistic);summary_candidates_keep_the_hand_written_lists.State-bytes cost model (#946, #947).
estimated_memory_bytesper family.state_bytes_bound_the_executors_charge. KLL past k is the exception (see Limitations).Before this PR
MemoryLimit.After this PR
EVALUATED_READOUTSis edited.u64keys, as Plan selection underestimates top-k heap memory by about 9× #946 measured).Evidence
state_bytes_bound_the_executors_charge.Verification
Unit tests
state_bytes_bound_the_executors_charge(new, plan-selection): builds every per-group family through the executor's factory and feeds rows shaped as the pricing assumes. It fails if plan selection and the executor disagree beyond the stated bounds. Priced ÷ charged with the state full:Runs in 2.1 s.
family_capability_table(types): every sketch row equals sketchlib's row (Hydra kinds included), and every exact row is pinned.supported_summary_families_match_the_factory_and_evaluations(executor): each declared readout is in the family's table row, and the factory and kernels answer exactly what is declared.summary_candidates_keep_the_hand_written_lists,a_dropped_readout_drops_the_family,preference_order_names_each_family_once(new, logical-asap-optimization):local_guarantee_needs_a_statistic_the_family_answers(new): a local guarantee isNonefor a statistic the family does not answer.merge_rejects_a_different_k_or_m(extended, executor).End-to-end tests
cargo test --workspace --no-fail-fast): 2,236 passed, 38 failed, 33 ignored. All 38 failures came from one intermediate choice: dropping Count-Min's and UnivMon'sTotal, which took away approximatecount(). That choice was reverted in sketchlib asap-aware-mapping: group-by-lattice roll-up reuse (fine-to-coarse) as a ReplacementStrategy #254, since both totals are read from the sketch's own counters. The 13 affected suites then pass: 873 tests, includingasap-plan-selection --lib(374),asap-logical-asap-optimization --lib(233) and itsper_computation_candidatestarget (35),stage_pipeline(47), and the executor, PromQL count and integration targets.Other checks
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings: clean.cargo fmt --all -- --check: clean.tools/verify_metricsql_external_consumer.sh,tools/verify_metricsql_vendored_baseline.py.Architectural decisions
DisjointInTime) stay in the Planner.EVALUATED_READOUTS), a positive list, so a sketchlib bump cannot widen the declared support.SketchAlgorithm/StatisticKindserialize as before ("Cms","Quantile", …) and deployment config is unchanged.Ordfollows sketchlib's variant order, not the Planner's old order. Every test passes with it.AsIfBuiltOverUnion→Exact. Every reader accepts both alike.validate_sketch_evaluation), and Pass 1 has no intent that reads them.AsIfBuiltOverUnion): sketchlib asap-aware-mapping: generalize summary/CSE selection into a ReplacementStrategy trait with multiple candidates #251 made that merge sound. Nothing builds a HydraKll yet; Pass 1 offers no Hydra and the executor builds only HydraCms.MemoryLimit. Rejected: keeping the Planner's clustered, non-negative model.memory_bytes().memory_bytes()walks every heap, and the build charges memory after every row (about 50× slower).sketch_shapepublicly.Limitations and follow-up
asap-typesnow depends on asap_sketchlib (a git dependency until 0.4.1 is released), soasap-typescannot be published to crates.io before then.sketch_shape's bytes are the size axis there, so deployments with a measured table get slightly different interpolation weights.Human review — do not complete with an agent
🤖 Generated with Claude Code