refactor!: rename planner candidate types to the #509 stage names - #514
Merged
Merged
Conversation
- PlanSpace -> CandidateLogicalASAPDAGs (stage 1 output) - PhysicalCandidate -> PhysicalASAPDAG (element of the stage 2 candidate set; one is chosen by selection) - UncheckedCandidate -> UncheckedPhysicalASAPDAG (private serde helper) Identifiers embedding these names, doc comments and docs/ follow. No behavior or serialized-output change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol
force-pushed
the
refactor/509-names
branch
from
October 2, 2026 00:30
530ec7d to
d8ecc9f
Compare
This was referenced Oct 2, 2026
zzylol
added a commit
that referenced
this pull request
Oct 2, 2026
Resolve the conflict in docs/design_docs/proposals/operator-sharing.md by keeping this PR's version. Since this branch was cut, main changed two lines in sections that this PR rewrites: - the MaintainPopulation/ReadPopulation timing row (lifecycle-set population timing, from the lifecycle stack), and - PlanSpace -> CandidateLogicalASAPDAGs (#514). Neither line exists in this PR's rewritten §2, which already assigns timing to physical planning under #509. 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
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.
Why
#509 names the planner stages' outputs. This aligns the code's two unambiguous types with those names.
What
PlanSpaceCandidateLogicalASAPDAGsPhysicalCandidatePhysicalASAPDAGUncheckedCandidate(private serde helper)UncheckedPhysicalASAPDAGfindings_from_plan_spacefindings_from_candidate_logical_asap_dagsAll references, intra-doc links and
docs/diagrams are updated too. This is a mechanical rename with no behavior change. Serialized output is unchanged, because serde does not write struct names and the field names stay the same. One error message's text now saysCandidateLogicalASAPDAGs::roots.Known gaps
PhysicalASAPDAGis still compiled from onePostAsapDag, so it covers a single root. Merging per-root plans into one workload DAG is a follow-up.PostAsapDag, because its nodes still carry ingestion/query timing, which docs: propose workload-wide planning, summary sharing, and materialization #509 assigns to physical planning; it will be revisited when that timing moves out. AlsoCandidateDagInventory,CompiledPhysicalDag,SummaryMaintenanceLifecycleCandidatesandQueryExpr, whose correspondence to a docs: propose workload-wide planning, summary sharing, and materialization #509 name is ambiguous.Validation
Passes on Rust 1.99:
cargo fmt --check, clippy with-D warnings, and the MetricsQL baseline. On the pre-rebase base, the full workspace test run passed 1492 tests. On the current base it is running locally and in CI.🤖 Generated with Claude Code