Skip to content

Bind physical plans to stable post-ASAP node IDs - #564

Merged
zzylol merged 4 commits into
mainfrom
refactor/stable-post-asap-bindings
Sep 10, 2026
Merged

zzylol merged 4 commits into
mainfrom
refactor/stable-post-asap-bindings

Conversation

@zzylol

@zzylol zzylol commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Why

The physical compiler keyed selected materializations by an Rc memory address. That identity exists only inside one process and bypasses the stable PostAsapNodeId assigned by ASAPPlanner's executable DAG contract.

Before this PR

For a selected post-ASAP node, compilation inserted Rc::as_ptr(node) as usize into a global map and later repeated the pointer cast while lowering the QueryPlan. Raw-entity safety checks used the same address convention. The deployed materialization decision therefore had no explicit relationship to the Planner DAG node identity.

After this PR

Each query's selected post-ASAP graph is compiled once to ExecutableDagCompilation. Materialization placement, raw-entity admission, and QueryPlan binding use (query index, PostAsapNodeId) throughout. The QueryPlan binder receives the existing Rc<SummaryNode> handle only to resolve its Planner-assigned ID through ExecutableNodeIdentityMap; pointer-sized values are no longer stored or compared by backend compilation.

For example, two queries may each have node PostAsapNodeId(2) without colliding because the backend keys placement by query scope plus node ID. A shared node inside one query resolves to one stable ID and one physical binding.

This is the base migration surface for language-neutral frontends: ClickHouse and MetricsQL should lower their typed post-ASAP nodes into the existing unified QueryPlan, using PostAsapNodeId for placement. They should not add a language-specific plan catalog, executable-plan DTO, or sidecar DAG.

Verification

  • cargo check -p control_plane --all-targets — passed.
  • cargo test -p control_plane --lib planner_workload_tests — 5/5 passed, including all 24 o11y workload queries.
  • Static guard: physical/compiler.rs contains no Rc::as_ptr, node_identity, or HashMap<usize, ...> binding path.
  • git diff --check — passed.

Dependency

This PR is based directly on main. The catalog-first ownership changes will be stacked after it.

@zzylol
zzylol merged commit 7dadae2 into main Sep 10, 2026
1 check passed
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.

1 participant