Skip to content

refactor!: audit and prune redundant planning enums - #447

Open
zzylol wants to merge 1 commit into
mainfrom
fix/428-prune-planning-enums
Open

zzylol wants to merge 1 commit into
mainfrom
fix/428-prune-planning-enums

Conversation

@zzylol

@zzylol zzylol commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

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

  • Remove ExecutableOperator, ExecutableDagNode.operator, MaterializationMedium, and SummaryWindowFramework::Extension(String).
  • Re-export the shared CostUnit from mapping; recurring formulas reject total-cost units.
  • Retain BoundaryKind, CacheProfile, EvaluationSchedule, QueryTimeScope, Predictability, and ExponentialHistogram because they drive estimation, lifecycle selection, or guarantee validation.

How

Use ExecutableOperatorPayload as 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" and payload.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 Rust CostUnit types.

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 reject CostUnits instead 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

  • Unit and end-to-end tests: cargo test --workspace --locked — 1,165 passed.
  • Formatting: cargo fmt --all -- --check — passed.
  • Lint: cargo clippy --workspace --all-targets --all-features --locked -- -D warnings — passed.
  • Viewer: 24 passed, 6 skipped.
  • MetricsQL external-consumer check and vendored-parser baseline verification — 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

  • The MVP boundary is correct.
  • New conceptual layers or public interfaces are necessary.
  • The before/after description matches the intended product behavior.
  • Human reviewer:
  • Decision and rationale:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor] Audit and prune redundant planning enums

1 participant