Conversation
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
Closes #428. Remove redundant planning representations while retaining enums with implemented consumers. The audit covers every item in the issue: decisions and migration notes.
What
ExecutableOperator,ExecutableDagNode.operator,MaterializationMedium, andSummaryWindowFramework::Extension(String).CostUnitfrom mapping; recurring formulas reject total-cost units.BoundaryKind,CacheProfile,EvaluationSchedule,QueryTimeScope,Predictability, andExponentialHistogrambecause they drive estimation, lifecycle selection, or guarantee validation.How
Use
ExecutableOperatorPayloadas the sole operator identity. Keep network/materialization byte dimensions separate, without an unused storage-medium label. Limit window frameworks to those with defined planning semantics.Before this PR
Compiling a sum into an executable DAG emitted both
operator: "SummaryAgg"andpayload.kind: "summary_agg"; these could disagree. Declaring a materialization required a medium even though all media used the same byte counter and coefficient. Mapping and shared cost annotations used distinct RustCostUnittypes.After this PR
The same sum emits only
payload.kind: "summary_agg"in a version-2 executable DAG. A materialization is{"kind":"materialization"}and preserves its byte estimate. Mapping and annotations share one cost-unit type; recurring formulas rejectCostUnitsinstead of interpreting a total as a rate.Evidence
The executable-DAG round-trip test checks payload-only node identity, rejection of the removed field, and rejection of document version 1. Boundary integration coverage preserves distinct once-only materialization and per-consumer transfer multiplicities. The rate-unit regression failed before the guard was added.
Verification
cargo test --workspace --locked— 1,165 passed.cargo fmt --all -- --check— passed.cargo clippy --workspace --all-targets --all-features --locked -- -D warnings— passed.New tests verify that opaque window extensions cannot enter planning and that shared total-cost units cannot enter recurring formulas. Existing DAG and boundary tests verify the simplified wire contracts.
Architectural decisions
Keep enums with actual semantic consumers: for example, predictable advance notice enables prepared state, moving real-time lookbacks require deletion support, and exponential histograms require framework-specific accuracy evidence. Remove the opaque window extension because name validation alone supplies no framework semantics.
Limitations and follow-up
Breaking API/wire cleanup: regenerate version-1 executable DAG documents; consumers must match the payload, remove materialization media from profiles, and stop supplying window extensions. The local ASAPQuery backend still imports the old executable operator API and must migrate before upgrading its planner dependency; its only window-extension use is a test rejecting an unsupported layout. No materialization-medium consumer was found there. Backend changes are outside this PR.
Human review — do not complete with an agent