refactor: unify logical operators and scalar expressions by Selvomega · Pull Request #528 · ProjectASAP/ASAPPlanner

Selvomega · 2026-10-02T11:58:45Z

Revised priority (2026-10-03)

This PR stays open as the #511 reference implementation until Phase A merges, then closes.

Status (2026-10-03, evening): Phase A is rebuilt and pushed as drafts: #567 → #560 → #537 → #539 → #540 → #541 → #542 → #543. Each tip passes clippy and the workspace tests, independently re-run. The design doc split out of #567 is #573, against main. Phase C is up as drafts on #543: #561 (Pass 1) → #575 (Stage 1 workload candidates + stage_pipeline) → #576 (Stage 2/3) → #577 (Example 1 acceptance tests), plus the viewer #574. Phase B is complete as drafts: #578 (B1 types modules) → #579 (top-k readout consistency) → #581 (D1: stage-pipeline facade with tree-DP selection, MajorPass retired) → #582 (D2: Stage 1 without the cost model) → #583 (B2 logical-optimizer) → #584 (B3 physical-optimizer) → #585 (B4 plan-selection) → #586 (B4b facade in planner, asap-aware-mapping removed) → #587 (B5 executor). After Phase B: #588 (Pass 2 identical-expression rule; Example 1 drops from 79.4 to 52.2) → #589 (whole-expression top-k heap sketch over raw samples). The viewer #574 is rebased onto #587. Follow-up: #580 (remaining Pass 2 rules and coverage parity).

Phase A — finish #511 operator sharing

feat(ir): add summary coverage metadata for time and population #567 → feat(ir): define compatible logical summary merges #560 → feat(ir): add logical sub-DAG sharing and export without execution timing #537 → feat(sql): lower SQL to unified operator and scalar IR #539 → feat(promql): lower queries to unified operator and scalar IR #540 (revised chain)
→ feat(runtime): export timed plans as PhysicalASAPDAG and compile them #541 native runtime, rebuilt on the revised feat(promql): lower queries to unified operator and scalar IR #540
→ refactor(planner): plan the workload on the unified operator IR #542 planner cutover only: today's planner emits the unified DAG, with no new docs: propose workload-wide planning, summary sharing, and materialization #509 stage logic
→ refactor(ir): remove the legacy QueryExpr and SummaryExpr IRs; move docs and DAG viewer to the unified IR #543 legacy cleanup, rebuilt on the new refactor(planner): plan the workload on the unified operator IR #542

Phase B — #572 crate/module reorganization: five pure-move PRs (types modules → logical-optimizer → physical-optimizer → plan-selection → executor).

Phase C — #509 end to end: Example 1 through LogicalDAG → LogicalASAPDAG → PhysicalASAPDAG, with cost estimation, shown in a three-lane DAG viewer and checked by an e2e test. Example 4 (window materialization) comes next. Stage 1 starts from #561.

Parked: #552–#569 and #561 are now drafts, because they sit on the old stack/528-legacy-physical-base chain and Phase B moves their files. Their content is re-scoped in Phase C. #551 is closed because it conflicts with the #572 accuracy decision.

Implements the unified operator/scalar representation in #511 and resolves the main-branch merge.

Before this PR: ordinary and summary plans used separate node types and KeepPreAsap wrappers; scalar computation could become operator nodes. For example, scalar(sum(up)) + 1 required bridge nodes, and SQL scalar-subquery rewrites could lose empty-input/cardinality semantics.

After this PR: both logical stages share OperatorNode with Operator::{NonASAP, ASAP} payloads, common typed schemas and explicit scalar producer edges. ScalarExpr owns literals, arithmetic, evaluation context, conversions and subquery reads; scalar roots need no fabricated relation. KeepPreAsap is removed, and unchanged work is retained directly. Native compilation and flat wire export consume this representation.

PromQL math/date operations use scalar projections, including dynamic scalar parameters. Unary minus preserves the metric name; arithmetic/math operations remove it as required.
SQL coercions, nullable SUM/window results, scalar-subquery cardinality and NOT IN are preserved. Unsafe scalar-subquery cross joins and ROW_NUMBER rewrites are removed. Unsupported signatures/native histogram samples fail explicitly instead of receiving placeholder semantics.
Structural and phase-dependency validation covers real planner candidates. SQL/PromQL document examples now run through lowering/export or native execution, including SQL empty/all-NULL input and scalar(sum(up)) + 1.

