Repository navigation
fix(ir): top-k sketch readouts return selected rows; Example 1 expectations follow planner output - #579
Draft
zzylol wants to merge 3 commits into
Draft
fix(ir): top-k sketch readouts return selected rows; Example 1 expectations follow planner output#579zzylol wants to merge 3 commits into
zzylol wants to merge 3 commits into
Conversation
zzylol
added a commit
that referenced
this pull request
Oct 4, 2026
Follows #579: the top-k sketch readout derives one row per selected item instead of a packed Utf8 column, matching the exact Sort → Limit shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 4, 2026
zzylol
force-pushed
the
stack/509-c5-runtime-consistency
branch
from
October 5, 2026 06:21
5796e23 to
8d68e5a
Compare
zzylol
force-pushed
the
stack/572-b1-types-modules
branch
from
October 5, 2026 06:21
dfaa958 to
7eda06e
Compare
zzylol
force-pushed
the
stack/509-c5-runtime-consistency
branch
from
October 8, 2026 16:47
8d68e5a to
38e7c90
Compare
zzylol
added a commit
that referenced
this pull request
Oct 9, 2026
Merge coverage is cached at construction (#646); full per-type overlap table from #592; k mismatch is a schema rejection; #579 top-k shape; tighter cost bounds; walk-order caveat for combining conditions; Project qualifier; absolute time marked later; Example B scope; one-walk wording; define by[job] and closed schema; link Pass 2; window notation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
zzylol
force-pushed
the
stack/509-c5-runtime-consistency
branch
from
October 10, 2026 14:32
38e7c90 to
6964909
Compare
SummaryEstimate{TopK} derived `partition keys + topk Utf8`, an encoding
no runtime operator produces. The runtime's keyed evaluation, the exact
Sort -> Limit path and EvaluatePopulation{TopK} all return the selected
rows, so every CountSketch+heap Example 1 candidate failed to compile.
Derive partition keys + item identity columns + Float64 `value`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The 6-candidate spec did not account for Pass 1's exact-accumulator options (user decision). Un-ignore the count-only tests, check that the runtime compiles exactly the candidates Stage 3 finds valid and rejects Count-Min + heap for the same reason, and list the 24 candidates in the acceptance spec for manual review. Hydra and shared-input tests stay ignored, naming the missing feature. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
zzylol
force-pushed
the
stack/572-b1-types-modules
branch
from
October 11, 2026 17:11
0babc9d to
bd25cf4
Compare
zzylol
force-pushed
the
stack/509-c5-runtime-consistency
branch
from
October 11, 2026 17:11
6964909 to
b3f8c6e
Compare
zzylol
added a commit
that referenced
this pull request
Oct 12, 2026
…ion) (#573) * docs: design for ASAP primitive schema and summary semantics One design document for the edge schema of ASAP primitives: fields that carry summary state, the summary semantics a schema must preserve, coverage, and per-operator examples. Related docs keep a summary and link to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: restore the hand-written sections before the examples Keep the author's sections 1–4 as written; the operator examples (§5) and key code interfaces (§6) stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: a top-k readout returns selected rows Follows #579: the top-k sketch readout derives one row per selected item instead of a packed Utf8 column, matching the exact Sort → Limit shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: explain why summary coverage is a node field, not part of the schema Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: summary coverage records rows and columns SummaryCoverage records which columns a state summarizes (input, group_by) as well as which rows. Update the SummaryAgg and SummaryMerge examples to #560: merges compare coverage columns, carry no coverage until #646, and merged_coverage is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: summary coverage as definition + selection, based on Goldstein-Larson view matching Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: the 5.1 example's time index is a Timestamp column Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: absolute time is a timestamp-column interval; match the coverage interface in #646 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: ambiguous column names and literal types in selections Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: fill §1 links and operator list, §2 terminology, §3 summary type terms Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: point links into asap-primitive-schema.md at its current sections Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: list the levels of a summary type as a table Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: list supported query intents per summary type Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: structure §4 with summary table, rule tables and bullet lists Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: rename §4.2 heading Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: explain the residual condition of view matching Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: define the three parts of a view's WHERE with examples Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: define equivalence classes, ranges and residuals Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: move the Goldstein & Larson mapping after §4.2 as §4.3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: say which paper requirement unions of views relax Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: SummaryMerge is implemented on main since #560 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: §4.7 leaves the deployment runtime, such as SDS, out of coverage Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: rewrite §4.1 as a worked G&L example and §4.2–§4.5 in plain terms Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: worked examples for §4.4 and §4.5; drop the §5.8 recap table Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: draw the §5 examples as diagrams and timelines Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: reword how coverage is computed in §4.4 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: explain the selection derivation in §4.4 as goal and steps Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: move the Goldstein & Larson mapping into a footnote of §4.2 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: clearer wording for Rule 1 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: draw §5 examples as bottom-up operator trees Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: draw §4.4 and §5 examples as DAGs with shared nodes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: rename step 3 in §4.3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: phrase step 3 outcomes as conditions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: plainer Rule 1 and Rule 2 details with reasons and examples Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: §4.3 derives the definition first, then the selection Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: name the code interface in the §4.5 heading Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: move the coverage code interface to §6.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: §6 intro points to §6.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: merge §4.2 and §4.3 (why two parts, then derivation); fold §4.4 into §5.6 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: refer to the downstream deployment runtime, not a specific backend Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: same wording in the §2 layout row Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: leave the SDS definition as a TODO for a separate doc Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: make §5 easier to read with one question table per operator Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: explain every struct and variant field in §6 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: label §5 operator descriptions as operator definitions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: explain every function in §6 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: describe SketchKind.category as the aggregation intents it answers Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: explain what §6.3 update input and readouts are for, with examples Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: rename §6.2 to summary-family parameters Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: show how the §6 code interfaces connect Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: order §6 subsections from the DAG node inward Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: add the cost of deriving coverage (§4.2.3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: fix correctness issues found in review Column identity follows the SummaryAgg's input schema (a Project renames and drops the table); mark §6.1/§6.6 as #646 and per-family overlap as #592; panes come from window composition (Pass 2); add IRate; three selection shapes; current SummaryEstimate top-k output; time row of the definition table; drop unchecked paper section numbers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: fix second-review findings Merge coverage is cached at construction (#646); full per-type overlap table from #592; k mismatch is a schema rejection; #579 top-k shape; tighter cost bounds; walk-order caveat for combining conditions; Project qualifier; absolute time marked later; Example B scope; one-walk wording; define by[job] and closed schema; link Pass 2; window notation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: fix §1 grammar; start every example from its PromQL or SQL query Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: restore the author's wording in §1, §3 and §4 openings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: show ASAPOp generic over its child reference, as on main Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: describe the union and instant-selector rules as #646 implements them Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: touching value ranges join; instant selectors join the later lifting rules Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: PromQL comparisons by lowering; constants from folded casts A vector/scalar comparison lowers to a Filter above rate and moves to the selection; a bool comparison is a Project and a vector/vector comparison a BinaryOp, both kept in the definition. The SQL front end folds numeric casts of literals as DataFusion does, so `latency < 100` reads as a constant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW * docs: only a Project may rename on the way to the selection Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW --------- Co-authored-by: Claude Opus 5.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
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.
Rebased on main d4869a7 (DF 54).
Why
#509 Example 1 runs end to end (1 → 24 → 24 → 1), but only 8 of the 24 Stage 2 candidates compiled in the runtime (
physical_planner::compile). The acceptance tests still expected the spec's 6 candidates.What
ASAPOp::output_schemaderivedSummaryEstimate{TopK}aspartition keys + topk Utf8. No runtime operator produces or reads that encoding. The runtime's keyed evaluation, the exact Sort → Limit path andEvaluatePopulation{TopK}all return the selected rows, sokeyed_evaluationrejected the IR's schema ("invalid keyed evaluation shape"). The IR now derives the selected rows: partition keys + item identity columns + Float64value. With this, all three top-k implementations of Q2 return one row per selected series.sum_over_timeof raw samples. Nothing in the workload or catalog declareshttp_requests_totalnon-negative, and neither existing proof (UnitCount,ResetAwareCounterDerivative) applies. The legacy rule also leaves this caseUnknownOrSigned. Proving it would need a metric-type input (a counter or non-negative declaration) and a matching proof variant. The runtime and Stage 3 rules are unchanged and agree.How
crates/types/src/ir/operator/asap.rs: addsranked_rows_schemaforSketchStatistic::TopK. New unit test:structure_contract::topk_readout_derives_selected_rows.stage2_count_sketch_heap_topk_compiles_in_the_physical_planner.stage2_runtime_compiles_exactly_the_candidates_stage3_finds_valid(replaces the ignored "every candidate compiles" test) checks that the runtime compiles the 16 valid candidates and rejects the 8 CMS+heap candidates, for the same reason Stage 3 gives.stage1_has_24_candidates_covering_every_combination(wasstage1_has_six…),stage2_keeps_every_logical_candidate,stage2_preserves_logical_choices,stage2_exact_topk_is_sort_then_limit(8 exact),stage2_summary_topk_is_build_then_estimate, andstage3_charges_each_node_once, which now covers the priced (valid) candidates.stage1_q2_summary_families_are_heap_sketches_and_hydra(no Hydra),stage1_keeps_independent_and_shared_variants(Pass 2 sharing, Hydra),stage3_shared_input_is_not_costlier(Pass 2 sharing, Hydra).stage_pipeline. Only the CountSketch/CMS estimate node schemas changed.Before this PR: 8 of 24 Example 1 candidates compile in the runtime. The 8 CountSketch+heap candidates fail on the readout shape, and the 8 CMS+heap candidates fail on the non-negative weight rule. 10 acceptance tests are ignored.
After this PR: 16 of 24 compile, which is every candidate Stage 3 finds valid. The 8 CMS+heap candidates are rejected by both the runtime and Stage 3 for the non-negative weight reason. Stage 3 still selects P20 (all exact, 79.401 cpu ms), and every rejection reason is unchanged. 3 tests are ignored, each for a missing feature.
Follow-up: §5.2 of the
docs/asap-primitive-schemabranch still saystopkUtf8 and should say "selected rows". This PR does not touch that branch.🤖 Generated with Claude Code