Repository navigation
Conversation
3 tasks
zzylol
force-pushed
the
stack/528-07b-promote-roots
branch
from
October 12, 2026 20:30
7dc95a1 to
26b7ab0
Compare
zzylol
force-pushed
the
stack/528-07c-planner-followups
branch
from
October 12, 2026 20:30
3e0ec28 to
fce55da
Compare
3 tasks
PlanOutput::execution_timed_dag times every query root, and every operator reference of a standalone scalar root, with one shared TimingMemo and compiles them with compile_physical_asap_workload, so a batch is one DAG with a root per operator query, a scalar root per standalone scalar query (keyed by entry) and shared sub-DAGs exported once. Lifecycle phases come from the deployments of every plan, including the plans of scalar-read sub-DAGs. The per-plan SummaryMaintenanceLifecyclePlan::execution_timed_dag is the batch of one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
Lower each example query of the ASAP primitive schema design doc (#573 §4.2.2), plan it with the default cost model and check the coverage of the selected KLL state: SQL value sets, ranges and renamed columns, a PromQL offset window as relative time, a matcher below rate kept in the definition, and a PromQL comparison above rate lifted into the selection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
…initions `execution_timed_workload_dag` becomes `pub(crate)`: callers export a planned workload through `PlanOutput::execution_timed_dag`, which also carries its standalone scalar roots. The coverage tests now check what each #573 §4.2.2 example's definition keeps, not only its selection: the range example yields exactly one box and a scan without the lifted predicate, the matcher below `rate` stays in the definition's scan, and the definition keeps the `rate` under a lifted `Filter`. The two missing rows are added: `rate(a[5m]) > rate(b[5m])` (a `BinaryOp`, nothing selected) and `rate(m[5m]) > bool 0` (a `Project`, kept in the definition). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
The #573 coverage tests no longer depend on which candidate the default cost model selects: each takes the one KLL SummaryAgg among the search space's candidates. The SQL tests also check that the definition's SummaryAgg has no filter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
…rkloads Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
zzylol
force-pushed
the
stack/528-07b-promote-roots
branch
from
October 12, 2026 22:55
26b7ab0 to
8aba6bc
Compare
zzylol
force-pushed
the
stack/528-07c-planner-followups
branch
from
October 12, 2026 22:55
fce55da to
1e719c9
Compare
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.
Why
With the planner on the unified IR (#542), a planned workload can be exported for execution as a whole, and the summary coverage that #646 derives from the node can be checked end to end from real queries.
What
PlanOutput::execution_timed_dag()times every query root, and every operator sub-DAG a standalone scalar query reads (refactor(planner): plan the workload on the unified operator IR #542), with one sharedTimingMemo, and exports a singlePhysicalASAPDAG: one root per operator query, onescalar_rootsentry per scalar query, shared states once. Scalar roots are keyed by position in the batch, which equals the entry index becausecheck_contractrequires dense entry indices. An empty workload returnsPhysicalASAPDAGValidationError::NoRoots; a workload of only literal scalars yields a DAG with no nodes, just itsscalar_roots. Lifecycle phases come from the deployments of every plan, including the plans of scalar-read sub-DAGs. The lifecycle-based helper behind it (execution_timed_workload_dag) is crate-private; onlyexecution_timed_dagis public, and the per-planSummaryMaintenanceLifecyclePlan::execution_timed_dagis the batch of one.integration-tests/tests/summary_coverage_e2e.rslowers the docs: ASAP primitive schema and summary coverage (definition + selection) #573 §4.2.2 example queries, plans them, and checks both the definition and the selection ofcoverage()on the KLLSummaryAgg. Each test takes the one KLLSummaryAggamong the search space's candidates, so the tests do not depend on which candidateDefaultCostModelselects.How
execution_timed_dagreuses #541'scompile_physical_asap_workload_with_node_idsover the planned roots. The coverage tests readOperatorNode::coverage()on the KLL summary candidate; nothing is declared by the planner.Before this PR
Each planned query could be exported only on its own, and coverage was checked only on hand-built nodes (#646).
After this PR
A planned batch exports as one
PhysicalASAPDAG, and the coverage derivation is tested on SQL and PromQL queries through the planner.Evidence
From
summary_coverage_e2e.rs: SQLregion = 'us'→region ∈ {us};latency < 100→latency ∈ (−∞, 100); a renamed column (region AS r) →r ∈ {us}, withlatency * 2 > 10kept in the definition's scan; PromQLquantile_over_time(0.99, m[1m] offset 2m)→ relative time(−3m, −2m];quantile(0.99, rate(m{job="api"}[5m]))→ unrestricted selection, with the matcher kept in the definition (it is belowrate);quantile(0.99, rate(m[5m]) > 0)→value ∈ (0, ∞). Fromoperator_design_examples.rs: a two-query batch exports with two roots and one sharedSummaryAgg.Verification
summary_coverage_e2e.rs(above, pluspromql_vector_comparison_stays_in_definition—rate(a[5m]) > rate(b[5m])is aBinaryOp, nothing selected — andpromql_bool_comparison_stays_in_definition—> bool 0is aProject, kept in the definition; the range test checks exactly one box and that the scan predicate was removed; the SQL tests assert that the definition'sSummaryAgghas no filter);batch_planning_replaces_and_shares_summary_operators(one physical DAG, shared state once); the scalar workload test exports scalar roots that reference the shared summary.cargo fmt --check; workspace all-target Clippy with-D warnings;cargo test --workspace(1,722 passed, 0 failed, 2 ignored); dag-viewertest_render(31 passed);tools/verify_metricsql_external_consumer.shOK.Architectural decisions
Not applicable: both changes use existing interfaces.
Limitations and follow-up
Stack: tracker #528 · main → #542 → #665 → #666 → #543 → #667 → #561.
Human review — do not complete with an agent
🤖 Generated with Claude Code
https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW