Skip to content

refactor(ir): remove legacy scaffolding and finish tooling migration - #543

Open
zzylol wants to merge 1 commit into
stack/528-07-plannerfrom
stack/528-08-cleanup
Open

zzylol wants to merge 1 commit into
stack/528-07-plannerfrom
stack/528-08-cleanup

Conversation

@zzylol

@zzylol zzylol commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem: the old two-IR representation is still on disk and in the tools after the #511 migration

#511 replaces the separate pre-ASAP and post-ASAP representations with one operator DAG. operator-sharing.md §Goal and problem: "Today, the post-ASAP representation wraps relational subplans and duplicates some relational operators outside those wrappers." Its §3 Acceptance criteria require that "Scalar expressions and conversions use the same representation before and after optimization, with no bridge nodes or hidden subplans". decoupling_op_and_expr.md §4 Acceptance and scope requires plans with no "PromqlScalarBridge or equivalent constant-wrapper node", and says implementation "will require frontend, validation and plan-format migration".

PRs 1–7 of this stack moved production code to the shared ir::OperatorNode with Operator::NonASAP(NonASAPOp) and Operator::ASAP(ASAPOp). Before this PR, three things were still left over:

  1. Legacy and transitional source files on disk. These files were not declared by any mod and were not compiled, but they still defined the old IR and parallel implementations:

    Left-over file(s) What it defined Canonical replacement at this PR
    types/src/pre_asap/query_expr.rs QueryExpr, "the canonical pre-ASAP intent algebra IR" ir::OperatorNode + NonASAPOp + ScalarExpr
    types/src/post_asap/expr.rs SummaryExpr (incl. KeepPreAsap), SummaryNode, ValueOperation ir::ASAPOp inside ir::OperatorNode
    types/src/post_asap/post_asap_dag.rs an older PostAsapDAG contract ir::export::PostAsapDAG (wire version 7)
    types/src/pre_asap/cse.rs, types/src/post_asap/cse.rs separate pre/post CSE (share_common_summary_sub_dags) ir::cse::share_common_sub_dags
    types/src/pre_asap/{canonicalize,resolve,schema_resolver}.rs, types/src/ir/schema_support.rs QueryExpr canonicalization and resolution ir::canonicalize, frontend-common::{resolve, schema_resolver}, pre_asap::schema
    asap-physical-operators/src/unified_physical_planner/*, unified_sources/*, expressions/unified_planner.rs, readout.rs temporary parallel physical planner, sources, expressions, readouts physical_planner/*, sources/*, expressions/planner.rs, evaluation.rs / summary_kernels/exact.rs
    frontend-{promql,sql,metricsql}/src/unified/* temporary parallel frontends frontend-promql/src/promql.rs, frontend-sql/src/sql/, frontend-metricsql/src/lib.rs
  2. The DAG viewer still spoke the old vocabulary. tools/dag-viewer/node-style.js categorized kinds the exporter can no longer emit, and missed kinds it does emit:

    In the viewer, not exported any more Exported, missing from the viewer
    KeepPreAsap, PromqlScalarBridge, EvalTimestamp, CurrentTimestamp, PromqlScalarFromVector, RelationalJoin, SummaryBinaryOp, ValueOperation Values, FinalizeExactAccumulator, MaintainPopulation, EvaluatePopulation, Extension

    The fixtures still showed a summary input as a wrapped sub-DAG:

    Before (fixture)                                After
    KeepPreAsap(Project)  { pre_asap_sub_dag }      Project(2 cols)
    └─ SummaryAgg(Kll)                              └─ SummaryAgg(Kll)
       └─ KeepPreAsap(Scan) { pre_asap_sub_dag }       └─ Scan(netflow_table)
    
  3. Docs described the old representation. Architecture and developer guides still used QueryExpr, SummaryExpr, KeepPreAsap, SketchAlgorithmStrategy and "readout".

Scope. This PR covers the last migration step of #511: delete the leftover definitions, move the viewer to Operator::kind_name, and update docs. Cleanup criterion: remove KeepPreASAP and the previous separate, non-shared logical DAG/node definitions. The logical compiler must use the shared ir::OperatorNode with NonASAPOp and ASAPOp; the physical-planner crate must not define a parallel logical planning-node IR. Remove the temporary parallel physical-planner implementation once it has been promoted into the canonical planner, and obsolete physical planning IR node definitions. It keeps the runtime PhysicalDAG and executable physical operators, which are needed to run a plan. The versioned ir::export::PostAsapDAG structs are serialization DTOs at the boundary, not a second in-memory planner IR; dag_export node structs are viewer/export formats. No production behavior changes.

Proposed method

  1. Delete every file in the table above (about 20,000 lines). None of them was in a module tree, so the compiled crates do not change. After this PR, the only Rust references to KeepPreAsap are a code comment in replacement.rs and the negative assertion in operator_design_examples.rs.
  2. Viewer. node-style.js maps exactly the 30 Operator::kind_name() values to color categories. The KeepPreAsap style override and legend row in viewer.js are removed. Fixtures and dag.example.json use flat ordinary nodes (Scan, Project) where they used KeepPreAsap(...), and ASAPStrategies where they used SketchAlgorithmStrategy. The README states that kind is Operator::kind_name, that scalar expressions are not nodes, and that an operator read by a scalar expression is a child shown as {"scalar_ref": <node id>}.
  3. Contract test. New crates/devtools/tests/viewer_contract.rs reads KIND_CATEGORY_JSON out of node-style.js and checks that its keys equal the set of exported kinds. Exhaustive matches over NonASAPOp and ASAPOp make a new operator variant a compile error until the kind list is updated.
  4. Docs. Architecture, concept and developer docs describe the unified node, schema, scalar expressions, execution timing and the exported DAG, and use "evaluation" for summary readouts. The merged docs: define unified operators and SQL/PromQL scalar boundaries #511 doc operator-sharing.md now names SchemaDerivationError instead of QueryExprError. New docs/develop_docs/operator-design-acceptance.md maps the docs: define unified operators and SQL/PromQL scalar boundaries #511 acceptance criteria and comparison-table rows to the implementation and tests.
  5. Two small fixes: the scalar_type_rules module doc, and the test name scalar_type_ruless_fail_closed → scalar_type_rules_fail_closed.

Pipeline position: none. This touches IR definitions, tooling and docs only.

Key code interfaces

This PR adds no public Rust API. It leaves these as the only planner IR (crates/types/src/ir/node.rs):

pub enum OperatorResultKind {
    Relation,
    InstantVector,
    RangeVector,
    State,
}

pub enum Operator {
    NonASAP(NonASAPOp),
    ASAP(ASAPOp),
}

impl Operator {
    pub fn kind_name(&self) -> &'static str; // NonASAPOp::kind_name / ASAPOp::kind_name
}

pub struct OperatorNode {
    pub operator: Operator,
    pub result_kind: OperatorResultKind,
    pub schema: Schema,
    pub guarantee: Option<ResultGuarantee>,
    pub timing: Option<ExecutionTiming>,
}

The viewer reads the export node (crates/types/src/dag_export.rs, unchanged here). Its kind is Operator::kind_name():

pub struct DAGNode {
    pub id: u32,
    pub kind: &'static str,
    pub label: String,
    pub detail: serde_json::Value,
    pub schema: Option<serde_json::Value>,
    pub children: Vec<u32>,
    pub workload_node_id: Option<u32>,
    // ... structural hash and cost annotations, unchanged
}

New contract test (crates/devtools/tests/viewer_contract.rs):

const NON_ASAP_KINDS: &[&str] = &["Scan", "Values", /* ... 20 total */];
const ASAP_KINDS: &[&str] = &["SummaryAgg", "SummaryEstimate", /* ... 10 total */];

#[allow(dead_code)]
fn kind_lists_track_every_variant(non_asap: &NonASAPOp, asap: &ASAPOp); // exhaustive matches
fn viewer_kind_categories() -> BTreeMap<String, String>;               // parses node-style.js

#[test]
fn viewer_categorizes_exactly_the_exported_node_kinds();

Viewer table (tools/dag-viewer/node-style.js):

const KIND_CATEGORY_JSON = `{ "Scan": "data", "Values": "data", "Filter": "filter", ... }`;

Deleted, by name: QueryExpr, SummaryExpr, SummaryNode, ValueOperation, the old post_asap::PostAsapDAG, share_common_summary_sub_dags, with_promql_series_identity (old copy), unified_physical_planner::{compile, bind, bind_with_data_sources, compile_node}, unified_sources::{DataSources, Scan}, CompiledExpression (old copy), readout::{exact_readout, insufficient_counter_samples} (old copies), and the unified frontends.

Fields

OperatorNode (unchanged; listed because it is now the only node type):

Field Type Meaning Set by
operator Operator The operation: NonASAP(NonASAPOp) for an ordinary query operator, ASAP(ASAPOp) for a summary operator. Inputs are Rc<OperatorNode> inside the payload. Frontends (NonASAP only), Pass 1/Pass 2 rewrites (ASAP).
result_kind OperatorResultKind Output category, derived from the operator and its inputs. Relation, InstantVector, RangeVector, or State (unfinalized summary or accumulator state). OperatorNode::new / with_schema.
schema Schema Output schema, derived at construction (with_schema may supply names and qualifiers). Construction.
guarantee Option<ResultGuarantee> Established accuracy guarantee. None means unassessed or unknown, never exact. Accuracy assessment.
timing Option<ExecutionTiming> IngestionTime or QueryTime once lifecycle assignment runs. None in logical plans; export rejects an executable node without it. Lifecycle assignment.

Operator::kind_name() returns the variant name. It is the kind string the exporter writes and the viewer looks up.

DAGNode fields read by the viewer:

Field Meaning
id Node id within the exported DAG.
kind Operator::kind_name(), for example "Aggregate" or "SummaryAgg". Key into KIND_CATEGORY_JSON.
label Short collapsed label, for example Scan(netflow_table).
detail The node's own scalar fields (predicates, measures, sort keys, family, ...), not its children. A scalar reference to an operator appears as {"scalar_ref": <node id>}.
schema OperatorNode::schema as JSON, entries under fields.
children Child ids in OperatorNode::children order: operator inputs, then nodes referenced from scalar expressions.
workload_node_id Workload-wide identity set by the exporter; the viewer unions nodes by it.

Contract test items:

Item Meaning
NON_ASAP_KINDS The 20 NonASAPOp::kind_name() values.
ASAP_KINDS The 10 ASAPOp::kind_name() values.
kind_lists_track_every_variant Never called. Its exhaustive match on every variant stops compiling when a variant is added, until the lists are updated.
viewer_kind_categories Extracts the template literal after const KIND_CATEGORY_JSON = \`` from node-style.js` and parses it as JSON, as the viewer does.
viewer_categorizes_exactly_the_exported_node_kinds Asserts the two lists have no duplicates and that the viewer's key set equals their union.

KIND_CATEGORY_JSON (kind → category) after this PR:

Category Kinds
data Scan, Values
filter Filter
sample PromqlSeriesSample
derive Project, PromqlRelabel, PromqlInfoEnrich, PromqlVectorFromScalar, BinaryOp
aggregate Aggregate
window TimeRange, PromqlSubquery, TimeShift, SQLWindowFunc
join Join
set Dedup, SetOp
combine Concat
sort Sort, Limit
summary SummaryAgg, SummaryEstimate, SummaryMerge, SummarySubtract, SummaryDelete, SummaryJoin, FinalizeExactAccumulator, MaintainPopulation, EvaluatePopulation, Extension

Examples

One summary plan, before and after

tools/dag-viewer/post_asap_fixture.json, KLL over netflow_table:

Before                                                After
id 0  KeepPreAsap  "KeepPreAsap(Scan)"                 id 0  Scan        "Scan(netflow_table)"
      detail.pre_asap_sub_dag = { nodes: [Scan], ... }
id 1  SummaryAgg   "SummaryAgg(Kll)"   children [0]     id 1  SummaryAgg  "SummaryAgg(Kll)"   children [0]
id 2  KeepPreAsap  "KeepPreAsap(Project)"  children [1] id 2  Project     "Project(2 cols)"   children [1]
      detail.pre_asap_sub_dag = { nodes: [Project], ... }

Every operator is one node with its own kind. The scan is visible, not hidden in a wrapper. This matches the "Proposed" side of the #511 §Goal and problem figure, where the scan under a KLL build is directly visible.

What the contract test catches

Change Result
Viewer table equals the 30 exported kinds passes
Viewer keeps a removed kind, for example KeepPreAsap fails: extra key
Viewer misses an exported kind, for example Values or EvaluatePopulation fails: missing key
A new NonASAPOp or ASAPOp variant without a list entry does not compile (kind_lists_track_every_variant)
node-style.js without KIND_CATEGORY_JSON, or invalid JSON fails with a named expect message

Out of scope

Stack and validation

Stack 8/8 of the #528 split · Base: #542 · Next: #551 · Reference/tracker: #528.

The stack is rebased onto main at 7734c68f, keeping #544 DAG naming and the later #535 schema compatibility/naming fixes, so it includes changes beyond the original #528 snapshot. This is a review split of that implementation. #532 is already in the main-branch base.

Validation: full workspace tests/doctests (1,590 passed); formatting; workspace all-target/all-feature Clippy with warnings denied; viewer tests (25 passed, 6 Node-dependent tests skipped because Node is unavailable). All eight rebased layers pass workspace/all-target/all-feature Clippy; the final stack passes the full workspace tests.

🤖 Generated with Claude Code

@zzylol
zzylol force-pushed the stack/528-08-cleanup branch from 6c2c313 to 2bd4c4d Compare October 2, 2026 18:32
@zzylol
zzylol force-pushed the stack/528-07-planner branch 2 times, most recently from 2a3bcd0 to a03efc2 Compare October 2, 2026 19:40
@zzylol
zzylol force-pushed the stack/528-08-cleanup branch 2 times, most recently from 78b6dc3 to e40ffdb Compare October 2, 2026 21:14
@zzylol
zzylol force-pushed the stack/528-07-planner branch from a03efc2 to 2c708f3 Compare October 2, 2026 21:14
@zzylol
zzylol force-pushed the stack/528-08-cleanup branch from e40ffdb to 8b2dfff Compare October 2, 2026 21:22
@zzylol
zzylol force-pushed the stack/528-07-planner branch 2 times, most recently from 03166e7 to 4b84314 Compare October 2, 2026 21:25
@zzylol
zzylol force-pushed the stack/528-08-cleanup branch from 8b2dfff to 5f9d447 Compare October 2, 2026 21:25
@zzylol
zzylol force-pushed the stack/528-07-planner branch from 4b84314 to d9da0f9 Compare October 2, 2026 21:56
@zzylol
zzylol force-pushed the stack/528-08-cleanup branch from 5f9d447 to cc36ea0 Compare October 2, 2026 21:56
@zzylol
zzylol force-pushed the stack/528-07-planner branch from d9da0f9 to ec9f8cb Compare October 3, 2026 02:31
@zzylol
zzylol force-pushed the stack/528-08-cleanup branch from cc36ea0 to fe7d713 Compare October 3, 2026 02:31
@zzylol

zzylol commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: the module/crate reorganization by #509 stages and the #511 IR (removing pre_asap / post_asap and splitting asap-aware-mapping) is tracked in #572 and starts after this cleanup lands.

🤖 Generated with Claude Code

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