docs: clean up L3 QueryExpr; add schema flow + L4 SketchExpr + system I/O - #1
Merged
Merged
Conversation
… I/O
Address review feedback on the QueryExpr/AggIntent design:
- Drop redundant L3 nodes (TopK = Sort+Limit; WindowedAgg = Window+Aggregate)
- Move sketch-bound nodes (SketchAgg, JoinSketch) out of L3 into a new L4
IR (SketchExpr) with SketchSubtract/Delete/Estimate/Merge added
- Drop language-shaped nodes from L3 (HistogramQuantile, PromQLSubquery
lower in PromQL L1->L2 instead)
- Rename Dedup -> Distinct{cols} (SQL DISTINCT, generalised to N cols)
- Drop QuantileOverTime/TopK from AggIntent; surrounding Window covers it
- Add Schema/Field/HasSchema with per-node input/output spec table
- Distinguish three schemas: DAG schema vs DB/source schema vs sketch
catalog metadata
- Expand SketchCatalog with mergeability, accuracy, supported intents,
aggregated-key shape, parameter ranges
- New "System I/O contract" subsection: input is QueryWorkload, output
is per-executor sub-DAG assignments under the premise that every
executor has full capability and the controller chooses placement
- Refresh resolved-questions list in Section 12
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- TopK is a first-class AggIntent at L3 (heavy-hitter sketches like SpaceSaving / CMS-with-heap / Misra-Gries serve it as a single primitive). Generic QueryExpr::Sort + QueryExpr::Limit stays for non-heavy-hitter cases (ORDER BY name LIMIT 10). L1→L2→L3 lowering picks the intent form when it recognises a heavy-hitter pattern. - Clarify CTE / let-binding "reuse edges" wording: one producer + N Ref(name) consumer edges, which is what makes the IR a DAG. - Per-executor split: sub-tree -> sub-tree / sub-DAG. - Cut-edge transports: drop Kafka mention, replace with "sketch-merge / compute-from-raw over the precompute engine". - serialise -> serialize. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reframe "DAG, not tree" around the operational distinction: a producer node can have multiple parents that share its precomputed intermediate state. List the three fan-in sources: explicit CTE / recording-rule references, L4 reuse rules introducing shared producers across queries, and L4 stage / shard structure. The cost-model implication (credit build cost once across consumers) is what makes the DAG shape a Pareto win over a tree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the S / L / R / S_i shorthand; spell out each input and output in plain language so every row reads independently. Also split Sort and Limit into separate rows now that each gets its own constraint description (Sort requires keys' fields be present in input; Limit has no field-level constraint). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a self-contained table covering the seven SketchExpr variants (Logical pass-through, SketchAgg, SketchJoin, SketchSubtract, SketchDelete, SketchEstimate, SketchMerge) with input and output schemas, and introduces DataType::Sketch(SketchKind, SketchParams) as the L4 field type that carries sketch state. Two type-system invariants spelled out: sketch-family mismatch is a plan-time error, and catalog capability flags (subtractable, deletable, mergeable) gate which nodes can fire. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…glossary - Rename scenario→deployment model across all three docs. Crate names get an asap- prefix (deployment-model-asaplifecycle / -asapquery / -asapfusion). "Scenario" understated the bundle; "deployment model" names the choice — rule selection + L5 topology + emitter all reflect deployment constraints. - Distinguish stage from executor as first-class concepts. Stage (StageId) is a categorical lifecycle tier; Executor is a concrete runtime instance occupying a stage with its own id / address / capabilities. StageAllocator stays stage-granularity; PhysicalPlanner fans out per executor via DeploymentConstraints::executors(). - Rename §6 "Three distinct schemas" → "DAG schema, DB schema, sketch catalog — three distinct metadata sources" and clarify the sketch catalog is a registry of available primitives, not a schema. - Add §14 glossary covering architecture roles, layer drivers, IR by layer, metadata sources, and workload identity. Renumber Success criteria to §15. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Naming was inverted: L3 was called "sketch algebra" but contains no sketches (intent-only); L4 is where sketches actually get bound. Split the module accordingly so the names match the contents: - L3: `core::intent_algebra` — QueryExpr + AggIntent (intent only, no sketch type, no params). - L4: `core::sketch_algebra` — SketchExpr (sketch-bound: kind + params committed). Promoted from a #### subsection of the old L3 section to a sibling ### section. Touches the §3 layer table, §6 module sections, the directory tree, the §14 glossary L3/L4 entries, and ~10 cross-references across README.md, design.md, migration-plan.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lanning The previous doc comment described unique_keys as a "sketch-reuse driver" without flagging that it's optional infrastructure for one specific feature (`CostModel::workload_cost` cross-query reuse credit). Tighten the comment to call out: who populates it (per-node spec), who reads it (workload_cost only), and what doesn't read it (Bind* rules, push-down, L5 emitters, single-query plans). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Worked example for `quantile_over_time(0.99, http_request_duration_seconds{service="api"}[5m])`
showing the L1→L5 transformation under each of the three deployment models, so readers can
see how the layered IR + topology-as-parameter design plays out concretely.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 27, 2026
…iguate joins (L2 review #1) The issue-#7 qualifier fix reached scalar predicates but not the grouping channels: `Aggregate.keys`, `TopK.by`, and `WindowFunc.partition_by` were plain `Vec<String>`, and `expr_to_group_name` discarded the relation qualifier, so `resolve_named_keys` resolved by first-match `column_id`. On a self-join, `GROUP BY a.k` and `GROUP BY b.k` both bound to column 0 → `b.k` silently grouped by `a.k` (wrong result). Same bug class #7 closed for predicates, still open for keys. Fix: the three group-key channels now carry `ColumnRef` (qualified-capable) like the scalar path, and resolve via `resolve_column_refs` → `column_id_qualified` with the bare-name fallback. `expr_to_group_ref` (SQL) preserves `col.relation`; PromQL emits `ColumnRef::Named` (labels have no qualifier). The Binder seeds the bare names; `resolve_named_keys` is removed (subsumed by `resolve_column_refs`). Confirmed by a self-join test: `GROUP BY b.service` → `Aggregate.by = [5]`, `GROUP BY a.service` → `[1]`. Full suite green (133), clippy + fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
zzylol
added a commit
that referenced
this pull request
Aug 24, 2026
Restructure the flat 19-section developer guide into the three-part structure the doc owner asked for: Part 1 - Code Architecture, Part 2 - Interfaces and Definitions, Part 3 - How to Add X, Y, Z (each ending in how to verify). Content is moved, not rewritten: Part 1 (Mental model first, per doc-owner follow-up, then a new whole-PR architecture diagram, then "How the current pieces fit together"): - old #1 Mental model -> Part 1 #1 - new: whole-PR architecture diagram (TargetSubDAG's two entry points through ReplacementStrategy, PlanSpace/cost_sorted, explanation.rs, to a downstream consumer) -> Part 1 #2 - old #3 How the current pieces fit together -> Part 1 #3 Part 2: - old Terminology's "Implementation" definition merged into the Glossary as one more entry (### Implementation), next to ReplacementStrategy - old #2 Glossary -> Part 2 #1 (plus the merged Implementation entry and old #10 Matcher, retitled to match glossary-entry style) - old #10 Matcher (implementation.rs) -> ### Matcher inside the Glossary; implementation.rs no longer exists, so the stale title is fixed - old #19's definitional content (ReplacementExplanation/ ExplanationKind shapes, node_hash, why there's no ExplanationRule trait, location-text ownership) -> Part 2 #2 Part 3: - old #4, #5, #6, #7, #13, #14 -> Part 3 #1, Adding a new ReplacementStrategy (ending in Testing a new strategy) - old #8, #9, #15 -> Part 3 #2, Adding or customizing a CostModel (ending in Testing a new cost model) - old #12 -> Part 3 #3, Adding a new sketch algorithm, with its stale implementation.rs/binder references fixed to replacement.rs/ construct_summary vocabulary, plus a new "Verifying a new sketch algorithm" close grounded in the existing coverage-matrix tests - old #11, #16, #17, #18 -> Part 3 #4-#7 (capstone + closing reference material); #18's extension-map table's implementation.rs row fixed to replacement.rs - old #19's "Using it"/"Adding a new kind" content -> Part 3 #8, Using and extending explanation.rs cargo build --workspace --all-targets is clean (docs-only change). 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
Address review feedback on the L3 IR (
QueryExpr+AggIntent) and add the missing input/output, schema, and system-I/O specs.L3 deduplicated and made language-orthogonal
TopK(=Sort + Limit),WindowedAgg(=Window+Aggregate),Dedup(renamed toDistinct{cols})SketchAgg,JoinSketchnow live in a new L4 IRSketchExprHistogramQuantile,PromQLSubquerylower in PromQL L1→L2 insteadAggIntentlosesTopKandQuantileOverTime(surroundingWindowcovers the latter)New L4 sketch-bound IR
SketchExprSketchAgg,SketchJoin,SketchSubtract,SketchDelete,SketchEstimate,SketchMerge+Logical(QueryExpr)pass-throughfn apply(&QueryExpr) -> Option<SketchExpr>— the L3/L4 boundary is type-enforcedPer-node input/output specs
Schema/Field/HasSchematypes — every L3 edge carries a typed schemaQueryExprvariantsSketch catalog metadata
SketchEntryexpanded withmergeable,deletable,subtractable,accuracy,aggregated_keys,param_ranges,cost_model,supported_intentsSystem I/O contract
QueryWorkload, output is per-executor sub-DAG assignments§12 Open questions
Test plan
TopK,WindowedAgg,JoinSketch,HistogramQuantile,PromQLSubquery,Dedup,QuantileOverTime) outside the cleanup-rationale tables — verified via grep before push🤖 Generated with Claude Code