The acceptance matrix maps all four requested criteria and the proposal's language comparison tables to implementation/tests. It records parser/runtime gaps and compatibility changes (notably rejected ClickHouse stub signatures, native histograms and older persisted SUM payloads). This implements the proposal's representation scope; it does not claim full current DataFusion/Prometheus execution support. Generalizing analytical evidence to at-rest workloads was split into and merged separately as #532; this branch incorporates the updated main. Physical placement policy remains #520/#530.

Review follow-up: one multi-root workload DAG is explicit in PlanOutput; replacements/CSE use sub-DAG terminology; shared operator properties, schema derivation and errors have separate modules. SketchStatistic and summary “evaluation” replace ambiguous query/readout names. Wire version 7 requires regenerated exports/native programs. A real SQL batch acceptance test selects a shared summary and executes both results; exact-count fixtures now use valid finalizers. Bulk retained evidence cannot hide any ASAP descendant (regression reproduced and fixed).

Validation: after integrating #532, all 1,588 workspace tests pass with two existing ignores. Workspace/all-target/all-feature clippy passes with warnings denied. cargo test --workspace --locked, fmt, clippy with warnings denied, external MetricsQL consumer, and DAG viewer Python tests. The viewer's six Node-dependent tests were skipped because Node.js is unavailable. The vendored MetricsQL baseline passes on Rust 1.99 (CI), verifying its existing 21 library and 3 doctest failures without changing fingerprints. Formatting and clippy also pass locally on Rust 1.99.

