Skip to content

docs: QuerySpec definition + batched-queries DAG-reuse example - #2

Closed
zzylol wants to merge 2 commits into
mainfrom
docs/workload-plan-and-queryspec
Closed

docs: QuerySpec definition + batched-queries DAG-reuse example#2
zzylol wants to merge 2 commits into
mainfrom
docs/workload-plan-and-queryspec

Conversation

@zzylol

@zzylol zzylol commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Expand the QuerySpec placeholder in core::workload into a full struct (id, query+language, time_range, accuracy/latency/dollars SLAs, optional deployment_model hint) plus supporting QueryLanguage and AccuracyTarget enums.
  • Add a second end-to-end example tracing three related PromQL queries through L3 (workload-level CSE producing fan-in), L4 (MergeKllSketches collapsing q1+q2 onto one KLL state, q3 stays exact), and L5 (colored multi-root DAG emission with edge-side memory dropping from 3× to 1× of scan/window).
  • Introduce WorkloadPlan { bindings, roots } as the workload-level multi-root container so QueryExpr stays single-root and cross-query fan-in lives one level above per-query IR. Within-query CTE fan-in keeps using the existing LetBinding/Ref mechanism unchanged.

Test plan

  • Render docs/design.md and confirm both new sub-sections read cleanly
  • Cross-check QuerySpec fields against existing references throughout the doc (HTTP POST /plan, OpAMP capability-miss callback, replan cycles)
  • Confirm the batched example's IR shapes are consistent with §6 design rule 5 (DAG fan-in) and §6 core::cost (workload_cost / ReusedComponent)

🤖 Generated with Claude Code

@zzylol @claude
docs: add QuerySpec definition and batched-queries DAG-reuse example
17604cf 
Expand the QuerySpec placeholder in core::workload into a full struct
(id, query+language, time_range, accuracy/latency/dollars SLAs, optional
deployment_model routing hint), plus supporting QueryLanguage and
AccuracyTarget enums.

Add a second end-to-end example walking three related PromQL queries
through L3 (workload-level CSE producing fan-in), L4 (MergeKllSketches
collapsing two quantile sketches onto one KLL state), and L5 (colored
multi-root DAG emission). Introduce WorkloadPlan { bindings, roots } as
the workload-level multi-root container so QueryExpr stays single-root
and cross-query fan-in lives one level above per-query IR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol marked this pull request as draft May 5, 2026 21:35
@zzylol @claude
docs: add QueryShape and DataShape to QuerySpec
559968e 
Add `shape: QueryShape` (OneShot / Streaming / Periodic{every}) for how
a query is evaluated, and `data: DataShape` (Batch / AppendOnlyStream /
Mutable / Mixed) for the data feeding it. Document the planner's
acceptance matrix across the cross-product (e.g. Streaming over Batch
is rejected at L1; Mutable disables sketch binding pending
retraction-aware operators).