Revised logical foundation (#509/#511)

The first five scopes are now ordered as:

feat(ir): define compatible logical summary merges #560 — compatible logical summary merge structure.
feat(ir): add logical sub-DAG sharing and export without execution timing #537 — canonicalization, common sub-DAG sharing and phase-free LogicalASAPDAG export.
feat(sql): lower SQL to unified operator and scalar IR #539 — SQL operator/scalar lowering.
feat(promql): lower queries to unified operator and scalar IR #540 — PromQL/ MetricsQL operator/scalar lowering.
feat(planner): Stage 1 Pass 1 per-computation candidate generation #561 — unranked local logical realization inventory, preserving exact/specialized/universal choices.

Timing follows physical materialization; there is no intermediate timed-DAG stage. #541 and its existing descendants remain on the frozen stack/528-legacy-physical-base snapshot until the physical contract/materialization/selection scopes are reorganized. The first-five tip passes 1,961 workspace tests/doctests, formatting and full workspace Clippy.

Summary coverage prerequisite

#567 adds separate summary observation coverage metadata identified as missing in #535. Updated logical review order: #567 → #560 → #537 → #539 → #540 → #561. #560 now requires known disjoint coverage and derives its union; #537 preserves coverage in CSE and logical transport.

…ieldDataType Column -> Field, Schema.columns -> fields, SummaryFamilyType -> FieldDataType (same variants); SummarySchema/SummaryField deleted. Post-ASAP node schemas are built through Schema::lifted (no unique keys, closed). Adds the unified operator IR module (ir::{OperatorNode, Operator, NonASAPOp, ASAPOp, ScalarExpr}) and the lifecycle timing pass alongside the existing types. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…the unified IR; frontend-common resolver crate - ir::canonicalize / ir::cse port the pre-ASAP passes to Rc<OperatorNode>; CSE serves NonASAP and ASAP nodes alike and follows scalar references. - ir::export emits one post-ASAP node per operator (wire version 6): relational operators are Relational payloads, scalar references are ScalarRef edges, no embedded subtrees. - ir::timing writes execution timings from a LifecycleAssignment and validates every edge; nodes carry no timing before that pass. - asap-frontend-common owns the name-based tree front ends build (UnresolvedOp / UnresolvedScalar) and resolve_root, which binds names, derives schemas and canonicalizes into Rc<OperatorNode>. - ParsedWorkload roots are Rc<OperatorNode>. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

One builder serves export / export_summary / export_post_asap; nodes are deduplicated by pointer, scalar operator references render as scalar_ref ids, every node carries structural_hash. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The SQL front end now emits ScalarExpr::{Exists, InSubquery, ScalarSubquery}; canonicalize rewrites them to the semi/anti/cross joins the planner saw before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

SQL, PromQL and MetricsQL front ends build UnresolvedOp/UnresolvedScalar and return Rc<OperatorNode> from resolve_root. New: SQL Values (SELECT without FROM, VALUES), unary minus as Negative, ExprSemantics on every scalar comparison/arithmetic, uncorrelated EXISTS / IN / scalar subqueries as scalar expressions (lowered to joins by canonicalize); PromQL TimeRange.kind (instant vs range selector), the bool modifier (return_bool), scalar negation as Negative. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…e unified IR (library) Replacement::{Summary, Rewrite} collapse into Subtree(Rc<OperatorNode>); assemble_residual becomes one rule (keep the operator, assemble its children); nodes carry no timing, legality checks run through ir::timing::validate_default and export through ir::export after apply_lifecycle_timings. Test modules are migrated separately. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

… kept subtree one node keep_pre_asap memoizes its exact-guarantee copy per input node, so a Scan kept by two candidates (AVG's SUM and COUNT branches) stays one Rc. The viewer contract test moves to crates/devtools/tests/viewer_contract.rs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Developer and architecture docs, the user guide and the viewer README now describe one OperatorNode DAG, lifecycle-assigned timing and wire version 6; the viewer renderer reads schema fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…gressions Regressions caught by the migrated suites: - accuracy-evidence and runtime-support checks now cover summary decisions rooted in a relational operator above their readouts; - exact read boundaries keep the placement binding chose (query-time snapshot vs. maintenance feed); the timing pass honors it; - generic assembly keeps a value-computing operator (aggregate, binary op, window) exact over an approximate input instead of passing a guarantee through it; - table populations name their unified-IR input, so filtered SQL scans are recognized again. Adds the #468 acceptance tests (integration-tests/tests/operator_sharing.rs). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

QueryExpr, SummaryNode/SummaryExpr, ValueOperation, the wire-5 post-ASAP DAG, the old resolver, canonicalize and both CSE passes are gone; the unified ir module is the only operator language. pre_asap keeps the shared vocabulary (query_expr.rs renamed vocabulary.rs); post_asap keeps summary state and timing vocabulary. CSE compares a node's own fields with the signed-zero-aware check. cargo fmt --all. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge main into refactor/operator-flattening while retaining the unified OperatorNode graph proposed in #511. Port native physical compilation, aggregate filters, shared summaries and lifecycle placement to that graph. Remove ScalarBridge, preserve scalar workload roots as owned expressions, and lower mixed PromQL arithmetic and comparisons to Project/Filter with complete series identity and explicit vector/scalar conversions. Preserve native execution coverage and correct cross-selector maintenance eligibility and empty PromQL aggregation behavior. Validation: 1,569 workspace tests pass; workspace all-target/all-feature Clippy with warnings denied, rustfmt, and MetricsQL external consumer pass. Vendored MetricsQL baseline retains its main-branch sources and checker: lib baseline passes, but one existing doctest diagnostic hash differs under rustc 1.98.1 (same expected failing test set).

zzylol · 2026-10-02T16:09:10Z

-    use asap_types::pre_asap::query_expr::{GroupKeys, Source};
-    use asap_types::pre_asap::schema::{Column, ColumnId, DataType, Schema};
+    use asap_types::pre_asap::schema::{ColumnId, DataType, Field, Schema};
+    use asap_types::pre_asap::vocabulary::{GroupKeys, Source};


Why this is called vocabulary

Does it reflect the actual content?

“Vocabulary” was too vague and the module mixed different responsibilities. It contained operator parameter types (joins, windows, predicates, reductions, etc.), aggregate schema derivation, and IR errors. Split it into ir::operator_properties, ir::aggregate_schema, and ir::error; migrated imports and removed pre_asap::vocabulary. The modules now describe their actual contents and belong to the common IR. Change.

ir::operator_properties, ir::aggregate_schema, and ir::error what does these three mean then?

For your follow-up, these modules separate three concrete responsibilities:

ir::operator_properties defines settings stored inside operator payloads: e.g. GroupKeys says which columns an aggregate groups by, JoinKind selects inner/left/etc., and WindowFrame describes a SQL window. It does not hold derived node metadata such as schema, guarantee, or timing.

ir::aggregate_schema computes an aggregate's output schema from its input schema and reduction. For example, grouping by host preserves that column and adds the aggregate result column with its derived type. It does not compute aggregate values.

ir::error defines QueryExprError for schema/type derivation failures: invalid grouping-column indices, scalar signatures, sample columns, or an empty concatenation. My previous module description was too broad: structural and execution-timing validation have separate errors.

fc328b12 adds these explanations and examples to the module docs, the IR module index, and the acceptance document. This makes the boundaries explicit without introducing another abstraction or changing behavior. Workspace tests and clippy pass.

Given that QueryExpr is removed, why QueryExprError still here?

@Selvomega needing your help on this.

You are right: QueryExprError was a leftover name from the removed QueryExpr, not a separate concept that needed to survive the migration. Fixed in 150735f2: it is now ir::SchemaDerivationError, describing its actual role in deriving operator output schemas and scalar types. Updated signatures, imports, re-exports, error construction, and documentation throughout; no QueryExprError alias remains. Error variants and behavior are unchanged. Types/frontend-common tests, formatting, and workspace/all-target/all-feature clippy pass.

How much stuffs is still pending on this thread?

Selvomega and others added 11 commits October 2, 2026 00:02

viewer: node kind categories follow the unified IR

e905008

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

docs: describe the unified operator IR

e258e17

Developer and architecture docs, the user guide and the viewer README now describe one OperatorNode DAG, lifecycle-assigned timing and wire version 6; the viewer renderer reads schema fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Selvomega requested review from milindsrivastava1997 and zzylol October 2, 2026 11:58

zzylol changed the title ~~refactor: operator flattening~~ Design: operator sharing and separating operator and ScalarExpr in logical plan Oct 2, 2026

zzylol mentioned this pull request Oct 2, 2026

Separate execution placement from logical semantics and cost ingestion-time alternatives #530

Open

zzylol added 3 commits October 2, 2026 15:28

fix(ir): complete scalar lowering and preserve language semantics

36e8838

fix(ir): validate unified state and readout contracts

fca91f5

docs: record operator design acceptance and update DAG viewer

bea0e41

zzylol changed the title ~~Design: operator sharing and separating operator and ScalarExpr in logical plan~~ refactor: unify logical operators and scalar expressions Oct 2, 2026

fix: satisfy Rust 1.99 lint checks

8630bd6

zzylol reviewed Oct 2, 2026

View reviewed changes

Comment thread crates/asap-aware-mapping/src/accuracy/reconciliation.rs Outdated

zzylol reviewed Oct 2, 2026

View reviewed changes

Comment thread crates/asap-aware-mapping/src/accuracy/reconciliation.rs Outdated

zzylol reviewed Oct 2, 2026

View reviewed changes

zzylol requested changes Oct 2, 2026

View reviewed changes

zzylol added 4 commits October 2, 2026 16:49

refactor: clarify workload DAG and summary evaluation contracts

1e0d4ee

fix: prevent retained evidence from hiding summary work

c036a14

docs: clarify retained DAG cost boundaries

f85ef87

fix: generalize summary cost evidence across arrival modes

b68b231

zzylol mentioned this pull request Oct 2, 2026

fix: generalize summary cost evidence across arrival modes #532

Merged

zzylol marked this pull request as draft October 3, 2026 20:02

zzylol mentioned this pull request Oct 12, 2026

Stage 3: memory, disk and latency trade-offs per candidate, and cost models beyond cloud billing #660

Open

zzylol closed this Oct 12, 2026

zzylol deleted the refactor/operator-flattening branch October 12, 2026 19:28

Conversation

Selvomega commented Oct 2, 2026 • edited by zzylol Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Revised priority (2026-10-03)

Revised logical foundation (#509/#511)

Summary coverage prerequisite

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Selvomega commented Oct 2, 2026 •

edited by zzylol

Loading