Note that per-leaf data shape rides on Source at L3; the QuerySpec
field is a workload-level summary, with Mixed deferring to per-leaf
detail during L4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@milindsrivastava1997 milindsrivastava1997 mentioned this pull request May 19, 2026
Merged
zzylol added a commit that referenced this pull request May 27, 2026
@zzylol @claude
test: add regression tests for the review fixes (#7 qualifiers, unary, …
e0e1b47 
…#2, #3)

Locks behavior introduced by the recent fixes, +5 tests (125→130):

- promql_conformance: nested unary negation propagates rejection
  (`a - -b`, `sum(-x)`), not just top-level; and count→Cardinality threads
  the AccuracyTarget (Exact stays exact, Epsilon carried) — pins review #2.
- sql_lowering: a qualified WHERE on the *duplicated* join column
  (`WHERE hosts.service = ...`) binds to the qualified position (4), not the
  first `service` (1) — extends the #7 disambiguation past the join key.
- error.rs: `UnsupportedFeature` Display is language-neutral (no "PromQL") —
  pins the #3 fix without depending on DataFusion plan shapes.
- schema.rs: `Column.table` deserializes to `None` when the key is absent
  (the `#[serde(default)]` backward-compat contract), and a qualified column
  round-trips.

Full workspace green (130 tests), clippy -D warnings + fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 27, 2026
@zzylol @claude
refactor(core): unify L2Expr/L3Expr into one generic Expr<C> (L2 review
a7f9c83 
#2, scalar)

The two scalar IRs were byte-identical 13-variant enums differing only in the
column-reference type, with duplicated `conjuncts`/`disjuncts`/`columns_referenced`
— maintained twice. Collapse them into a single `Expr<C>`:

- `type L2Expr = Expr<ColumnRef>` (name-based, front-end-emitted)
- `type L3Expr = Expr<ColumnId>`  (positional, resolved)

The helpers are now one generic `impl<C> Expr<C>` (`columns_referenced` returns
`Vec<&C>`, unifying the L2 `&ColumnRef` and L3 owned-`ColumnId` variants — the
sole caller is the L2 Binder). Because the aliases preserve variant
construction/pattern syntax (`L2Expr::Compare { .. }` etc.), every front end,
the converter's `resolve_expr` map, and all tests compile unchanged.

This is the clean, low-risk half of the QueryExpr<C> parameterization the review
recommended starting with. No behavior change. Full suite green (133), clippy +
fmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol zzylol mentioned this pull request May 27, 2026
Merged
3 tasks
@zzylol zzylol mentioned this pull request Jul 10, 2026
Open
@zzylol zzylol mentioned this pull request Jul 21, 2026
Merged
zzylol added a commit that referenced this pull request Jul 22, 2026
@zzylol
docs: fill L4 TODO, split Bind/canonicalize between L2/L3, add per-la…
8e2e189 
…yer diagrams

- Filled in the "implementation refers to [TODO]" placeholder in L4.
- Moved Bind (name resolution) into L2's own bullets and canonicalize
  (cross-language shape unification) into L3's own bullets, instead of
  a separate transition section — Bind is why L2 stays "per-language"
  even though both front ends share one Rust type; canonicalize is
  where cross-language convergence actually happens (issue #34).
- Added a Mermaid diagram to each L1-L5 section showing its actual
  data flow, verified against the real code paths.
- Fixed the Glossary's "Bind" lookup table to point at sections that
  still exist, and marked Implementation/implement_tree/Match/Bind #2
  as currently undocumented in this file (their old transition-section
  homes were removed) rather than leaving dangling links.
@milindsrivastava1997

milindsrivastava1997 commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Closing as stale.

zzylol added a commit that referenced this pull request Aug 24, 2026
@zzylol @claude
refactor(asap-aware-mapping): delete bind.rs, retire "select_and_bind" (
51ae6e4 
#251, part of #33)

## Why

`asap-aware-mapping`'s output boundary is `PlanSpace` — every candidate
`TargetSubDAG` replacement plus its cost, meant for a downstream
DAG-visualization consumer. It does not own "commit to one final,
physically-materialized, shared post-ASAP DAG for a whole workload" —
that's a downstream deployment's job (picking *which* sketch and *where*
to place it are a joint decision only a deployment can see the full
picture for; this crate pre-deciding one in isolation, with no real
consumer of that single materialized answer inside this crate, is out
of scope). `bind.rs`'s `implement_workload`/`implement_workload_with`
(keep first/cost-preferred candidate per node, memoized by `Rc` identity
across roots so pre-ASAP CSE sharing survives into a single physical
binding) was exactly that out-of-scope "commit to one answer" step, so
it's deleted.

## What

1. Deleted `bind.rs` entirely (no replacement file, no rename). Removed
   `pub mod bind;` and its re-exports from `lib.rs`. `ImplementError`
   moved into `replacement.rs` (`pub`, since it's part of what a
   `ReplacementStrategy` implementor's construction path can realistically
   fail with) — still needed by `realize_child`/`keep_pre_asap`/the
   child-recursion path. `bind.rs`'s own test suite (16 tests exercising
   `select_and_bind`'s/`construct_summary_agg`'s schema-derivation
   behavior across various `AggIntent` shapes) moved into
   `replacement.rs`'s existing test module, adapted to call
   `realize_child` directly — this crate's own production logic, not
   `bind.rs`'s deleted orchestration. The one test specific to
   `implement_workload_with`'s CSE-decline memoization behavior has no
   equivalent left to test and was dropped (that comparison is already
   covered by `cost_sorted_orders_shared_subtree_candidates_by_cse_share_decision`
   and `cost_model.rs`'s own `cse_share_decision` tests).

   Rewrote `lib.rs`'s `## Status` section: two capabilities —
   `replacement::ReplacementStrategy` (per-target, exhaustive) and
   `replacement::search_workload`/`PlanSpace::cost_sorted` (workload-wide,
   every candidate + cost) — plus an explicit statement that picking/
   materializing one final answer is a downstream deployment's job.
   Rewrote the `## Terminology` table: dropped the `implement_workload`
   row; the `Bind #2` (downstream placement) row now also covers "which
   candidate to commit to", since that's where the deleted step's
   behavior would live if a deployment still wants it.

2. Renamed the `pub(crate)` single-target rank-and-take-first helper
   `select_and_bind` → `realize_child` (word "bind" is retired from this
   crate's vocabulary — see `lib.rs`'s Terminology section). Also renamed
   its sibling `bind_one` (the representative-node helper `cse_preference`
   uses for a `CostModel::cse_share_decision` comparison) → `realize_one`
   for the same reason. Updated every call site and doc comment; behavior
   unchanged (rank via `SketchFamilyStrategy`, take first, `Rewrite` is
   unreachable, `None` falls back to `keep_pre_asap`).

3. Renamed "site" → "target"/`TargetSubDAG` throughout the merged
   search-engine content in `replacement.rs`: `discover_sites` →
   `discover_targets`, `discover_new_descendant_sites` →
   `discover_new_descendant_targets`, local variables/doc prose
   (`MemoGroup`'s "one distinct site", the "Where 'for site in
   plan.bindable_sites()' comes from" section heading, `PlanSpace`/
   dedup/termination prose). The quoted historical pseudocode block
   itself (`for site in plan.bindable_sites()`) stays verbatim, framed
   explicitly as a quote. (This item was landed independently by a
   concurrent session as commit c562f3c while this work was in
   progress on the same shared branch — reconciled here via
   `git reset --hard origin/...` + manual reapply per the shared-branch
   protocol; this commit only carries items 1/2/4/5 on top of it.)

4. Rewrote `crates/integration-tests/tests/cse.rs`: it called the now-
   deleted `implement_workload` and asserted two bound `SummaryNode`s were
   `Rc::ptr_eq` — a workload-level "one materialized answer" claim that no
   longer has an API to make. Rewrote its three tests to call
   `replacement::search_workload` and assert on the resulting `PlanSpace`
   instead: the shared subtree collapses onto one `MemoGroup`/one shared
   `Rc<QueryExpr>`, mirroring `replacement.rs`'s own
   `shared_aggregate_across_two_roots_gets_both_strategies_candidates`
   test through the crate's public API.

5. Fixed every other `asap_aware_mapping::bind::*` import (devtools'
   `show_post_asap_ir`, `frontend-promql`'s corpus test,
   `integration-tests`' `l4_binding`/`l4_binding_sql`) to
   `asap_aware_mapping::replacement::*`, and updated stale doc references
   in `asap-types::pre_asap::cse`/`mod.rs` that pointed at
   `asap_aware_mapping::implement_workload`/`bind::implement_workload_with`.

## Verification

$ cargo build --workspace --all-targets && cargo test --workspace \
    && cargo fmt --all -- --check \
    && cargo clippy --workspace --all-targets --all-features -- -D warnings
(all clean; asap-aware-mapping: 68 passed, 0 failed; cse.rs: 3 passed)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Aug 24, 2026
@zzylol @claude
docs(asap-aware-mapping): reorganize developer guide into three parts
848c38f 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zzylol @milindsrivastava1997