From 86edc7d4425face621e62f35609003aa678cb1fb Mon Sep 17 00:00:00 2001 From: Selvomega Date: Mon, 28 Sep 2026 02:07:47 +0000 Subject: [PATCH 1/5] docs: propose operator flattening (#468) Add a design proposal for merging the pre- and post-ASAP operator sets into one Operator { Basic(OriginalOp), Ext(ASAPOp) } tree, and list it in the proposals index. Co-Authored-By: Claude Opus 5.5 --- docs/design_docs/proposals/README.md | 1 + .../proposals/operator-flattening.md | 500 ++++++++++++++++++ 2 files changed, 501 insertions(+) create mode 100644 docs/design_docs/proposals/operator-flattening.md diff --git a/docs/design_docs/proposals/README.md b/docs/design_docs/proposals/README.md index 76cc23402..78084caa4 100644 --- a/docs/design_docs/proposals/README.md +++ b/docs/design_docs/proposals/README.md @@ -8,3 +8,4 @@ extensions. A design document is not a promise of downstream runtime support. - [ASAPQuery rule coverage](asapquery-rule-coverage.md) - [UnivMon frequency summary](univmon-frequency-summary.md) - [ASAP-aware mapping proposals](asap-aware-mapping/README.md) +- [Operator flattening](operator-flattening.md) diff --git a/docs/design_docs/proposals/operator-flattening.md b/docs/design_docs/proposals/operator-flattening.md new file mode 100644 index 000000000..577ba45fb --- /dev/null +++ b/docs/design_docs/proposals/operator-flattening.md @@ -0,0 +1,500 @@ +# Operator flattening + +> Status: proposed, not implemented. Problem statement: +> [#468](https://github.com/ProjectASAP/ASAPPlanner/issues/468). Code +> references and counts are against `main` at `8acb472`. + +**In one sentence**: split `QueryExpr` into original operators (`OriginalOp`) and +scalar expressions (`ScalarExpr`); introduce `Operator { Basic(OriginalOp), Ext(ASAPOp) }` +so that every operator's children are `Rc`; then delete the parallel +post-ASAP IR (`SummaryExpr`, `SummaryNode`, `SummarySchema`, and the relational +variants of `ValueOperation`). + +**Roadmap**: + +| Part | Sections | Question answered | +|---|---|---| +| I. New IR | §1 Types, §2 Per-node information | Which node kinds exist, and what each carries | +| II. Upstream and downstream changes | §3 Frontends and entry → §4 Planner → §5 Validation → §6 Export → §7 Other consumers | How each stage changes, in data-flow order | +| III. Implementation plan | §8 Stages and tests, §9 Out of scope | How to land it, and what is deliberately left out | + +--- + +# I. New IR + +## 1. Types + +### 1.1 Overview + +``` +Operator operator: one node of the DAG; every child is Rc> +├─ Basic(OriginalOp) original operator: today's relational QueryExpr variants (Scan / Filter / Join / Aggregate / SetOp …) +└─ Ext(ASAPOp) ASAP operator: summary operators (SummaryAgg / SummaryEstimate …) + +ScalarExpr scalar expression: only appears inside operator fields (predicates, SELECT lists); never a node +``` + +- **Naming**: the suffix says what a type is — `*Op` is an operator, `*Expr` is an + expression. `OriginalOp` holds the operators pre-ASAP already has, as opposed to + the extension `ASAPOp`. The name `QueryExpr` goes away. +- **The generic `C`**: the existing column-reference stage of `QueryExpr` — + `ColumnRef` (by name) out of the frontends, `ColumnId` (by position) after + `resolve_root`. A parent and its children must be in the same stage, so all + three types carry the same `C`. + +### 1.2 `OriginalOp` and `ScalarExpr`: splitting `QueryExpr` + +Today one `QueryExpr` enum holds two kinds of things: + +```sql +SELECT l_quantity * 2 AS q2 FROM lineitem WHERE l_quantity > 10 +``` +``` +Project { cols: [ProjectItem { expr: Arithmetic(Column(4) * Literal(2)) }], ← scalar expression: computes one value per row + child: Filter { pred: Predicate(Compare(Column(4) > Literal(10))), ← scalar expression + child: Scan lineitem } } ← relational operator: rows in, rows out +``` + +`Filter.child` (rows) and the `Compare` inside `Filter.pred` (a value) are both +`QueryExpr`; only the field position tells them apart. The code already knows +which variants are scalars: `output_schema` returns `ScalarHasNoRowSchema` for all +of them (`types/src/pre_asap/query_expr.rs:1447`). Split them into two types: + +```rust +pub enum OriginalOp { + Scan { .. }, Filter { pred: Predicate, child: Rc> }, Project { cols: Vec>, child }, + Aggregate { .. }, Join { .. }, SetOp { .. }, Concat { .. }, Dedup { .. }, Sort { .. }, Limit { .. }, BinaryOp { .. }, + SQLWindowFunc { .. }, TimeRange { .. }, TimeShift { .. }, Promql* { .. }, + ScalarBridge(Rc>), // formerly PromqlScalarBridge: a scalar in operator position (the `2` in PromQL `v * 2`, a bare scalar query) + EvalTimestamp, // PromQL time(): operator position, one row, one column +} +pub enum ScalarExpr { // children are ScalarExpr only + Column(C), Literal(ScalarValue), Compare { .. }, BoolAnd(..), BoolOr(..), Not(..), IsNull(..), IsNotNull(..), + Cast { .. }, InList { .. }, FunctionCall { .. }, Arithmetic { .. }, Case { .. }, CurrentTimestamp, +} +pub struct Predicate(pub Rc>); +pub struct ProjectItem { pub alias: Option, pub expr: ScalarExpr } +``` + +- **Ambiguous variants**: `PromqlScalarBridge`, `EvalTimestamp`, + `PromqlScalarFromVector` and `PromqlVectorFromScalar` appear in operator position + and have a row schema, so they belong to `OriginalOp`. `CurrentTimestamp` (SQL + `NOW()`) is used both ways: it has a row schema (`query_expr.rs:1388`) and takes + part in scalar type inference (`:1674`, e.g. `WHERE ts > NOW()`). It goes into + `ScalarExpr` and is written `ScalarBridge(CurrentTimestamp)` in operator + position. **Open**: confirm how the frontends place `CurrentTimestamp` in + operator position. +- **Benefits**: a scalar in operator position (`Basic(Column(3))`) is no longer + expressible; the `ScalarHasNoRowSchema` runtime error disappears; `canonicalize` + and `pre_asap/cse` already walk only the relational skeleton and treat scalars as + opaque data (see the `pre_asap/cse.rs` module docs) — the types now say so. + +### 1.3 `ASAPOp` + +```rust +pub enum ASAPOp { + SummaryAgg { child: Rc>, family: ASAPType, input, reduction, grouping, // family was SummaryFamilyType, "never Plain" by convention; now by type + guarantee: Option }, // Some only for the ExactAggregate family + SummaryEstimate { child, query: SketchQuery, + guarantee: Option }, // None = the AccuracyModel has no error model for this family + SummaryMerge { children: Vec>>, timing: ExecutionTiming }, + SummarySubtract { left, right }, + SummaryDelete { child, key: C }, // was ColumnRef; now C like everything else + SummaryJoin { outer, inner, key: C, family: ASAPType }, + // The four summary-specific ValueOperation variants, lifted to the top level instead of { child, operation, timing } + FinalizeExactAccumulator { child, timing: ExecutionTiming }, + MaintainPopulation { child, population }, // always ingestion time + ReadPopulation { child, readout }, // always query time + Extension { child, name: String }, +} +impl ASAPOp { pub fn guarantee(&self) -> Option<&ResultGuarantee>; } // only the two variants above store one; see §2.2 +``` + +**Deleted post-ASAP types** — each is expressed by an existing `OriginalOp` variant: + +| Deleted | Expressed as | +|---|---| +| `SummaryExpr::KeepPreAsap(q)` | no wrapper: the subtree `q` is itself `Basic(..)` | +| `ValueOperation::{Project, Filter, Sort, Limit}` | `OriginalOp::{Project, Filter, Sort, Limit}` | +| `SummaryExpr::BinaryOp`, `SummaryExpr::RelationalJoin` | `OriginalOp::BinaryOp`, `OriginalOp::Join` | +| `ValueOperation::Exact(Aggregate)`, `ExactOperation` | `OriginalOp::Aggregate` | +| `SummaryNode` (the node wrapper) | not needed; per-node information is covered in §2 | + +### 1.4 Child slots + +```rust +// Today // After +Filter { Filter { + pred: Predicate(Rc), // scalar pred: Predicate(Rc), + child: Rc, // rows child: Rc, // either Basic(..) or Ext(SummaryEstimate ..) +} } +``` + +**Rule**: every `OriginalOp` field that holds input rows has type `Rc>`. +There are about 25 such fields — `Filter.child`, `Join.left` / `Join.right`, both +sides of `SetOp`, `Concat.children`, `BinaryOp.lhs` / `BinaryOp.rhs`, the `child` of +each `Promql*` operator, and so on. + +**Effect**: an ASAP operator can sit directly under any original operator. Both +sides of a `SetOp`, for example, can be `SummaryEstimate`s. + +**Exception: `Concat.children`**. Today it is `Vec`: branches are stored +by value and have no `Rc` identity. The planner's search and assembly (§4) +identify nodes by `Rc` pointer — targets are registered by pointer and holes are +recognized with `Rc::ptr_eq` — so a `Concat` branch can be neither a target nor a +hole. This affects, for example, the branches SQL `ROLLUP` and PromQL +`histogram_quantiles` are lowered into. As `Vec>` it is handled like +any other child slot and §4 needs no special case (§8 stage 0). + +## 2. Per-node information + +A post-ASAP node carries three pieces of information today. After the change: + +| Field | Meaning | Today | After | +|---|---|---|---| +| `schema` | which columns the node outputs, and their types | stored on every `SummaryNode` | not stored; `output_schema()` computes it on demand (§2.1) | +| `guarantee` | how far the node's output value can be off (metric, bound, failure probability, provenance) | stored on every `SummaryNode` | stored on `SummaryAgg` / `SummaryEstimate`; derived by `GuaranteeIndex` for everything else (§2.2) | +| `timing` | whether the node runs at ingestion time or at query time | stored on the `BinaryOp` / `ValueOperation` / `SummaryMerge` variants | not stored on `OriginalOp` (§5); kept on `SummaryMerge` and `FinalizeExactAccumulator` | + +### 2.1 Schema: two types become one + +A schema is a node's list of output columns: name, type, nullability. There are two today: + +| | pre-ASAP | post-ASAP | +|---|---|---| +| Type | `Schema { columns: Vec, time_index, unique_keys, closed }` | `SummarySchema { fields: Vec, time_index }` | +| Column type | `Column.dtype: DataType` — plain values only (`Int64`, `Float64`, `Utf8`, …) | `SummaryField.dtype: SummaryFamilyType` — a plain value `Plain(DataType)` or summary state (`Sketch(Kll, …)`, …) | +| Where it lives | not stored; `QueryExpr::output_schema()` computes it | stored on every `SummaryNode` | + +In the new IR both kinds of operator share one tree, so `Operator::output_schema()` +must describe any node, and a column type must be able to hold either a plain +value or summary state: + +``` +SummaryEstimate(Quantile .99) → [p99: DataType(Float64)] + SummaryAgg(Kll) → [state: ASAPType(Sketch(Kll, k=269))] + Scan lineitem → [l_orderkey: DataType(Int64), l_quantity: DataType(Float64), …] +``` + +So keep `Schema`, delete `SummarySchema` / `SummaryField`, and give `Column.dtype` a new type, `ColumnType`: + +```rust +pub enum ColumnType { + DataType(DataType), // a plain value; the existing DataType, unchanged (Int64, Float64, Utf8, …) + ASAPType(ASAPType), // ASAP state +} +pub enum ASAPType { // SummaryFamilyType without its Plain variant + ExactAggregate(ExactKind, ExactParams), Sketch(SketchKind, GroupingStrategy), + Sample(SamplingKind, SamplingParams), Wavelet(WaveletKind, WaveletParams), StatModel(StatModelKind, StatModelParams), +} + +pub struct Column { pub name, pub dtype: ColumnType, pub nullable, pub table: Option } +impl Column { + pub fn plain(name, DataType) -> Self; // build a plain column (frontends, catalogs) + pub fn plain_dtype(&self) -> Option<&DataType>; // None for an ASAP-state column + pub fn expect_plain_dtype(&self) -> &DataType; // frontends and scalar type inference: a state column is a bug, panic +} +``` + +- **Naming**: as in §1, `DataType` / `ASAPType` mirror `OriginalOp` / `ASAPOp`. + The name `SummaryFamilyType` goes away: once every column carries it, a plain + column reading `SummaryFamilyType::Plain(Float64)` is a misnomer. With two + levels, "is this column state?" is a check on the outer variant only — exactly + the split `check_plain_operands` needs. +- **`DataType` itself is unchanged.** It remains part of the scalar type system + (the type of a `Literal`, the result type of `Arithmetic`, catalog column + declarations); it now sits inside `ColumnType::DataType(..)`. Code that reads + `Column.dtype` expecting a `DataType` unwraps one more level: code that only + ever sees plain columns (frontends, scalar type inference) uses + `expect_plain_dtype()`; code that may see state (planner, validation, export) + uses `plain_dtype()` or matches. A scalar expression never legitimately reads a + state column — `check_plain_operands` (§5) rejects such a DAG first. +- **Elements of nested types**: `DataType::List { element: Box }` and + `DataType::Struct { fields: Vec }` have `Column` elements. With + `Column.dtype: ColumnType` an element could become ASAP state (a "list of KLL + sketches"), which is not intended. These two use a plain-only field instead: + + ```rust + pub struct PlainField { pub name: String, pub dtype: DataType, pub nullable: bool } // nested fields are already unqualified (List's doc comment), so no table + DataType::List { element: Box } + DataType::Struct { fields: Vec } + ``` + + `DataType::Map { key: Box, value: Box, .. }` already uses + `DataType` only and is unchanged. There are 36 construction or match sites of + `DataType::{List, Struct, Map}`. +- **Why keep `Schema`**: `ColumnType::DataType(..)` represents every plain type, so + nothing is lost, and `Schema` additionally has `unique_keys` / `closed` / + `Column.table`, which merging the other way would drop. +- **Computed on demand**: schemas are no longer stored on nodes; `Operator::output_schema()` + computes them. `OriginalOp` keeps today's `QueryExpr::output_schema()` logic, and + `ASAPOp` follows the table below. These rules are currently spread over + `asap-aware-mapping/src/replacement.rs` and `types/src/post_asap/execution_data_state.rs`; + they move into `asap-types`. +- **Deleted conversions**: `lift()` (`replacement.rs:3352`) and `lift_plain` + (`execution_data_state.rs:725`), which turn a `Schema` into a `SummarySchema`, and + the reverse `plain_schema` (`execution_data_state.rs:707`). With one schema type + there is nothing to convert. +- **Size of the change**: 7 direct `Column { .. }` literals; the deleted + `SummarySchema {}` (36) and `SummaryField {}` (19) literals. In non-test code + `SummaryFamilyType` appears 197 times (42 of them with `Plain(`) and `.dtype` is + read at 67 sites; all are rewritten against the new types. + +| `ASAPOp` | Output schema | +|---|---| +| `SummaryAgg` | the grouping columns as-is + one `ASAPType(family)` column | +| `SummaryEstimate` | the row schema determined by the `SketchQuery` | +| `SummaryMerge` / `Subtract` / `Delete` / `Join` | one `ASAPType(family)` column | +| `FinalizeExactAccumulator` | the child's schema, with each `ASAPType(ExactAggregate ..)` column turned into the matching `DataType(..)` | +| `MaintainPopulation` / `ReadPopulation` | existing implementation | + +### 2.2 Guarantee: some stored, some derived + +| Node | Where its guarantee comes from | +|---|---| +| `SummaryAgg`, `SummaryEstimate` | **stored in a field**. Computed at binding time by the `AccuracyModel` from the summary's parameters and the evidence; it cannot be recovered afterwards, and selection needs it during search to decide whether a candidate meets the accuracy target (`replacement.rs:5831`) | +| `OriginalOp` (`Project`, `Join`, …), `FinalizeExactAccumulator` | composed from the children by rule (`relational_join_guarantee`; `exact_operation_rule` in `accuracy/composition.rs`). A subtree with no `Ext` descendant is exact | +| `MaintainPopulation`, `ReadPopulation` | always exact | +| `SummaryMerge` / `Subtract` / `Delete` / `Join` | output state, so no guarantee of their own — but they change the error of a later readout (see below) | + +The derived part goes into one table: + +```rust +/// One traversal, keyed by node pointer — the same approach as the ExecutionDataStateAssignment +/// returned by validate_execution_data_states. +pub struct GuaranteeIndex(HashMap<*const Operator, ResultGuarantee>); +pub fn guarantee_index(root: &Rc) -> GuaranteeIndex; +``` + +- The composition rules also depend on the `AccuracyModel`, so the planner must + compute `GuaranteeIndex` with the same model and hand it over together with the + assembled root (`assemble_selected_dag`). `compile_executable_dag` uses it to fill + `ExecutableDagNode.guarantee`. +- Not chosen: wrapping every node to store a guarantee — frontends and `resolve` + would then have to build the wrapper too. +- **Gap for state nodes**: `SummaryMerge` / `Subtract` / `Delete` / `Join` change the + error of a later readout. For a CMS, `a − b` has error bound `ε·(‖a‖₁ + ‖b‖₁)`, so + the relative error is large when `a` and `b` are close. A readout's guarantee is + computed today as if its input were a sketch built directly by a `SummaryAgg`, + ignoring any intermediate operation. The planner on `main` never emits these four + nodes (they are built only in tests, and `SummaryMerge` is documented as inserted by + a deployment), so the gap is latent. Not addressed here (§9). + +--- + +# II. Upstream and downstream changes + +## 3. Frontends and the optimizer entry + +Trees built by the frontends and by `resolve` contain only `Basic`. They access +children through `expect_basic()`; before a tree reaches the optimizer, the entry +checks it once and rejects any `Ext`: + +```rust +impl Operator { + pub fn contains_ext(&self) -> bool; + pub fn expect_basic(&self) -> &OriginalOp; // an Ext here is a bug: panic +} +``` + +**Where the check goes**: on `main` the optimizer entries are `search_workload` / +`search_workload_with` / `search_workload_with_targets`. They do not return +`Result`, so the check starts as `assert!(!root.contains_ext())` at each entry. If +the single `ParsedWorkload` entry point (#429) lands on `main` first, the check +moves into `ParsedWorkload::new` and returns an error (it already checks the entry +count). + +**Why not a compile-time guarantee** (an associated type `Ext` on `ColState`, with +`ColumnRef::Ext = Never`): + +| | Compile time | Run time (this proposal) | +|---|---|---| +| Types | one more associated type on `ColState`, plus an empty enum | the `Ext` variant holds `ASAPOp` directly | +| What it catches | only frontend code before `resolve`. A frontend already returns a `ColumnId` tree, where `Ext` is allowed | **every input to the optimizer**: frontend code after `resolve`, deserialized plans, third-party frontends, hand-written test IR | +| Frontends matching children | `basic()` is total | `expect_basic()`, which can in principle panic | + +The compile-time version protects only a small stretch inside the frontends; the +runtime check sits at the entry, covers more, and keeps the types simpler. So one +`Operator` type serves both pre- and post-ASAP, told apart by the entry check +rather than by a type parameter. + +## 4. Planner: search and assembly + +**Candidates**: three shapes become two. + +```rust +pub enum Replacement { + Subtree(Rc), // formerly Summary(Rc) and Rewrite(Rc); "introduces a summary" = contains_ext() + ExactComposition { .. }, // unchanged, except its plan becomes Rc instead of Rc +} +``` + +**Holes**: a subtree of a candidate that is `Rc::ptr_eq` to some target is a hole, +left for that target's own choice to fill. A candidate that needs a specific +implementation of a child (for example a `SummaryAgg` that needs an exact +accumulator value, as in `realize_temporal_average`) inlines the child as a new node; +the pointer differs, so it is not a hole. `realize_child_with` changes from +"realize the child recursively, else `keep_pre_asap`" to "else leave a hole". + +**Assembly**: one generic rule replaces `assemble_residual`. + +```rust +fn assemble(&self, t: &Rc) -> Rc { + memo by ptr; // a shared child stays one Rc — today's assembled_nodes + let body = match self.chosen(t) { + Some(Subtree(r)) => r, // Rewrites are assembled further down too (today replacement.rs:4571 wraps the whole rewrite in keep_pre_asap and ignores the choices below) + Some(ExactComposition{..}) => composition.plan, + None => t, // no candidate chosen: keep the node, recurse into its children — what assemble_residual does for only four operators + }; + body.map_children(|c| if is_target(c) { self.assemble(c) } else { c }) +} +``` + +After assembly, run the data-state validation (§5) once; if filling a hole is +illegal (e.g. a query-time `SummaryEstimate` under a `SummaryAgg`), that hole falls +back to its original subtree. This generalizes the fallback `relink_summary` does +today for `Aggregate` only. Finally compute `GuaranteeIndex` (§2.2). + +- **`map_children`**: reuse `rebuild_children` from `pre_asap/cse.rs:576` as + `Operator::map_children`, dispatching `Basic` to `OriginalOp::map_children` and + `Ext` to `ASAPOp::map_children`. +- **Deleted**: `assemble_residual`, `relink_summary`, the `query_time_nested_sum` / + `contains_aggregate` special cases, `keep_pre_asap` / `keep_pre_asap_rc`, and the + branch of `finalize_exact_accumulator` that builds an empty schema for `KeepPreAsap`. + +How the three problems in #468 are resolved: + +| Problem | Resolution | +|---|---| +| 1. One operator, two spellings: a `Project` is a `QueryExpr` inside `KeepPreAsap` and a `ValueOperation` outside it | one set of types; `assemble` no longer builds `ValueOperation`s | +| 2. `KeepPreAsap` is opaque: nothing outside can reference the `Scan` inside, so an exact aggregate and a sketch cannot share one scan | `Scan` is no longer wrapped; `SummaryAgg{child: scan}` and `Aggregate{child: scan}` can point to the same `Rc`. Splitting a multi-measure `Aggregate` into `avg` + KLL is a binding-rule change; this proposal only makes the split expressible | +| 3. `SetOp` and similar operators have no post-ASAP counterpart and must stay inside `KeepPreAsap`, so no subtree below them can use a summary | `SetOp` takes the `None => t` path and both children are assembled independently | + +## 5. Validation: execution data states + +Today's rule `produced_data_state(KeepPreAsap) = None` (decided by the edge that +reaches the node) extends to every `OriginalOp`: + +| Node | Produced state | +|---|---| +| `OriginalOp` | none of its own; assigned by the consuming edge (`QUERY_ROWS` at the root) and passed down unchanged | +| `SummaryAgg` | `{the child's timing, SummaryState}`; ingestion time when the child is an `OriginalOp` | +| other `ASAPOp` | unchanged | + +The `KeepPreAsap` / `BinaryOp` / `ValueOperation` / `RelationalJoin` arms of +`validate_execution_data_states` merge into one `Basic` arm: + +- Pass the node's state to each child; an `Ext` child is checked against the + `Ext` edge rules (e.g. `SummaryEstimate` only on the query side). +- `check_plain_operands` stays: every column an `OriginalOp` references must be + `ColumnType::DataType`; `Project` / `Filter` / `Sort` / `Limit` may pass + `ExactAggregate` columns through (today's exception). This is what rejects + `Project(Ext(SummaryAgg))`, i.e. projecting sketch state as if it were rows. +- The extra ingestion-side constraints on `BinaryOp` (arithmetic only, identical + schemas on both sides, exactly one `Float64`) move into this arm, per variant. +- `AmbiguousKeepPreAsap` is renamed `AmbiguousDataState`; its meaning is unchanged. + +## 6. Export: fragments in the executable DAG + +`ExecutableDag` has four payloads for original operators today — `fallback{expression: QueryExpr}`, +`binary`, `value` and `relational_join`. Afterwards there is one: + +```rust +ExecutableOperatorPayload::Relational { + /// An Operator with no Ext (checked at construction). Leaves are real Scans, or + /// Scan { source: Source::DagInput { role } } standing for an incoming edge whose + /// schema is that edge's intermediate_schema. + expression: Operator, +} +``` + +`Source` gains a variant `DagInput { role: EdgeRole }`. `compile_executable_dag` works as follows: + +1. Starting from the root, take the **largest connected subtree containing no + `Ext`** as one fragment node; wherever the fragment meets an `Ext`, cut an edge + and put a `DagInput` leaf in the fragment. +2. Each `Ext` node maps one-to-one onto the existing `summary_agg` / + `summary_estimate` / … payloads; `FinalizeExactAccumulator` / `MaintainPopulation` / + `ReadPopulation` / `Extension` become `value{operation}`. `ValueOperation` stays as a + wire type with only these four variants. +3. Today's `fallback` is a fragment with no `DagInput` leaf. A backend's existing + path that recursively lowers a `QueryExpr` handles every fragment after unwrapping + one `Basic` level and adding a `DagInput` arm (treat the incoming edge as a + materialized table); the dedicated lowerings for `binary` / `value::Project` / + `relational_join` can go. + +- **Wire version 5 → 6**: three fewer payloads; `fallback` is renamed `relational` + and allows `DagInput` leaves; `output_schema` / `intermediate_schema` change from + `SummarySchema` to `Schema` (adding `unique_keys` / `closed` / `table`). +- **Phase granularity**: ingestion / query phase is now one per fragment, and + `with_execution_phases` still assigns it per node. Switching phase inside a + fragment would need a materialization point, and materialization points are + exactly `Ext` nodes (`FinalizeExactAccumulator`, `MaintainPopulation`), so no real + granularity is lost. + +## 7. Other consumers + +| Location | Change | +|---|---| +| `post_asap/cse.rs` | delete. `ASAPOp` derives `PartialEq` + serde, so `share_common_subtrees` in `pre_asap/cse.rs` covers it | +| `dag_export.rs` | delete `build_summary` / `build_summary_hybrid` / `summary_kind_tag`; the one exporter gains an `Ext` arm. The viewer's `node-style.js` drops `KeepPreAsap` / `SummaryBinaryOp` / `ValueOperation` / `RelationalJoin` and adds the `ASAPOp` variant names; the pin test at `dag_export.rs:1843` follows | +| `summary_maintenance_cost/estimator.rs` (the largest, 80 sites) | the `KeepPreAsap` branches through `query_source_selections` / `retained_queries` switch to "largest subtree with no `Ext`" (exactly the §6 fragment); `BinaryOp \| RelationalJoin => "exact_binary"` and `ValueOperation => "value_operation"` fold into the fragment cost. This also fixes the missing `RelationalJoin` arm at `evidence.rs:292`, which can raise `InconsistentOperatorStatistics` | +| `physical_plan_cost_model.rs::estimate_candidate` | `Summary(KeepPreAsap(q))` and `Rewrite(q)` already share `lower_query_physical_dag`; every fragment will go through it | +| `summary_maintenance_lifecycle.rs` | `selected_raw_recompute = matches!(root, KeepPreAsap)` becomes `!contains_ext(root)`; `keep_pre_asap(target)` at `:551` becomes `target` itself | +| `maintained_population.rs` | `KeepPreAsap(source)` becomes `source` itself; `MaintainPopulation`'s `population.matches_input(child)` inspects a `Basic` child directly | +| `exact_composition.rs` | the `ExactOperation::Aggregate` shell becomes a `Basic(Aggregate)` node whose `child` is a hole | +| `RelationalJoin.pruning` | production code never sets `Some`; delete it. Candidate pruning can return later as an `ASAPOp` variant; the `CandidateCompleteness` type stays | + +--- + +# III. Implementation plan + +## 8. Stages and tests + +`main` builds and passes all tests after every stage. + +| Stage | Content | Main changes | +|---|---|---| +| 0 Preparation | `Rc` for `Concat.children`; `rebuild_children` becomes `map_children`; add `Column::plain` | `asap-types`, mechanical | +| 1 Split operators and expressions | §1.2: split `QueryExpr` into `OriginalOp` + `ScalarExpr`; `Predicate` / `ProjectItem` etc. hold `ScalarExpr`; relational children stay `Rc` for now. No semantic change | every place that builds or matches scalars: the three frontends' expression lowering, column resolution in `resolve`, scalar rewrites in `canonicalize`, `scalar_signature.rs`, `column_resolution.rs`. Mechanical | +| 2 Two levels | §1.1, §1.4: add `Operator`, `ASAPOp` (a placeholder with no producer yet), `contains_ext()` and `expect_basic()`; every relational child slot becomes `Rc>`. No semantic change | every place that builds or matches children: the three frontends, `resolve`, `canonicalize`, `pre_asap/cse`, `output_schema`, `dag_export`, all of `asap-aware-mapping`. Each change is the same shape; afterwards the remaining work is local | +| 3 One schema | §2.1: `SummarySchema` → `Schema`, `SummaryFamilyType` → `ColumnType` + `ASAPType`, `PlainField` for `List` / `Struct` elements; delete `lift*` | all of `asap-types` plus schema construction in every crate. The wire format changes only in schema fields; no version bump yet | +| 4 New types alongside old | fill in the `ASAPOp` variants; add the `Ext` arms of `output_schema` and data-state validation (§5), `GuaranteeIndex` (§2.2) and the optimizer-entry check (§3); write `flatten(&SummaryNode) -> Rc`; `compile_executable_dag` and `dag_export` consume the flat tree (planner output goes through `flatten` first). Wire → 6 | `asap-types`, `devtools`, viewer. The planner is untouched, but export and execution already run on the new types | +| 5 Planner switch | §4: candidates and assembly results become `Rc`, `assemble` is rewritten; the cost / lifecycle / estimator code in §7 moves to the new types; delete `flatten` | `asap-aware-mapping`; the largest stage | +| 6 Cleanup | delete `SummaryExpr` / `SummaryNode` / the redundant `ValueOperation` variants / `ExactOperation` / `post_asap/cse.rs`; update `post-asap-ir.md`, `physical-plan-integration.md`, the developer guide and the viewer docs | mostly docs | + +An alternative transition would first make `ValueOperation` wrap a pre-ASAP operator +without changing the structure. Its benefit — export and execution see a flat +structure early — is already delivered by stage 4, which is built on the final types +and so is not throwaway code. That transition is therefore not planned. + +**Tests**: + +- One integration test per problem in §4, asserting the shape of the assembled plan: + 1. `WITH metric AS (SELECT avg(CASE WHEN l_quantity BETWEEN 1 AND 50 THEN 1.0 ELSE 0.0 END) AS in_range FROM lineitem) SELECT in_range, in_range = 1.0 AS ok FROM metric`: + no post-ASAP-only node other than `Ext`, and all three `Project`s are the same variant; + 2. `SELECT avg(l_extendedprice), approx_percentile_cont(l_discount, 0.99) FROM lineitem`: + the `child` of the `avg` `Aggregate` and of the KLL `SummaryAgg` is the same `Scan` by `Rc::ptr_eq` (enabled once a binding rule splits measures); + 3. `SELECT approx_distinct(l_partkey) FROM lineitem UNION ALL SELECT approx_distinct(l_suppkey) FROM lineitem`: + each side of the `SetOp` has a `SummaryEstimate`. +- The optimizer entry rejects a tree containing `Ext`. +- Rewrite the 117 `SummaryExpr::` assertions in `sql_to_post_asap.rs` / + `promql_to_post_asap.rs` / `exact_composition.rs` against the new shape. +- Wire 6 round trip: a fragment with a `DagInput` leaf is equal after serialization + and deserialization; a version-5 document is rejected by `deny_unknown_fields`. +- Keep the shapes of the 52 existing tests in `execution_data_state.rs`; only their + construction changes. + +## 9. Out of scope + +- The binding rule that splits a multi-measure `Aggregate` into "exact + summary" + sharing one child (the second half of problem 2 in §4). +- Candidate pruning as an `ASAPOp` variant. +- **Accuracy of summary state** (§2.2): making a readout's guarantee account for + `SummaryMerge` / `Subtract` / `Delete` / `Join`. Either attach an accuracy + descriptor to state (e.g. "ε relative to the L1 norm"), or compose the error along + the state chain at readout. To be done once the planner starts emitting these nodes. +- Folding `ExactComposition` into `Subtree`. Semantically it equals an `Aggregate` + fragment (query time) or `Finalize(SummaryAgg{ExactAggregate})` (ingestion time), + both expressible afterwards; the cost machinery of `composition_plans` and + `CompositionDecision` stays as is until stage 5 is stable. From 755291e10ed035c2301c0fb9b24bd379ee1b3e6c Mon Sep 17 00:00:00 2001 From: Selvomega Date: Mon, 28 Sep 2026 02:32:27 +0000 Subject: [PATCH 2/5] proposal doc updated --- .../proposals/operator-flattening.md | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/docs/design_docs/proposals/operator-flattening.md b/docs/design_docs/proposals/operator-flattening.md index 577ba45fb..7d2eea9df 100644 --- a/docs/design_docs/proposals/operator-flattening.md +++ b/docs/design_docs/proposals/operator-flattening.md @@ -1,14 +1,26 @@ -# Operator flattening +# Sharing Operators Between Pre-ASAP IR and Post-ASAP IR > Status: proposed, not implemented. Problem statement: > [#468](https://github.com/ProjectASAP/ASAPPlanner/issues/468). Code > references and counts are against `main` at `8acb472`. -**In one sentence**: split `QueryExpr` into original operators (`OriginalOp`) and -scalar expressions (`ScalarExpr`); introduce `Operator { Basic(OriginalOp), Ext(ASAPOp) }` -so that every operator's children are `Rc`; then delete the parallel -post-ASAP IR (`SummaryExpr`, `SummaryNode`, `SummarySchema`, and the relational -variants of `ValueOperation`). +**The idea.** Today the post-ASAP-plan is glued together by different operator types. +This proposal keeps one operator language and makes summary operators extra node kinds in it: any relational operator can sit above a summary, and a summary can read any relational subtree. +Nothing is wrapped and nothing is duplicated. + +``` +Today Proposed +ValueOperation(Project) ← a copy Project + SummaryEstimate Ext(SummaryEstimate) + SummaryAgg(Kll) Ext(SummaryAgg(Kll)) + KeepPreAsap(Scan lineitem) ← a black box Scan lineitem +``` + +Concretely: split `QueryExpr` into operators (`OriginalOp`) and scalar +expressions (`ScalarExpr`), make every operator's children `Rc` with +`Operator = Basic(OriginalOp) | Ext(ASAPOp)`, and delete the parallel post-ASAP +types (`SummaryExpr`, `SummaryNode`, `SummarySchema`, and the relational variants +of `ValueOperation`). **Roadmap**: From ebaba7d1c21b9691c6696d430a04669d34a094b6 Mon Sep 17 00:00:00 2001 From: Selvomega Date: Tue, 29 Sep 2026 03:02:55 +0000 Subject: [PATCH 3/5] design doc updated --- docs/design_docs/proposals/README.md | 3 +- .../proposals/decoupling_op_and_expr.md | 114 ++++ .../proposals/operator-flattening.md | 512 ------------------ .../design_docs/proposals/operator-sharing.md | 428 +++++++++++++++ 4 files changed, 544 insertions(+), 513 deletions(-) create mode 100644 docs/design_docs/proposals/decoupling_op_and_expr.md delete mode 100644 docs/design_docs/proposals/operator-flattening.md create mode 100644 docs/design_docs/proposals/operator-sharing.md diff --git a/docs/design_docs/proposals/README.md b/docs/design_docs/proposals/README.md index 78084caa4..2cebb3b27 100644 --- a/docs/design_docs/proposals/README.md +++ b/docs/design_docs/proposals/README.md @@ -8,4 +8,5 @@ extensions. A design document is not a promise of downstream runtime support. - [ASAPQuery rule coverage](asapquery-rule-coverage.md) - [UnivMon frequency summary](univmon-frequency-summary.md) - [ASAP-aware mapping proposals](asap-aware-mapping/README.md) -- [Operator flattening](operator-flattening.md) +- [Operator sharing](operator-sharing.md) +- [Decoupling operators from scalar expressions](decoupling_op_and_expr.md) diff --git a/docs/design_docs/proposals/decoupling_op_and_expr.md b/docs/design_docs/proposals/decoupling_op_and_expr.md new file mode 100644 index 000000000..9d34b4e4a --- /dev/null +++ b/docs/design_docs/proposals/decoupling_op_and_expr.md @@ -0,0 +1,114 @@ +# Decoupling Operators From Scalar Expressions + +> Status: proposed, not implemented. Companion to [Operator sharing](operator-sharing.md) +> (same PR): this document splits `QueryExpr`; that one builds the shared operator +> language on the result. Code is referenced by file and function; counts are +> approximate, measured on `main` at `8acb472`. + +**The idea.** `QueryExpr` holds two different kinds of node in one enum. This proposal +splits it into `NonASAPOp` (operators) and `ScalarExpr` (scalar expressions), so the +field a node sits in decides its type. + +``` +Today Proposed +Filter { pred: Rc, Filter { pred: Predicate(Rc), + child: Rc } child: Rc } +``` + +## 1. Problem + +``` +SELECT l_quantity * 2 AS q2 FROM lineitem WHERE l_quantity > 10 + +Project ← operator: outputs a table + cols: [Column(4) * Literal(2)] ← scalar expression: outputs one value per input row + child: Filter ← operator + pred: Column(4) > Literal(10) ← scalar expression + child: Scan lineitem ← operator +``` + +- An **operator** outputs a table. It is a node of the plan: the planner can replace it, + share it, or put a summary under it. +- A **scalar expression** has no table of its own. `Column(4)` means "column 4 of the + input of the operator I sit in"; outside that operator it means nothing. + +Today both are `QueryExpr` variants, told apart only by field position. The code already +separates them, but only by convention: + +- `QueryExpr::output_schema` returns `ScalarHasNoRowSchema` for all 13 scalar variants + (`query_expr.rs`), so `Filter { child: Literal(2) }` compiles and fails at run time. +- `pre_asap/cse.rs` never descends into a scalar, and repeats a "scalar: nothing to do" + arm in each of its three traversals; `canonicalize` likewise never rewrites one. + +## 2. Types + +```rust +pub enum NonASAPOp { + Scan { .. }, Filter { pred: Predicate, child: Rc> }, Project { cols: Vec>, child }, + Aggregate { .. }, Join { .. }, SetOp { .. }, Concat { .. }, Dedup { .. }, Sort { .. }, Limit { .. }, BinaryOp { .. }, + SQLWindowFunc { .. }, TimeRange { .. }, TimeShift { .. }, Promql* { .. }, + ScalarBridge(Rc>), // formerly PromqlScalarBridge (the `2` in PromQL `v * 2`) + EvalTimestamp, // PromQL time() +} +pub enum ScalarExpr { + Column(C), Literal(ScalarValue), Compare { .. }, BoolAnd(..), BoolOr(..), Not(..), IsNull(..), IsNotNull(..), + Cast { .. }, InList { .. }, FunctionCall { .. }, Arithmetic { .. }, Case { .. }, CurrentTimestamp, +} +pub struct Predicate(pub Rc>); +pub struct ProjectItem { pub alias: Option, pub expr: ScalarExpr } +``` + +```text +NonASAPOp +├─ children: Rc> (Rc> after operator sharing) +└─ scalar fields: Predicate / ProjectItem / SortKey / ScalarBridge / ... + └─ ScalarExpr + └─ children: ScalarExpr only, never an operator +``` + +- **Naming**: `NonASAPOp` is named for [Operator sharing](operator-sharing.md), where it + becomes the non-ASAP category of `Operator`. `QueryExpr` goes away. +- **Scalar fields**: `Filter.pred`, `Join.pred`, `Aggregate.having` (`Predicate`); + `Project.cols` (`ProjectItem`); `Sort` / `SQLWindowFunc` sort keys (`SortKey`); + `SQLWindowFunc.args`; `PromqlRelabel.value`. +- **Borderline variants** go by position, not by look. `PromqlScalarBridge`, + `EvalTimestamp`, `PromqlScalarFromVector` and `PromqlVectorFromScalar` sit in operator + position with a row schema (e.g. a `BinaryOp` operand) → `NonASAPOp`. `CurrentTimestamp` + (SQL `NOW()`) is produced only by scalar lowering (`df_expr_to_unresolved`); its only + operator-position use is in a unit test → `ScalarExpr`. +- **Gain**: neither a scalar in operator position nor an operator in scalar position is + expressible, and `ScalarHasNoRowSchema` is deleted. + +## 3. Changes + +| Location | Change | +|---|---| +| frontend expression lowering (`df_expr_to_unresolved`, PromQL `walk`) | scalar positions build `ScalarExpr`, operator positions `NonASAPOp` | +| `resolve`, `column_resolution.rs` | separate operator and scalar resolvers; a scalar resolves against its operator's input schema | +| `canonicalize`, `pre_asap/cse.rs` | the "scalar: nothing to do" arms go; scalars are hashed as plain data | +| `scalar_signature.rs`, `infer_expr_type` | take `ScalarExpr` | +| `QueryExpr::output_schema` | becomes `NonASAPOp::output_schema`; the scalar arms and `ScalarHasNoRowSchema` go | + +## 4. Implementation and tests + +This is stage 1 of the joint plan ([Operator sharing §8](operator-sharing.md#8-stages-and-tests)): +children stay `Rc`; operator sharing widens them to `Rc` in its +stage 2. + +**No wire change.** The `fallback` payload serializes a `QueryExpr`, externally tagged. +Variant names are kept, so a tree serializes the same; `ScalarBridge` keeps the name +`PromqlScalarBridge` with `#[serde(rename)]`. + +- Existing tests pass unchanged apart from construction syntax. +- Tests that place a scalar in operator position no longer compile and are rewritten or + deleted: the `CurrentTimestamp` unit test, the `ScalarHasNoRowSchema` tests, and the + `executable_dag.rs` tests using `QueryExpr::Literal` as a `fallback` expression. + +## 5. Limits + +The split relies on no scalar containing an operator. That holds today: SQL +`IN (SELECT …)` / `EXISTS` in a filter lower to a semi-join (`lower_filter`), and every +other subquery-valued expression is rejected (`frontend-sql/src/sql/expr.rs`). Supporting +a scalar subquery (`WHERE x > (SELECT avg(x) …)`) would add `ScalarExpr::Subquery(Rc<..>)`, +make the two types mutually recursive, and require CSE and the planner to look inside +scalars. diff --git a/docs/design_docs/proposals/operator-flattening.md b/docs/design_docs/proposals/operator-flattening.md deleted file mode 100644 index 7d2eea9df..000000000 --- a/docs/design_docs/proposals/operator-flattening.md +++ /dev/null @@ -1,512 +0,0 @@ -# Sharing Operators Between Pre-ASAP IR and Post-ASAP IR - -> Status: proposed, not implemented. Problem statement: -> [#468](https://github.com/ProjectASAP/ASAPPlanner/issues/468). Code -> references and counts are against `main` at `8acb472`. - -**The idea.** Today the post-ASAP-plan is glued together by different operator types. -This proposal keeps one operator language and makes summary operators extra node kinds in it: any relational operator can sit above a summary, and a summary can read any relational subtree. -Nothing is wrapped and nothing is duplicated. - -``` -Today Proposed -ValueOperation(Project) ← a copy Project - SummaryEstimate Ext(SummaryEstimate) - SummaryAgg(Kll) Ext(SummaryAgg(Kll)) - KeepPreAsap(Scan lineitem) ← a black box Scan lineitem -``` - -Concretely: split `QueryExpr` into operators (`OriginalOp`) and scalar -expressions (`ScalarExpr`), make every operator's children `Rc` with -`Operator = Basic(OriginalOp) | Ext(ASAPOp)`, and delete the parallel post-ASAP -types (`SummaryExpr`, `SummaryNode`, `SummarySchema`, and the relational variants -of `ValueOperation`). - -**Roadmap**: - -| Part | Sections | Question answered | -|---|---|---| -| I. New IR | §1 Types, §2 Per-node information | Which node kinds exist, and what each carries | -| II. Upstream and downstream changes | §3 Frontends and entry → §4 Planner → §5 Validation → §6 Export → §7 Other consumers | How each stage changes, in data-flow order | -| III. Implementation plan | §8 Stages and tests, §9 Out of scope | How to land it, and what is deliberately left out | - ---- - -# I. New IR - -## 1. Types - -### 1.1 Overview - -``` -Operator operator: one node of the DAG; every child is Rc> -├─ Basic(OriginalOp) original operator: today's relational QueryExpr variants (Scan / Filter / Join / Aggregate / SetOp …) -└─ Ext(ASAPOp) ASAP operator: summary operators (SummaryAgg / SummaryEstimate …) - -ScalarExpr scalar expression: only appears inside operator fields (predicates, SELECT lists); never a node -``` - -- **Naming**: the suffix says what a type is — `*Op` is an operator, `*Expr` is an - expression. `OriginalOp` holds the operators pre-ASAP already has, as opposed to - the extension `ASAPOp`. The name `QueryExpr` goes away. -- **The generic `C`**: the existing column-reference stage of `QueryExpr` — - `ColumnRef` (by name) out of the frontends, `ColumnId` (by position) after - `resolve_root`. A parent and its children must be in the same stage, so all - three types carry the same `C`. - -### 1.2 `OriginalOp` and `ScalarExpr`: splitting `QueryExpr` - -Today one `QueryExpr` enum holds two kinds of things: - -```sql -SELECT l_quantity * 2 AS q2 FROM lineitem WHERE l_quantity > 10 -``` -``` -Project { cols: [ProjectItem { expr: Arithmetic(Column(4) * Literal(2)) }], ← scalar expression: computes one value per row - child: Filter { pred: Predicate(Compare(Column(4) > Literal(10))), ← scalar expression - child: Scan lineitem } } ← relational operator: rows in, rows out -``` - -`Filter.child` (rows) and the `Compare` inside `Filter.pred` (a value) are both -`QueryExpr`; only the field position tells them apart. The code already knows -which variants are scalars: `output_schema` returns `ScalarHasNoRowSchema` for all -of them (`types/src/pre_asap/query_expr.rs:1447`). Split them into two types: - -```rust -pub enum OriginalOp { - Scan { .. }, Filter { pred: Predicate, child: Rc> }, Project { cols: Vec>, child }, - Aggregate { .. }, Join { .. }, SetOp { .. }, Concat { .. }, Dedup { .. }, Sort { .. }, Limit { .. }, BinaryOp { .. }, - SQLWindowFunc { .. }, TimeRange { .. }, TimeShift { .. }, Promql* { .. }, - ScalarBridge(Rc>), // formerly PromqlScalarBridge: a scalar in operator position (the `2` in PromQL `v * 2`, a bare scalar query) - EvalTimestamp, // PromQL time(): operator position, one row, one column -} -pub enum ScalarExpr { // children are ScalarExpr only - Column(C), Literal(ScalarValue), Compare { .. }, BoolAnd(..), BoolOr(..), Not(..), IsNull(..), IsNotNull(..), - Cast { .. }, InList { .. }, FunctionCall { .. }, Arithmetic { .. }, Case { .. }, CurrentTimestamp, -} -pub struct Predicate(pub Rc>); -pub struct ProjectItem { pub alias: Option, pub expr: ScalarExpr } -``` - -- **Ambiguous variants**: `PromqlScalarBridge`, `EvalTimestamp`, - `PromqlScalarFromVector` and `PromqlVectorFromScalar` appear in operator position - and have a row schema, so they belong to `OriginalOp`. `CurrentTimestamp` (SQL - `NOW()`) is used both ways: it has a row schema (`query_expr.rs:1388`) and takes - part in scalar type inference (`:1674`, e.g. `WHERE ts > NOW()`). It goes into - `ScalarExpr` and is written `ScalarBridge(CurrentTimestamp)` in operator - position. **Open**: confirm how the frontends place `CurrentTimestamp` in - operator position. -- **Benefits**: a scalar in operator position (`Basic(Column(3))`) is no longer - expressible; the `ScalarHasNoRowSchema` runtime error disappears; `canonicalize` - and `pre_asap/cse` already walk only the relational skeleton and treat scalars as - opaque data (see the `pre_asap/cse.rs` module docs) — the types now say so. - -### 1.3 `ASAPOp` - -```rust -pub enum ASAPOp { - SummaryAgg { child: Rc>, family: ASAPType, input, reduction, grouping, // family was SummaryFamilyType, "never Plain" by convention; now by type - guarantee: Option }, // Some only for the ExactAggregate family - SummaryEstimate { child, query: SketchQuery, - guarantee: Option }, // None = the AccuracyModel has no error model for this family - SummaryMerge { children: Vec>>, timing: ExecutionTiming }, - SummarySubtract { left, right }, - SummaryDelete { child, key: C }, // was ColumnRef; now C like everything else - SummaryJoin { outer, inner, key: C, family: ASAPType }, - // The four summary-specific ValueOperation variants, lifted to the top level instead of { child, operation, timing } - FinalizeExactAccumulator { child, timing: ExecutionTiming }, - MaintainPopulation { child, population }, // always ingestion time - ReadPopulation { child, readout }, // always query time - Extension { child, name: String }, -} -impl ASAPOp { pub fn guarantee(&self) -> Option<&ResultGuarantee>; } // only the two variants above store one; see §2.2 -``` - -**Deleted post-ASAP types** — each is expressed by an existing `OriginalOp` variant: - -| Deleted | Expressed as | -|---|---| -| `SummaryExpr::KeepPreAsap(q)` | no wrapper: the subtree `q` is itself `Basic(..)` | -| `ValueOperation::{Project, Filter, Sort, Limit}` | `OriginalOp::{Project, Filter, Sort, Limit}` | -| `SummaryExpr::BinaryOp`, `SummaryExpr::RelationalJoin` | `OriginalOp::BinaryOp`, `OriginalOp::Join` | -| `ValueOperation::Exact(Aggregate)`, `ExactOperation` | `OriginalOp::Aggregate` | -| `SummaryNode` (the node wrapper) | not needed; per-node information is covered in §2 | - -### 1.4 Child slots - -```rust -// Today // After -Filter { Filter { - pred: Predicate(Rc), // scalar pred: Predicate(Rc), - child: Rc, // rows child: Rc, // either Basic(..) or Ext(SummaryEstimate ..) -} } -``` - -**Rule**: every `OriginalOp` field that holds input rows has type `Rc>`. -There are about 25 such fields — `Filter.child`, `Join.left` / `Join.right`, both -sides of `SetOp`, `Concat.children`, `BinaryOp.lhs` / `BinaryOp.rhs`, the `child` of -each `Promql*` operator, and so on. - -**Effect**: an ASAP operator can sit directly under any original operator. Both -sides of a `SetOp`, for example, can be `SummaryEstimate`s. - -**Exception: `Concat.children`**. Today it is `Vec`: branches are stored -by value and have no `Rc` identity. The planner's search and assembly (§4) -identify nodes by `Rc` pointer — targets are registered by pointer and holes are -recognized with `Rc::ptr_eq` — so a `Concat` branch can be neither a target nor a -hole. This affects, for example, the branches SQL `ROLLUP` and PromQL -`histogram_quantiles` are lowered into. As `Vec>` it is handled like -any other child slot and §4 needs no special case (§8 stage 0). - -## 2. Per-node information - -A post-ASAP node carries three pieces of information today. After the change: - -| Field | Meaning | Today | After | -|---|---|---|---| -| `schema` | which columns the node outputs, and their types | stored on every `SummaryNode` | not stored; `output_schema()` computes it on demand (§2.1) | -| `guarantee` | how far the node's output value can be off (metric, bound, failure probability, provenance) | stored on every `SummaryNode` | stored on `SummaryAgg` / `SummaryEstimate`; derived by `GuaranteeIndex` for everything else (§2.2) | -| `timing` | whether the node runs at ingestion time or at query time | stored on the `BinaryOp` / `ValueOperation` / `SummaryMerge` variants | not stored on `OriginalOp` (§5); kept on `SummaryMerge` and `FinalizeExactAccumulator` | - -### 2.1 Schema: two types become one - -A schema is a node's list of output columns: name, type, nullability. There are two today: - -| | pre-ASAP | post-ASAP | -|---|---|---| -| Type | `Schema { columns: Vec, time_index, unique_keys, closed }` | `SummarySchema { fields: Vec, time_index }` | -| Column type | `Column.dtype: DataType` — plain values only (`Int64`, `Float64`, `Utf8`, …) | `SummaryField.dtype: SummaryFamilyType` — a plain value `Plain(DataType)` or summary state (`Sketch(Kll, …)`, …) | -| Where it lives | not stored; `QueryExpr::output_schema()` computes it | stored on every `SummaryNode` | - -In the new IR both kinds of operator share one tree, so `Operator::output_schema()` -must describe any node, and a column type must be able to hold either a plain -value or summary state: - -``` -SummaryEstimate(Quantile .99) → [p99: DataType(Float64)] - SummaryAgg(Kll) → [state: ASAPType(Sketch(Kll, k=269))] - Scan lineitem → [l_orderkey: DataType(Int64), l_quantity: DataType(Float64), …] -``` - -So keep `Schema`, delete `SummarySchema` / `SummaryField`, and give `Column.dtype` a new type, `ColumnType`: - -```rust -pub enum ColumnType { - DataType(DataType), // a plain value; the existing DataType, unchanged (Int64, Float64, Utf8, …) - ASAPType(ASAPType), // ASAP state -} -pub enum ASAPType { // SummaryFamilyType without its Plain variant - ExactAggregate(ExactKind, ExactParams), Sketch(SketchKind, GroupingStrategy), - Sample(SamplingKind, SamplingParams), Wavelet(WaveletKind, WaveletParams), StatModel(StatModelKind, StatModelParams), -} - -pub struct Column { pub name, pub dtype: ColumnType, pub nullable, pub table: Option } -impl Column { - pub fn plain(name, DataType) -> Self; // build a plain column (frontends, catalogs) - pub fn plain_dtype(&self) -> Option<&DataType>; // None for an ASAP-state column - pub fn expect_plain_dtype(&self) -> &DataType; // frontends and scalar type inference: a state column is a bug, panic -} -``` - -- **Naming**: as in §1, `DataType` / `ASAPType` mirror `OriginalOp` / `ASAPOp`. - The name `SummaryFamilyType` goes away: once every column carries it, a plain - column reading `SummaryFamilyType::Plain(Float64)` is a misnomer. With two - levels, "is this column state?" is a check on the outer variant only — exactly - the split `check_plain_operands` needs. -- **`DataType` itself is unchanged.** It remains part of the scalar type system - (the type of a `Literal`, the result type of `Arithmetic`, catalog column - declarations); it now sits inside `ColumnType::DataType(..)`. Code that reads - `Column.dtype` expecting a `DataType` unwraps one more level: code that only - ever sees plain columns (frontends, scalar type inference) uses - `expect_plain_dtype()`; code that may see state (planner, validation, export) - uses `plain_dtype()` or matches. A scalar expression never legitimately reads a - state column — `check_plain_operands` (§5) rejects such a DAG first. -- **Elements of nested types**: `DataType::List { element: Box }` and - `DataType::Struct { fields: Vec }` have `Column` elements. With - `Column.dtype: ColumnType` an element could become ASAP state (a "list of KLL - sketches"), which is not intended. These two use a plain-only field instead: - - ```rust - pub struct PlainField { pub name: String, pub dtype: DataType, pub nullable: bool } // nested fields are already unqualified (List's doc comment), so no table - DataType::List { element: Box } - DataType::Struct { fields: Vec } - ``` - - `DataType::Map { key: Box, value: Box, .. }` already uses - `DataType` only and is unchanged. There are 36 construction or match sites of - `DataType::{List, Struct, Map}`. -- **Why keep `Schema`**: `ColumnType::DataType(..)` represents every plain type, so - nothing is lost, and `Schema` additionally has `unique_keys` / `closed` / - `Column.table`, which merging the other way would drop. -- **Computed on demand**: schemas are no longer stored on nodes; `Operator::output_schema()` - computes them. `OriginalOp` keeps today's `QueryExpr::output_schema()` logic, and - `ASAPOp` follows the table below. These rules are currently spread over - `asap-aware-mapping/src/replacement.rs` and `types/src/post_asap/execution_data_state.rs`; - they move into `asap-types`. -- **Deleted conversions**: `lift()` (`replacement.rs:3352`) and `lift_plain` - (`execution_data_state.rs:725`), which turn a `Schema` into a `SummarySchema`, and - the reverse `plain_schema` (`execution_data_state.rs:707`). With one schema type - there is nothing to convert. -- **Size of the change**: 7 direct `Column { .. }` literals; the deleted - `SummarySchema {}` (36) and `SummaryField {}` (19) literals. In non-test code - `SummaryFamilyType` appears 197 times (42 of them with `Plain(`) and `.dtype` is - read at 67 sites; all are rewritten against the new types. - -| `ASAPOp` | Output schema | -|---|---| -| `SummaryAgg` | the grouping columns as-is + one `ASAPType(family)` column | -| `SummaryEstimate` | the row schema determined by the `SketchQuery` | -| `SummaryMerge` / `Subtract` / `Delete` / `Join` | one `ASAPType(family)` column | -| `FinalizeExactAccumulator` | the child's schema, with each `ASAPType(ExactAggregate ..)` column turned into the matching `DataType(..)` | -| `MaintainPopulation` / `ReadPopulation` | existing implementation | - -### 2.2 Guarantee: some stored, some derived - -| Node | Where its guarantee comes from | -|---|---| -| `SummaryAgg`, `SummaryEstimate` | **stored in a field**. Computed at binding time by the `AccuracyModel` from the summary's parameters and the evidence; it cannot be recovered afterwards, and selection needs it during search to decide whether a candidate meets the accuracy target (`replacement.rs:5831`) | -| `OriginalOp` (`Project`, `Join`, …), `FinalizeExactAccumulator` | composed from the children by rule (`relational_join_guarantee`; `exact_operation_rule` in `accuracy/composition.rs`). A subtree with no `Ext` descendant is exact | -| `MaintainPopulation`, `ReadPopulation` | always exact | -| `SummaryMerge` / `Subtract` / `Delete` / `Join` | output state, so no guarantee of their own — but they change the error of a later readout (see below) | - -The derived part goes into one table: - -```rust -/// One traversal, keyed by node pointer — the same approach as the ExecutionDataStateAssignment -/// returned by validate_execution_data_states. -pub struct GuaranteeIndex(HashMap<*const Operator, ResultGuarantee>); -pub fn guarantee_index(root: &Rc) -> GuaranteeIndex; -``` - -- The composition rules also depend on the `AccuracyModel`, so the planner must - compute `GuaranteeIndex` with the same model and hand it over together with the - assembled root (`assemble_selected_dag`). `compile_executable_dag` uses it to fill - `ExecutableDagNode.guarantee`. -- Not chosen: wrapping every node to store a guarantee — frontends and `resolve` - would then have to build the wrapper too. -- **Gap for state nodes**: `SummaryMerge` / `Subtract` / `Delete` / `Join` change the - error of a later readout. For a CMS, `a − b` has error bound `ε·(‖a‖₁ + ‖b‖₁)`, so - the relative error is large when `a` and `b` are close. A readout's guarantee is - computed today as if its input were a sketch built directly by a `SummaryAgg`, - ignoring any intermediate operation. The planner on `main` never emits these four - nodes (they are built only in tests, and `SummaryMerge` is documented as inserted by - a deployment), so the gap is latent. Not addressed here (§9). - ---- - -# II. Upstream and downstream changes - -## 3. Frontends and the optimizer entry - -Trees built by the frontends and by `resolve` contain only `Basic`. They access -children through `expect_basic()`; before a tree reaches the optimizer, the entry -checks it once and rejects any `Ext`: - -```rust -impl Operator { - pub fn contains_ext(&self) -> bool; - pub fn expect_basic(&self) -> &OriginalOp; // an Ext here is a bug: panic -} -``` - -**Where the check goes**: on `main` the optimizer entries are `search_workload` / -`search_workload_with` / `search_workload_with_targets`. They do not return -`Result`, so the check starts as `assert!(!root.contains_ext())` at each entry. If -the single `ParsedWorkload` entry point (#429) lands on `main` first, the check -moves into `ParsedWorkload::new` and returns an error (it already checks the entry -count). - -**Why not a compile-time guarantee** (an associated type `Ext` on `ColState`, with -`ColumnRef::Ext = Never`): - -| | Compile time | Run time (this proposal) | -|---|---|---| -| Types | one more associated type on `ColState`, plus an empty enum | the `Ext` variant holds `ASAPOp` directly | -| What it catches | only frontend code before `resolve`. A frontend already returns a `ColumnId` tree, where `Ext` is allowed | **every input to the optimizer**: frontend code after `resolve`, deserialized plans, third-party frontends, hand-written test IR | -| Frontends matching children | `basic()` is total | `expect_basic()`, which can in principle panic | - -The compile-time version protects only a small stretch inside the frontends; the -runtime check sits at the entry, covers more, and keeps the types simpler. So one -`Operator` type serves both pre- and post-ASAP, told apart by the entry check -rather than by a type parameter. - -## 4. Planner: search and assembly - -**Candidates**: three shapes become two. - -```rust -pub enum Replacement { - Subtree(Rc), // formerly Summary(Rc) and Rewrite(Rc); "introduces a summary" = contains_ext() - ExactComposition { .. }, // unchanged, except its plan becomes Rc instead of Rc -} -``` - -**Holes**: a subtree of a candidate that is `Rc::ptr_eq` to some target is a hole, -left for that target's own choice to fill. A candidate that needs a specific -implementation of a child (for example a `SummaryAgg` that needs an exact -accumulator value, as in `realize_temporal_average`) inlines the child as a new node; -the pointer differs, so it is not a hole. `realize_child_with` changes from -"realize the child recursively, else `keep_pre_asap`" to "else leave a hole". - -**Assembly**: one generic rule replaces `assemble_residual`. - -```rust -fn assemble(&self, t: &Rc) -> Rc { - memo by ptr; // a shared child stays one Rc — today's assembled_nodes - let body = match self.chosen(t) { - Some(Subtree(r)) => r, // Rewrites are assembled further down too (today replacement.rs:4571 wraps the whole rewrite in keep_pre_asap and ignores the choices below) - Some(ExactComposition{..}) => composition.plan, - None => t, // no candidate chosen: keep the node, recurse into its children — what assemble_residual does for only four operators - }; - body.map_children(|c| if is_target(c) { self.assemble(c) } else { c }) -} -``` - -After assembly, run the data-state validation (§5) once; if filling a hole is -illegal (e.g. a query-time `SummaryEstimate` under a `SummaryAgg`), that hole falls -back to its original subtree. This generalizes the fallback `relink_summary` does -today for `Aggregate` only. Finally compute `GuaranteeIndex` (§2.2). - -- **`map_children`**: reuse `rebuild_children` from `pre_asap/cse.rs:576` as - `Operator::map_children`, dispatching `Basic` to `OriginalOp::map_children` and - `Ext` to `ASAPOp::map_children`. -- **Deleted**: `assemble_residual`, `relink_summary`, the `query_time_nested_sum` / - `contains_aggregate` special cases, `keep_pre_asap` / `keep_pre_asap_rc`, and the - branch of `finalize_exact_accumulator` that builds an empty schema for `KeepPreAsap`. - -How the three problems in #468 are resolved: - -| Problem | Resolution | -|---|---| -| 1. One operator, two spellings: a `Project` is a `QueryExpr` inside `KeepPreAsap` and a `ValueOperation` outside it | one set of types; `assemble` no longer builds `ValueOperation`s | -| 2. `KeepPreAsap` is opaque: nothing outside can reference the `Scan` inside, so an exact aggregate and a sketch cannot share one scan | `Scan` is no longer wrapped; `SummaryAgg{child: scan}` and `Aggregate{child: scan}` can point to the same `Rc`. Splitting a multi-measure `Aggregate` into `avg` + KLL is a binding-rule change; this proposal only makes the split expressible | -| 3. `SetOp` and similar operators have no post-ASAP counterpart and must stay inside `KeepPreAsap`, so no subtree below them can use a summary | `SetOp` takes the `None => t` path and both children are assembled independently | - -## 5. Validation: execution data states - -Today's rule `produced_data_state(KeepPreAsap) = None` (decided by the edge that -reaches the node) extends to every `OriginalOp`: - -| Node | Produced state | -|---|---| -| `OriginalOp` | none of its own; assigned by the consuming edge (`QUERY_ROWS` at the root) and passed down unchanged | -| `SummaryAgg` | `{the child's timing, SummaryState}`; ingestion time when the child is an `OriginalOp` | -| other `ASAPOp` | unchanged | - -The `KeepPreAsap` / `BinaryOp` / `ValueOperation` / `RelationalJoin` arms of -`validate_execution_data_states` merge into one `Basic` arm: - -- Pass the node's state to each child; an `Ext` child is checked against the - `Ext` edge rules (e.g. `SummaryEstimate` only on the query side). -- `check_plain_operands` stays: every column an `OriginalOp` references must be - `ColumnType::DataType`; `Project` / `Filter` / `Sort` / `Limit` may pass - `ExactAggregate` columns through (today's exception). This is what rejects - `Project(Ext(SummaryAgg))`, i.e. projecting sketch state as if it were rows. -- The extra ingestion-side constraints on `BinaryOp` (arithmetic only, identical - schemas on both sides, exactly one `Float64`) move into this arm, per variant. -- `AmbiguousKeepPreAsap` is renamed `AmbiguousDataState`; its meaning is unchanged. - -## 6. Export: fragments in the executable DAG - -`ExecutableDag` has four payloads for original operators today — `fallback{expression: QueryExpr}`, -`binary`, `value` and `relational_join`. Afterwards there is one: - -```rust -ExecutableOperatorPayload::Relational { - /// An Operator with no Ext (checked at construction). Leaves are real Scans, or - /// Scan { source: Source::DagInput { role } } standing for an incoming edge whose - /// schema is that edge's intermediate_schema. - expression: Operator, -} -``` - -`Source` gains a variant `DagInput { role: EdgeRole }`. `compile_executable_dag` works as follows: - -1. Starting from the root, take the **largest connected subtree containing no - `Ext`** as one fragment node; wherever the fragment meets an `Ext`, cut an edge - and put a `DagInput` leaf in the fragment. -2. Each `Ext` node maps one-to-one onto the existing `summary_agg` / - `summary_estimate` / … payloads; `FinalizeExactAccumulator` / `MaintainPopulation` / - `ReadPopulation` / `Extension` become `value{operation}`. `ValueOperation` stays as a - wire type with only these four variants. -3. Today's `fallback` is a fragment with no `DagInput` leaf. A backend's existing - path that recursively lowers a `QueryExpr` handles every fragment after unwrapping - one `Basic` level and adding a `DagInput` arm (treat the incoming edge as a - materialized table); the dedicated lowerings for `binary` / `value::Project` / - `relational_join` can go. - -- **Wire version 5 → 6**: three fewer payloads; `fallback` is renamed `relational` - and allows `DagInput` leaves; `output_schema` / `intermediate_schema` change from - `SummarySchema` to `Schema` (adding `unique_keys` / `closed` / `table`). -- **Phase granularity**: ingestion / query phase is now one per fragment, and - `with_execution_phases` still assigns it per node. Switching phase inside a - fragment would need a materialization point, and materialization points are - exactly `Ext` nodes (`FinalizeExactAccumulator`, `MaintainPopulation`), so no real - granularity is lost. - -## 7. Other consumers - -| Location | Change | -|---|---| -| `post_asap/cse.rs` | delete. `ASAPOp` derives `PartialEq` + serde, so `share_common_subtrees` in `pre_asap/cse.rs` covers it | -| `dag_export.rs` | delete `build_summary` / `build_summary_hybrid` / `summary_kind_tag`; the one exporter gains an `Ext` arm. The viewer's `node-style.js` drops `KeepPreAsap` / `SummaryBinaryOp` / `ValueOperation` / `RelationalJoin` and adds the `ASAPOp` variant names; the pin test at `dag_export.rs:1843` follows | -| `summary_maintenance_cost/estimator.rs` (the largest, 80 sites) | the `KeepPreAsap` branches through `query_source_selections` / `retained_queries` switch to "largest subtree with no `Ext`" (exactly the §6 fragment); `BinaryOp \| RelationalJoin => "exact_binary"` and `ValueOperation => "value_operation"` fold into the fragment cost. This also fixes the missing `RelationalJoin` arm at `evidence.rs:292`, which can raise `InconsistentOperatorStatistics` | -| `physical_plan_cost_model.rs::estimate_candidate` | `Summary(KeepPreAsap(q))` and `Rewrite(q)` already share `lower_query_physical_dag`; every fragment will go through it | -| `summary_maintenance_lifecycle.rs` | `selected_raw_recompute = matches!(root, KeepPreAsap)` becomes `!contains_ext(root)`; `keep_pre_asap(target)` at `:551` becomes `target` itself | -| `maintained_population.rs` | `KeepPreAsap(source)` becomes `source` itself; `MaintainPopulation`'s `population.matches_input(child)` inspects a `Basic` child directly | -| `exact_composition.rs` | the `ExactOperation::Aggregate` shell becomes a `Basic(Aggregate)` node whose `child` is a hole | -| `RelationalJoin.pruning` | production code never sets `Some`; delete it. Candidate pruning can return later as an `ASAPOp` variant; the `CandidateCompleteness` type stays | - ---- - -# III. Implementation plan - -## 8. Stages and tests - -`main` builds and passes all tests after every stage. - -| Stage | Content | Main changes | -|---|---|---| -| 0 Preparation | `Rc` for `Concat.children`; `rebuild_children` becomes `map_children`; add `Column::plain` | `asap-types`, mechanical | -| 1 Split operators and expressions | §1.2: split `QueryExpr` into `OriginalOp` + `ScalarExpr`; `Predicate` / `ProjectItem` etc. hold `ScalarExpr`; relational children stay `Rc` for now. No semantic change | every place that builds or matches scalars: the three frontends' expression lowering, column resolution in `resolve`, scalar rewrites in `canonicalize`, `scalar_signature.rs`, `column_resolution.rs`. Mechanical | -| 2 Two levels | §1.1, §1.4: add `Operator`, `ASAPOp` (a placeholder with no producer yet), `contains_ext()` and `expect_basic()`; every relational child slot becomes `Rc>`. No semantic change | every place that builds or matches children: the three frontends, `resolve`, `canonicalize`, `pre_asap/cse`, `output_schema`, `dag_export`, all of `asap-aware-mapping`. Each change is the same shape; afterwards the remaining work is local | -| 3 One schema | §2.1: `SummarySchema` → `Schema`, `SummaryFamilyType` → `ColumnType` + `ASAPType`, `PlainField` for `List` / `Struct` elements; delete `lift*` | all of `asap-types` plus schema construction in every crate. The wire format changes only in schema fields; no version bump yet | -| 4 New types alongside old | fill in the `ASAPOp` variants; add the `Ext` arms of `output_schema` and data-state validation (§5), `GuaranteeIndex` (§2.2) and the optimizer-entry check (§3); write `flatten(&SummaryNode) -> Rc`; `compile_executable_dag` and `dag_export` consume the flat tree (planner output goes through `flatten` first). Wire → 6 | `asap-types`, `devtools`, viewer. The planner is untouched, but export and execution already run on the new types | -| 5 Planner switch | §4: candidates and assembly results become `Rc`, `assemble` is rewritten; the cost / lifecycle / estimator code in §7 moves to the new types; delete `flatten` | `asap-aware-mapping`; the largest stage | -| 6 Cleanup | delete `SummaryExpr` / `SummaryNode` / the redundant `ValueOperation` variants / `ExactOperation` / `post_asap/cse.rs`; update `post-asap-ir.md`, `physical-plan-integration.md`, the developer guide and the viewer docs | mostly docs | - -An alternative transition would first make `ValueOperation` wrap a pre-ASAP operator -without changing the structure. Its benefit — export and execution see a flat -structure early — is already delivered by stage 4, which is built on the final types -and so is not throwaway code. That transition is therefore not planned. - -**Tests**: - -- One integration test per problem in §4, asserting the shape of the assembled plan: - 1. `WITH metric AS (SELECT avg(CASE WHEN l_quantity BETWEEN 1 AND 50 THEN 1.0 ELSE 0.0 END) AS in_range FROM lineitem) SELECT in_range, in_range = 1.0 AS ok FROM metric`: - no post-ASAP-only node other than `Ext`, and all three `Project`s are the same variant; - 2. `SELECT avg(l_extendedprice), approx_percentile_cont(l_discount, 0.99) FROM lineitem`: - the `child` of the `avg` `Aggregate` and of the KLL `SummaryAgg` is the same `Scan` by `Rc::ptr_eq` (enabled once a binding rule splits measures); - 3. `SELECT approx_distinct(l_partkey) FROM lineitem UNION ALL SELECT approx_distinct(l_suppkey) FROM lineitem`: - each side of the `SetOp` has a `SummaryEstimate`. -- The optimizer entry rejects a tree containing `Ext`. -- Rewrite the 117 `SummaryExpr::` assertions in `sql_to_post_asap.rs` / - `promql_to_post_asap.rs` / `exact_composition.rs` against the new shape. -- Wire 6 round trip: a fragment with a `DagInput` leaf is equal after serialization - and deserialization; a version-5 document is rejected by `deny_unknown_fields`. -- Keep the shapes of the 52 existing tests in `execution_data_state.rs`; only their - construction changes. - -## 9. Out of scope - -- The binding rule that splits a multi-measure `Aggregate` into "exact + summary" - sharing one child (the second half of problem 2 in §4). -- Candidate pruning as an `ASAPOp` variant. -- **Accuracy of summary state** (§2.2): making a readout's guarantee account for - `SummaryMerge` / `Subtract` / `Delete` / `Join`. Either attach an accuracy - descriptor to state (e.g. "ε relative to the L1 norm"), or compose the error along - the state chain at readout. To be done once the planner starts emitting these nodes. -- Folding `ExactComposition` into `Subtree`. Semantically it equals an `Aggregate` - fragment (query time) or `Finalize(SummaryAgg{ExactAggregate})` (ingestion time), - both expressible afterwards; the cost machinery of `composition_plans` and - `CompositionDecision` stays as is until stage 5 is stable. diff --git a/docs/design_docs/proposals/operator-sharing.md b/docs/design_docs/proposals/operator-sharing.md new file mode 100644 index 000000000..4ee165901 --- /dev/null +++ b/docs/design_docs/proposals/operator-sharing.md @@ -0,0 +1,428 @@ +# Sharing Operators Between Pre-ASAP IR and Post-ASAP IR + +> Status: proposed, not implemented. Problem statement: +> [#468](https://github.com/ProjectASAP/ASAPPlanner/issues/468). Implementation +> starts after the single entry point and the pluggable pass (#429, #430) land on +> `main`. Builds on [Decoupling operators from scalar expressions](decoupling_op_and_expr.md) +> (same PR), which splits `QueryExpr` into `NonASAPOp` and `ScalarExpr`. Code is +> referenced by file and function; counts are approximate, measured on `main` at `8acb472`. + +**The idea.** Today a post-ASAP plan is glued together from two sets of operator types. +This proposal keeps one operator language and makes summary operators extra node kinds in it: any relational operator can sit above a summary, and a summary can read any relational subtree. +Nothing is wrapped and nothing is duplicated. + +``` +Today Proposed +ValueOperation(Project) ← a copy NonASAP(Project) + SummaryEstimate ASAP(SummaryEstimate) + SummaryAgg(Kll) ASAP(SummaryAgg(Kll)) + KeepPreAsap(Scan lineitem) ← a black box NonASAP(Scan lineitem) +``` + +| Part | Sections | +|---|---| +| I. New IR | §1 Types, §2 Per-node information | +| II. Changes, in data-flow order | §3 Entry → §4 Planner → §5 Timing → §6 Export → §7 Other consumers | +| III. Implementation | §8 Stages and tests, §9 Out of scope, §10 Open questions | + +--- + +# I. New IR + +## 1. Types + +### 1.1 Overview + +Operator attributes differ in how widely they apply. Each is defined at the level +that matches its breadth: + +| Applies to | Examples | Defined as | +|---|---|---| +| every operator | children, schema, timing, guarantee | methods of `Operator`; the values may be stored per variant | +| one category | for all `NonASAP` operators, timing is derived from the consuming edge, and guarantee from the children | a variant of `Operator` | +| one operator | `Aggregate.measures`, `SummaryAgg.family` | fields of that variant | + +```rust +pub enum Operator { // category + NonASAP(NonASAPOp), // today's relational QueryExpr variants (§1.2) + ASAP(ASAPOp), // summary operators (§1.3) +} + +impl Operator { // every operator + pub fn children(&self) -> Vec<&Rc>>; + pub fn map_children(&self, f: impl FnMut(&Rc>) -> Rc>) -> Self; + pub fn output_schema(&self) -> Result; // computed (§2.1) + pub fn timing(&self) -> &Slot; // §2.3 + pub fn guarantee(&self) -> &Slot>; // §2.2; Set(None): unknown, never read as exact + pub fn with_timing(&self, timing: ExecutionTiming) -> Self; + pub fn with_guarantee(&self, guarantee: Option) -> Self; +} +pub enum Slot { Unset, Set(T) } + +pub fn derive_guarantees(root: &Rc, model: &dyn AccuracyModel, + evidence: &dyn AccuracyEvidenceProvider) -> Result, AccuracyError>; // §2.2 +pub fn derive_timings(root: &Rc) -> Result, ExecutionDataStateError>; // §2.3, §5 +``` + +### 1.2 `NonASAPOp` + +`NonASAPOp` and `ScalarExpr` come from splitting `QueryExpr` +([decoupling doc](decoupling_op_and_expr.md#2-types)). Here `NonASAPOp` becomes the +non-ASAP category of `Operator`: its child slots widen from `Rc` to +`Rc` (§1.4), and every variant gets the `timing` / `guarantee` slots (§1.1). +Scalar fields are unchanged; only `NonASAPOp` holds them. + +```text +Operator +├─ NonASAP(NonASAPOp) +│ ├─ children: Rc> → back to Operator: NonASAP or ASAP +│ ├─ timing / guarantee slots +│ └─ scalar fields: Predicate / ProjectItem / SortKey / ScalarBridge / ... +│ └─ ScalarExpr: never contains an Operator +└─ ASAP(ASAPOp) + ├─ children: Rc> → back to Operator: NonASAP or ASAP + └─ timing / guarantee slots +``` + +### 1.3 `ASAPOp` + +```rust +pub enum ASAPOp { + SummaryAgg { child: Rc>, family: ASAPType, input, reduction, grouping, + exact_rule: Option }, // ExactAggregate only, §2.2 + SummaryEstimate { child, query: SketchQuery, + local_guarantee: Option }, // §2.2 + SummaryMerge { children: Vec>> }, + SummarySubtract { left, right }, + SummaryDelete { child, key: C }, // key was ColumnRef + SummaryJoin { outer, inner, key: C, family: ASAPType }, + // the summary-specific ValueOperation variants, lifted to the top level + FinalizeExactAccumulator { child }, + MaintainPopulation { child, population }, // always ingestion time + ReadPopulation { child, readout }, // always query time + Extension { child, name: String }, +} +``` + +`family` was the original `SummaryFamilyType`. + +Every variant also carries the `timing` and `guarantee` slots (§1.1), omitted above. + +**Unexercised variants**: `SummaryMerge`, `SummarySubtract`, `SummaryDelete`, `SummaryJoin` +and `Extension` are built only in tests today. They are migrated, but all their methods +return `Unimplemented`. + +Following table shows how some legacy types get expressed in the new framework. + +| Legacy types | Expressed as | +|---|---| +| `SummaryExpr::KeepPreAsap(q)` | `q` itself, an `NonASAP(..)` subtree | +| `ValueOperation::{Project, Filter, Sort, Limit}` | `NonASAPOp::{Project, Filter, Sort, Limit}` | +| `SummaryExpr::{BinaryOp, RelationalJoin}` | `NonASAPOp::{BinaryOp, Join}` | +| `ValueOperation::Exact(Aggregate)`, `ExactOperation` | `NonASAPOp::Aggregate` | +| `SummaryNode` | `Operator` itself: `schema` is computed, `timing` / `guarantee` are slots on every variant (§2) | + +### 1.4 Child field + +Non-ASAP operators now sit on the same level as ASAP operators, so their children must +be `Rc` to allow free placement: + +```rust +// After the decoupling doc // After this proposal +Filter { pred: Predicate(Rc), Filter { pred: Predicate(Rc), + child: Rc } child: Rc } // NonASAP(..) or ASAP(SummaryEstimate ..) +``` + +Now an original operator can also sit on ASAP operators, e.g. a `SetOp` sitting on two `SummaryEstimate` operators. + +`Concat.children` is `Vec` today: branches are stored by value and have no `Rc` identity, so the planner (§4), which identifies targets and holes by pointer, +cannot replace a branch — e.g. the branches of SQL `ROLLUP` or PromQL `histogram_quantiles`. It becomes `Vec>` (§8 stage 0). + +## 2. Per-node information + +Besides children (§1.4), every operator has the three attributes below: the +every-operator level of §1.1. Each row says where the value lives and who supplies it. + +| Field | Meaning | Today | After | +|---|---|---|---| +| `schema` | output columns and their types | stored on every `SummaryNode` | obtained by `output_schema()` (§2.1) | +| `guarantee` | how far the output value can be off | stored on every `SummaryNode` | obtained by `guarantee()`: derived for every node; binding stores only a sketch's own error, as a `SummaryEstimate` field (§2.2) | +| `timing` | ingestion time or query time | on `BinaryOp` / `ValueOperation` / `SummaryMerge` | obtained by `timing()`: set by binding and lifecycle (`SummaryAgg`) or the planner (`FinalizeExactAccumulator`); derived for the rest (§2.3) | + +### 2.1 Schema: fused into one type + +Today pre-ASAP uses `Schema { columns: Vec, time_index, unique_keys, closed }` +with `Column.dtype: DataType` (plain values only), and post-ASAP stores a +`SummarySchema { fields: Vec, time_index }` on every node, with +`SummaryField.dtype: SummaryFamilyType` (`Plain(DataType)` or summary state). One +tree now needs one schema type whose columns can be either: + +``` +SummaryEstimate(Quantile .99) → [p99: DataType(Float64)] + SummaryAgg(Kll) → [state: ASAPType(Sketch(Kll, k=269))] + Scan lineitem → [l_orderkey: DataType(Int64), l_quantity: DataType(Float64), …] +``` + +We merge semantics of the above two types into one `Schema` type. +The struct of `Schema` stays, with two changes: + +- `Column` is renamed `Field`, and `Schema.columns` `Schema.fields`: the struct describes + a column and holds none of its data. Arrow and DataFusion use the same names. +- `Field.dtype` widens from `DataType` to an enum `FieldType`, so a field can describe either a + plain value or ASAP state. + +`SummarySchema` / `SummaryField` are then redundant and deleted: + +```rust +pub enum FieldType { DataType(DataType), ASAPType(ASAPType) } +pub enum ASAPType { // SummaryFamilyType without Plain + ExactAggregate(ExactKind, ExactParams), Sketch(SketchKind, GroupingStrategy), + Sample(SamplingKind, SamplingParams), Wavelet(WaveletKind, WaveletParams), StatModel(StatModelKind, StatModelParams), +} +pub struct Schema { pub fields: Vec, pub time_index, pub unique_keys, pub closed } +pub struct Field { pub name, pub dtype: FieldType, pub nullable, pub table: Option } +impl Field { + pub fn plain(name, DataType) -> Self; + pub fn plain_dtype(&self) -> Option<&DataType>; // None for a state column + pub fn expect_plain_dtype(&self) -> &DataType; // frontends, scalar type inference; panics on state +} +``` + +Today every `SummaryNode` stores its schema, built at construction. After, every node +computes it with `output_schema()`: + +| Node | Today | After | +|---|---|---| +| `NonASAPOp` | `QueryExpr::output_schema()` lifted to `SummarySchema` (`KeepPreAsap`), or stored on the `ValueOperation` / `BinaryOp` / `RelationalJoin` copy | today's `QueryExpr::output_schema()` logic | +| `SummaryAgg` | the replaced `Aggregate`'s output with the measure column retyped to `family` | grouping columns + one `ASAPType(family)` column | +| `SummaryEstimate` | the replaced operator's output schema | the child's grouping columns + the value columns of the `SketchQuery` | +| `FinalizeExactAccumulator` | the logical operator's output, lifted | the child's schema, `ASAPType(ExactAggregate ..)` columns turned into `DataType(..)` | +| `MaintainPopulation` / `ReadPopulation` | the source's schema / the replaced aggregate's output | the same rules, computed from the child and the `readout` | +| unexercised variants | one field typed `family` | unimplemented (§1.3) | + +### 2.2 Guarantee: always derived + +| Node | Today | After | +|---|---|---| +| `SummaryEstimate` | stored at binding: the sketch's own error composed with the child's (`compose_guarantee`) | **derived**: `local_guarantee` composed with the child's. `local_guarantee` is a field set at binding: the sketch's error over an exact input, `None` when the model has no error model for the family | +| `SummaryAgg` | stored: ExactAggregate family composed with the child's; sketch families `None` | **derived**: ExactAggregate family: exact, composed with the child's under `exact_rule`; sketch families `Set(None)`, state has no guarantee | +| `NonASAPOp` | `KeepPreAsap`: exact; the `ValueOperation` / `BinaryOp` / `RelationalJoin` copies: composed at construction | **derived**: composed from the children (`relational_join_guarantee`, `exact_operation_rule`); exact if no `ASAP` descendant | +| `FinalizeExactAccumulator` | copies the child's | **derived**: the child's | +| `MaintainPopulation` / `ReadPopulation` | stored: exact | **derived**: exact | +| unexercised variants | `None`: state has no guarantee of its own | unimplemented (§1.3) | + +- **Only the local part is stored.** Today binding stores the composed value, built + from the child it sees. Assembly can fill that child's hole with a different plan, and + a stored composition would go stale (`relink_agg_child` copies it today, relying on the + new child being exact). +- `derive_guarantees` uses the same `AccuracyModel` as binding, and the evidence for + `propagate`'s `PropagationStats`. It reads only the subtree, so search runs it on a + candidate to check its accuracy target (the candidate filter in + `search_workload_with_targets`). +- The value travels with the node through cloning, CSE and serialization, as + `SummaryNode.guarantee` does today. + +### 2.3 Timing: set where position does not decide it + +| Node | Today | After | +|---|---|---| +| `NonASAPOp` | `KeepPreAsap`: from the consuming edge; the `ValueOperation` / `BinaryOp` copies: a stored field | **derived** from the consuming edge (§5) | +| `SummaryAgg` | from the child; ingestion time under `KeepPreAsap` | **set**: binding sets `QueryTime`; a lifecycle decision may change it to `IngestionTime` | +| `FinalizeExactAccumulator` | a stored field, set by the planner | **set** by the planner: the same position allows either time | +| `SummaryEstimate` | query time, fixed by the kind | **derived** from the kind: query time | +| `MaintainPopulation` / `ReadPopulation` | a stored field, always ingestion / query time | **derived** from the kind: ingestion / query time | +| unexercised variants | `SummaryMerge`: a stored field; `Join` / `Subtract` / `Delete`: ingestion time | unimplemented (§1.3) | + +Unlike a guarantee, a timing depends on the parents, so `derive_timings` needs the whole +DAG and runs only after assembly. + +--- + +# II. Changes, in data-flow order + +## 3. Optimizer entry + +Frontends and `resolve` build `NonASAP` trees only and access children with +`expect_non_asap()`. `ParsedWorkload::new` rejects a tree that `contains_asap()`, +next to its existing entry-count check: + +```rust +impl Operator { + pub fn contains_asap(&self) -> bool; + pub fn expect_non_asap(&self) -> &NonASAPOp; // an ASAP node here is a bug: panic +} +``` + +A compile-time alternative — an associated type on `ColState` with +`ColumnRef::ASAP = Never` — only protects frontend code before `resolve`: frontends +already return `ColumnId` trees, where `ASAP` is allowed. The entry check covers every +input (frontends after `resolve`, deserialized plans, third-party frontends, test IR) +with simpler types. + +## 4. Planner: search and assembly + +```rust +pub enum Replacement { + Subtree(Rc), // formerly Summary(Rc) and Rewrite(Rc) + ExactComposition { .. }, // its plan becomes Rc +} +``` + +**Holes**: a subtree of a candidate that is `Rc::ptr_eq` to a target is filled by that +target's own choice. A candidate needing a specific child implementation (e.g. +`realize_temporal_average`) inlines a new node, so it is not a hole. +`realize_child_with` falls back to leaving a hole instead of `keep_pre_asap`. + +**Assembly** — one rule replaces `assemble_residual`: + +```rust +fn assemble(&self, t: &Rc) -> Rc { + memo by ptr; // shared children stay one Rc + let body = match self.chosen(t) { + Some(Subtree(r)) => r, // rewrites are assembled further down too; today assemble_target wraps them in keep_pre_asap + Some(ExactComposition{..}) => composition.plan, + None => t, // keep the node, recurse — assemble_residual does this for four operators only + }; + body.map_children(|c| if is_target(c) { self.assemble(c) } else { c }) +} +``` + +Then run `derive_timings` (§5) once: + +- **Illegal fill** (e.g. a query-time `SummaryEstimate` under a `SummaryAgg`): + `derive_timings` returns an error, and the hole falls back to its original subtree — + `relink_summary`'s fallback, for every operator. +- **A shared subtree read at two timings**, e.g. a query-time `Aggregate` and an + ingestion-time `SummaryAgg` reading one `Scan`: both choices are legal, only the sharing + is not. `derive_timings` memoizes by (pointer, timing), so it builds one copy per + timing; a subtree read at one timing stays one `Rc`. + +Finally run `derive_guarantees` (§2.2). `map_children` is `rebuild_children` from +`pre_asap/cse.rs`, dispatching to `NonASAPOp::map_children` / `ASAPOp::map_children`. +Deleted: `assemble_residual`, `relink_summary`, the `query_time_nested_sum` / +`contains_aggregate` special cases, `keep_pre_asap` / `keep_pre_asap_rc`, and the +`KeepPreAsap` branch of `finalize_exact_accumulator`. + +| #468 problem | Resolution | +|---|---| +| 1. A `Project` is a `QueryExpr` inside `KeepPreAsap` and a `ValueOperation` outside | one set of types | +| 2. Nothing outside `KeepPreAsap` can reference the `Scan` inside, so an exact aggregate and a sketch cannot share a scan | `Aggregate` and `SummaryAgg` can point to the same `Scan`. This holds when both run at the same time — always, without lifecycle decisions; otherwise the scan is duplicated (above). Splitting a multi-measure `Aggregate` into exact + sketch is a binding rule, out of scope (§9) | +| 3. `SetOp` and similar have no post-ASAP copy, so no summary below them | `SetOp` takes `None => t`; both children are assembled | + +## 5. Timing: execution data states + +`validate_execution_data_states` becomes `derive_timings`: instead of returning the +`ExecutionDataStateAssignment` side table (deleted), it writes each node's `timing` slot. +`produced_data_state(KeepPreAsap) = None` (set by the consuming edge) extends to all +`NonASAPOp`s: + +| Node | Produced state | +|---|---| +| `NonASAPOp` | set by the consuming edge (`QUERY_ROWS` at the root), passed to its children | +| `SummaryAgg` | `{timing, SummaryState}` from its `timing` slot (§2.3); a `NonASAPOp` child takes the same timing | +| other `ASAPOp` | unchanged | + +A data state is the `timing` slot plus a primitive (`Raw` / `SummaryState` / …) fixed by +the kind; only the timing is stored. + +The `KeepPreAsap` / `BinaryOp` / `ValueOperation` / `RelationalJoin` arms of today's +`validate_execution_data_states` merge into one `NonASAP` arm of `derive_timings`: + +- Pass the state to each child; an `ASAP` child is checked by the `ASAP` edge rules. +- `check_plain_operands` stays: referenced columns must be `FieldType::DataType` + (`Project` / `Filter` / `Sort` / `Limit` may pass `ExactAggregate` columns through). + This rejects `Project(ASAP(SummaryAgg))`. +- `BinaryOp`'s ingestion-side constraints move into this arm. +- `AmbiguousKeepPreAsap` is deleted: a subtree read at two timings is copied (§4). + +## 6. Export: fragments in the executable DAG + +The four original-operator payloads (`fallback{expression: QueryExpr}`, `binary`, +`value`, `relational_join`) become one: + +```rust +ExecutableOperatorPayload::Relational { + /// No ASAP node inside. Leaves are Scans, or Scan { source: Source::DagInput { role } } + /// for an incoming edge whose schema is the edge's intermediate_schema. + expression: Operator, +} +``` + +`compile_executable_dag` takes each **largest connected subtree without `ASAP`** as one +fragment, cutting an edge with a `DagInput` leaf wherever it meets an `ASAP` node. +`ASAP` nodes map one-to-one onto the existing summary payloads; +`FinalizeExactAccumulator` / `MaintainPopulation` / `ReadPopulation` stay +`value{operation}`. A backend lowers every fragment with its existing `QueryExpr` +lowering plus a `DagInput` arm (an incoming edge as a materialized table); the +`binary` / `value::Project` / `relational_join` lowerings go. + +- **Wire 5 → 6**: three fewer payloads; `fallback` becomes `relational` with `DagInput` + leaves; `output_schema` / `intermediate_schema` become `Schema`. One cutover (§8 + stage 4), together with the downstream readers. +- **Timing and guarantee** are read from the node slots; an `Unset` slot is rejected. + An edge's `data_state` is its producer's timing plus the primitive of its kind (§5). + `compile_executable_dag` no longer re-runs data-state validation. +- **Phases** become per fragment. Switching phase inside a fragment would need a + materialization point, and those are `ASAP` nodes, so nothing is lost. + +## 7. Other consumers + +| Location | Change | +|---|---| +| `post_asap/cse.rs` | delete; `share_common_subtrees` covers `ASAPOp` (derives `PartialEq` + serde) | +| `dag_export.rs` | delete `build_summary` / `build_summary_hybrid` / `summary_kind_tag`; one exporter with an `ASAP` arm; update the viewer's `node-style.js` and the pin test `viewer_categorizes_exactly_the_exported_node_kinds` | +| `summary_maintenance_cost/estimator.rs` (80 sites) | `KeepPreAsap` branches (`query_source_selections`, `retained_queries`) use the §6 fragment; `exact_binary` / `value_operation` costs fold into it. Also fixes the missing `RelationalJoin` arm in `summary_operation_evidence` | +| `physical_plan_cost_model.rs::estimate_candidate` | every fragment goes through `lower_query_physical_dag` | +| `summary_maintenance_lifecycle.rs` | `selected_raw_recompute` becomes `!contains_asap(root)`; the `keep_pre_asap(target)` fallback in `assemble_selected_dag_with_summary_maintenance_lifecycles` becomes `target`; the lifecycle decision sets a `SummaryAgg` to `IngestionTime` with `with_timing` (§2.3) | +| `maintained_population.rs` | `KeepPreAsap(source)` becomes `source`; `population.matches_input` reads an `NonASAP` child directly | +| `exact_composition.rs` | `ExactOperation::Aggregate` becomes an `NonASAP(Aggregate)` whose child is a hole | +| `RelationalJoin.pruning` | never set to `Some` in production; delete. Candidate pruning can return as an `ASAPOp` variant | + +--- + +# III. Implementation + +## 8. Stages and tests + +`main` builds and passes all tests after every stage. + +| Stage | Content | Touches | +|---|---|---| +| 0 Preparation | `Rc` for `Concat.children`; `rebuild_children` → `map_children`; `Column::plain` | `asap-types` | +| 1 Split | [decoupling doc](decoupling_op_and_expr.md): `NonASAPOp` + `ScalarExpr`; children stay `Rc` | scalar code ([decoupling doc §3](decoupling_op_and_expr.md#3-changes)) | +| 2 Two levels | §1.1, §1.4: `Operator`, an empty `ASAPOp`, `contains_asap()`, `expect_non_asap()`; child slots become `Rc>`; every variant gets `timing` / `guarantee` slots, left `Unset` | every crate; the same mechanical change everywhere | +| 3 One schema | §2.1: `Column` → `Field` and `Schema.columns` → `fields` (serde keeps the name `columns` until stage 4); `FieldType`, `ASAPType`, `PlainField`, `Schema` everywhere except the `executable_dag.rs` wire types, which keep `SummarySchema` until stage 4. **No wire change** | `asap-types` + schema construction in every crate | +| 4 New types | fill `ASAPOp`; `ASAP` arms of `output_schema`; `derive_guarantees` and `derive_timings` (§2.2, §5); the entry check (§3); `flatten(&SummaryNode) -> Rc` so export runs on the new types; wire types become `Schema`, and `Schema.fields` serializes as `fields`. Wire → 6. **The only wire-breaking stage**; merged together with ASAPQuery-backend and ASAPCollector | `asap-types`, `devtools`, viewer | +| 5 Planner | §4: candidates and assembly on `Rc`; §7 moves to the new types; delete `flatten` | `asap-aware-mapping` | +| 6 Cleanup | delete `SummaryExpr`, `SummaryNode`, extra `ValueOperation` variants, `ExactOperation`, `post_asap/cse.rs`; update `post-asap-ir.md`, `physical-plan-integration.md`, developer and viewer docs | docs | + +Wrapping pre-ASAP operators in `ValueOperation` first is not planned: stage 4 gives +the same early flat export, on the final types. + +**Tests**: + +- One integration test per #468 problem: + 1. `WITH metric AS (SELECT avg(CASE WHEN l_quantity BETWEEN 1 AND 50 THEN 1.0 ELSE 0.0 END) AS in_range FROM lineitem) SELECT in_range, in_range = 1.0 AS ok FROM metric` — no post-ASAP-only node besides `ASAP`; all `Project`s are one variant. + 2. `SELECT avg(l_extendedprice), approx_percentile_cont(l_discount, 0.99) FROM lineitem` — the `avg` `Aggregate` and the KLL `SummaryAgg` share one `Scan` by `Rc::ptr_eq` (once a binding rule splits measures). + 3. `SELECT approx_distinct(l_partkey) FROM lineitem UNION ALL SELECT approx_distinct(l_suppkey) FROM lineitem` — each side of the `SetOp` has a `SummaryEstimate`. +- A shared `Scan` read at two timings is copied once per timing; read at one timing, it stays one `Rc`. +- After `derive_*`, no slot is `Unset`; export rejects a tree with one. +- A `SummaryEstimate` over a hole reports the error of the plan that fills the hole. +- Each unexercised variant (§1.3) returns `Unimplemented` from `output_schema`, `derive_*` and export. +- `ParsedWorkload::new` rejects a tree containing `ASAP`. +- Rewrite the 117 `SummaryExpr::` assertions in `sql_to_post_asap.rs` / `promql_to_post_asap.rs` / `exact_composition.rs`. +- Wire 6 round trip with a `DagInput` fragment; a version-5 document is rejected. +- The 52 `execution_data_state.rs` tests keep their shapes; assertions read the `timing` slot instead of `ExecutionDataStateAssignment`. + +## 9. Out of scope + +- The binding rule splitting a multi-measure `Aggregate` into exact + summary over one child. +- Candidate pruning as an `ASAPOp` variant. +- Accuracy through `SummaryMerge` / `Subtract` / `Delete` / `Join` (§2.2): an accuracy + descriptor on state, or composing error along the state chain at readout. +- Folding `ExactComposition` into `Subtree` (both of its forms become expressible); + deferred until stage 5 is stable. + +## 10. Open questions + +None at present. From 19536469269363c0652e4ce32e93c5caee38a1a2 Mon Sep 17 00:00:00 2001 From: Selvomega Date: Tue, 29 Sep 2026 16:51:13 +0000 Subject: [PATCH 4/5] document updated --- .../proposals/decoupling_op_and_expr.md | 2 +- .../design_docs/proposals/operator-sharing.md | 358 +++++++++++------- 2 files changed, 229 insertions(+), 131 deletions(-) diff --git a/docs/design_docs/proposals/decoupling_op_and_expr.md b/docs/design_docs/proposals/decoupling_op_and_expr.md index 9d34b4e4a..b28727b4b 100644 --- a/docs/design_docs/proposals/decoupling_op_and_expr.md +++ b/docs/design_docs/proposals/decoupling_op_and_expr.md @@ -84,7 +84,7 @@ NonASAPOp | Location | Change | |---|---| | frontend expression lowering (`df_expr_to_unresolved`, PromQL `walk`) | scalar positions build `ScalarExpr`, operator positions `NonASAPOp` | -| `resolve`, `column_resolution.rs` | separate operator and scalar resolvers; a scalar resolves against its operator's input schema | +| `resolve`, `column_resolution.rs` | already separate: `resolve` walks operators and calls `resolve_expr` for scalars. Each scalar resolves against one schema its operator picks (usually the child's output; the `Aggregate`'s output for `HAVING`, left + right for a `Join` predicate, the `Scan`'s own schema for `Scan` predicates). The split only changes their signatures: `resolve` takes `NonASAPOp`, `resolve_expr` takes `ScalarExpr`. Leaf schemas are still inferred from scalar column references across the whole tree | | `canonicalize`, `pre_asap/cse.rs` | the "scalar: nothing to do" arms go; scalars are hashed as plain data | | `scalar_signature.rs`, `infer_expr_type` | take `ScalarExpr` | | `QueryExpr::output_schema` | becomes `NonASAPOp::output_schema`; the scalar arms and `ScalarHasNoRowSchema` go | diff --git a/docs/design_docs/proposals/operator-sharing.md b/docs/design_docs/proposals/operator-sharing.md index 4ee165901..44254c4cd 100644 --- a/docs/design_docs/proposals/operator-sharing.md +++ b/docs/design_docs/proposals/operator-sharing.md @@ -1,11 +1,8 @@ # Sharing Operators Between Pre-ASAP IR and Post-ASAP IR -> Status: proposed, not implemented. Problem statement: -> [#468](https://github.com/ProjectASAP/ASAPPlanner/issues/468). Implementation -> starts after the single entry point and the pluggable pass (#429, #430) land on -> `main`. Builds on [Decoupling operators from scalar expressions](decoupling_op_and_expr.md) -> (same PR), which splits `QueryExpr` into `NonASAPOp` and `ScalarExpr`. Code is -> referenced by file and function; counts are approximate, measured on `main` at `8acb472`. +> - Status: proposed, not implemented. +> - Problem statement: [#468](https://github.com/ProjectASAP/ASAPPlanner/issues/468). +> - Builds on [Decoupling operators from scalar expressions](decoupling_op_and_expr.md) (same PR), which splits `QueryExpr` into `NonASAPOp` and `ScalarExpr`. **The idea.** Today a post-ASAP plan is glued together from two sets of operator types. This proposal keeps one operator language and makes summary operators extra node kinds in it: any relational operator can sit above a summary, and a summary can read any relational subtree. @@ -21,7 +18,7 @@ ValueOperation(Project) ← a copy NonASAP(Project) | Part | Sections | |---|---| -| I. New IR | §1 Types, §2 Per-node information | +| I. New IR | §1 Types, §2 Schema, guarantee, and timing | | II. Changes, in data-flow order | §3 Entry → §4 Planner → §5 Timing → §6 Export → §7 Other consumers | | III. Implementation | §8 Stages and tests, §9 Out of scope, §10 Open questions | @@ -31,86 +28,125 @@ ValueOperation(Project) ← a copy NonASAP(Project) ## 1. Types -### 1.1 Overview +### 1.1 Unified `Operator` type -Operator attributes differ in how widely they apply. Each is defined at the level -that matches its breadth: +Operator attributes differ in how widely they apply. +We define the `Operator` type structure based on the breadth of its attributes. | Applies to | Examples | Defined as | |---|---|---| -| every operator | children, schema, timing, guarantee | methods of `Operator`; the values may be stored per variant | -| one category | for all `NonASAP` operators, timing is derived from the consuming edge, and guarantee from the children | a variant of `Operator` | +| every operator | children, schema, timing, guarantee | methods implemented for `Operator` | +| one category | for all `NonASAP` operators, timing is derived from the consuming edge, and guarantee from the children | implementation specified to one enum branch of `Operator` | | one operator | `Aggregate.measures`, `SummaryAgg.family` | fields of that variant | ```rust -pub enum Operator { // category - NonASAP(NonASAPOp), // today's relational QueryExpr variants (§1.2) - ASAP(ASAPOp), // summary operators (§1.3) +pub enum Operator { + NonASAP(NonASAPOp), // today's relational and timeseries operators in `QueryExpr` (§1.2) + ASAP(ASAPOp), // summary operators (§1.3) } -impl Operator { // every operator +impl Operator { // implemented for every operator pub fn children(&self) -> Vec<&Rc>>; pub fn map_children(&self, f: impl FnMut(&Rc>) -> Rc>) -> Self; - pub fn output_schema(&self) -> Result; // computed (§2.1) - pub fn timing(&self) -> &Slot; // §2.3 - pub fn guarantee(&self) -> &Slot>; // §2.2; Set(None): unknown, never read as exact - pub fn with_timing(&self, timing: ExecutionTiming) -> Self; - pub fn with_guarantee(&self, guarantee: Option) -> Self; + pub fn output_schema(&self) -> Result; // schema: §2.1 + pub fn guarantee(&self) -> &Slot>; // accuracy guarantee: §2.2 + pub fn timing(&self) -> &Slot; // execution timing: §2.3 + pub fn with_guarantee(&self, guarantee: Option) -> Self; // Setter of accuracy guarantee + pub fn with_timing(&self, timing: ExecutionTiming) -> Self; // Setter of execution timing } + +/// `Slot` represents a value that may be unset or set. +/// In the current design, it will be used to wrap the `timing` and `guarantee` values, +/// whose values will only be determined after derivation. pub enum Slot { Unset, Set(T) } -pub fn derive_guarantees(root: &Rc, model: &dyn AccuracyModel, - evidence: &dyn AccuracyEvidenceProvider) -> Result, AccuracyError>; // §2.2 -pub fn derive_timings(root: &Rc) -> Result, ExecutionDataStateError>; // §2.3, §5 +/// Memo of one derivation, keyed by (node pointer, incoming timing). +/// One is shared by every root of a workload, so a node shared by two roots stays one `Rc`. +pub struct DerivationMemo { .. } + +/// Build the accuracy guarantee of one DAG root by derivation +pub fn derive_guarantees( + root: &Rc, + model: &dyn AccuracyModel, // accuracy model used for derivation + evidence: &dyn AccuracyEvidenceProvider, // evidence provider used for derivation + memo: &mut DerivationMemo, +) -> Result, AccuracyError>; +/// Build the execution timing of one DAG root by derivation +pub fn derive_timings( + root: &Rc, + memo: &mut DerivationMemo, +) -> Result, ExecutionDataStateError>; ``` -### 1.2 `NonASAPOp` - -`NonASAPOp` and `ScalarExpr` come from splitting `QueryExpr` -([decoupling doc](decoupling_op_and_expr.md#2-types)). Here `NonASAPOp` becomes the -non-ASAP category of `Operator`: its child slots widen from `Rc` to -`Rc` (§1.4), and every variant gets the `timing` / `guarantee` slots (§1.1). -Scalar fields are unchanged; only `NonASAPOp` holds them. +- **Derivation recomputes**: `derive_*` keep the values set at construction (§2.2, §2.3) + and recompute every other slot, so calling them again after a rewrite is safe. +- **Equality**: both slots take part in `PartialEq` and hashing, so CSE never merges two + nodes that differ in timing or guarantee. +Following diagram conceptually displays the structure of `Operator`: ```text Operator ├─ NonASAP(NonASAPOp) │ ├─ children: Rc> → back to Operator: NonASAP or ASAP -│ ├─ timing / guarantee slots -│ └─ scalar fields: Predicate / ProjectItem / SortKey / ScalarBridge / ... -│ └─ ScalarExpr: never contains an Operator +│ ├─ timing / guarantee +│ └─ scalar expressions: Predicate / ProjectItem / SortKey / ScalarBridge / ... +│ └─ ScalarExpr: never contains an Operator └─ ASAP(ASAPOp) ├─ children: Rc> → back to Operator: NonASAP or ASAP - └─ timing / guarantee slots + └─ timing / guarantee ``` +### 1.2 `NonASAPOp` + +`NonASAPOp` is the non-ASAP category of `Operator`. +It comes from splitting `QueryExpr` into "operator" and "scalar expression" parts ([decoupling doc](decoupling_op_and_expr.md#2-types)). + +```rust +pub enum NonASAPOp { + Scan { .. }, + Filter { pred: Predicate, child: Rc> }, + Project { cols: Vec>, child: Rc> }, + Aggregate { reduction, measures, having: Option>, child: Rc> }, + Join { kind, pred: Predicate, left: Rc>, right: Rc> }, + SetOp { kind, all, left: Rc>, right: Rc> }, + Concat { children: Vec>> }, + Sort { keys: Vec>, child: Rc> }, + Limit { n, offset, child: Rc> }, + BinaryOp { op, lhs, rhs }, + SQLWindowFunc { args: Vec>, order_by: Vec>, child: Rc>, .. }, + Dedup { .. }, TimeRange { .. }, TimeShift { .. }, Promql* { .. }, + ScalarBridge(Rc>), // the `2` in PromQL `v * 2` + EvalTimestamp, // PromQL time() +} +``` + +Every variant also carries the `timing` and `guarantee` slots (§1.1), omitted above. + ### 1.3 `ASAPOp` +`ASAPOp` is the ASAP category of `Operator`. +`ASAPOp` comes from today's `SummaryExpr`: its summary variants, and the summary-specific `ValueOperation` variants. + ```rust pub enum ASAPOp { SummaryAgg { child: Rc>, family: ASAPType, input, reduction, grouping, - exact_rule: Option }, // ExactAggregate only, §2.2 - SummaryEstimate { child, query: SketchQuery, - local_guarantee: Option }, // §2.2 + exact_rule: Option }, + SummaryEstimate { child: Rc>, query: SketchQuery, + local_guarantee: Option }, SummaryMerge { children: Vec>> }, - SummarySubtract { left, right }, - SummaryDelete { child, key: C }, // key was ColumnRef - SummaryJoin { outer, inner, key: C, family: ASAPType }, - // the summary-specific ValueOperation variants, lifted to the top level - FinalizeExactAccumulator { child }, - MaintainPopulation { child, population }, // always ingestion time - ReadPopulation { child, readout }, // always query time - Extension { child, name: String }, + SummarySubtract { left: Rc>, right: Rc> }, + SummaryDelete { child: Rc>, key: C }, + SummaryJoin { outer: Rc>, inner: Rc>, key: C, family: ASAPType }, + FinalizeExactAccumulator { child: Rc> }, + MaintainPopulation { child: Rc>, population }, + ReadPopulation { child: Rc>, readout }, + Extension { child: Rc>, name: String }, } ``` -`family` was the original `SummaryFamilyType`. - Every variant also carries the `timing` and `guarantee` slots (§1.1), omitted above. -**Unexercised variants**: `SummaryMerge`, `SummarySubtract`, `SummaryDelete`, `SummaryJoin` -and `Extension` are built only in tests today. They are migrated, but all their methods -return `Unimplemented`. +**Unused branches**: `SummaryMerge`, `SummarySubtract`, `SummaryDelete`, `SummaryJoin` and `Extension` are built only in tests today. They are migrated, but for safety, we have all their methods return `Unimplemented`. Following table shows how some legacy types get expressed in the new framework. @@ -135,44 +171,35 @@ Filter { pred: Predicate(Rc), Filter { pred: Predicate(Rc` today: branches are stored by value and have no `Rc` identity, so the planner (§4), which identifies targets and holes by pointer, +`Concat.children` is `Vec` today: branches are stored by value and have no `Rc` identity, so the planner (§4), which identifies targets by pointer, cannot replace a branch — e.g. the branches of SQL `ROLLUP` or PromQL `histogram_quantiles`. It becomes `Vec>` (§8 stage 0). -## 2. Per-node information +## 2. Schema, Guarantee, and Timing -Besides children (§1.4), every operator has the three attributes below: the -every-operator level of §1.1. Each row says where the value lives and who supplies it. +This section discusses three key per-node attributes, `schema`, `guarantee`, and `timing`, as well as how they are stored and derived in the new framework. | Field | Meaning | Today | After | |---|---|---|---| -| `schema` | output columns and their types | stored on every `SummaryNode` | obtained by `output_schema()` (§2.1) | -| `guarantee` | how far the output value can be off | stored on every `SummaryNode` | obtained by `guarantee()`: derived for every node; binding stores only a sketch's own error, as a `SummaryEstimate` field (§2.2) | -| `timing` | ingestion time or query time | on `BinaryOp` / `ValueOperation` / `SummaryMerge` | obtained by `timing()`: set by binding and lifecycle (`SummaryAgg`) or the planner (`FinalizeExactAccumulator`); derived for the rest (§2.3) | +| `schema` | output columns and their types | pre-ASAP: computed by `QueryExpr::output_schema()`
post-ASAP: a `SummarySchema` stored on every `SummaryNode` | can be obtained by `output_schema()` | +| `guarantee` | accuracy bound | pre-ASAP: none
post-ASAP: stored on every `SummaryNode` | can be obtained by `guarantee()`
binding stores only each operator's own error
complete error bound need to be derived by `derive_guarantees()` | +| `timing` | execution time | pre-ASAP: none
post-ASAP, stored: a field on `BinaryOp` / `ValueOperation` / `SummaryMerge`
post-ASAP, not stored: `KeepPreAsap` from the consuming edge, `SummaryAgg` from the child. | can be obtained by `timing()`
set by binding (`SummaryAgg`) or the planner (`FinalizeExactAccumulator`)
timing of the rest of operators need to be derived by `derive_timings()` | ### 2.1 Schema: fused into one type -Today pre-ASAP uses `Schema { columns: Vec, time_index, unique_keys, closed }` -with `Column.dtype: DataType` (plain values only), and post-ASAP stores a -`SummarySchema { fields: Vec, time_index }` on every node, with -`SummaryField.dtype: SummaryFamilyType` (`Plain(DataType)` or summary state). One -tree now needs one schema type whose columns can be either: - -``` -SummaryEstimate(Quantile .99) → [p99: DataType(Float64)] - SummaryAgg(Kll) → [state: ASAPType(Sketch(Kll, k=269))] - Scan lineitem → [l_orderkey: DataType(Int64), l_quantity: DataType(Float64), …] -``` +Today schemas of pre-ASAP operators and post-ASAP operators are different: +- pre-ASAP uses `Schema { columns: Vec, time_index, unique_keys, closed }` with `Column.dtype: DataType` (plain values only), +- post-ASAP stores a `SummarySchema { fields: Vec, time_index }` on every node, with `SummaryField.dtype: SummaryFamilyType` (`Plain(DataType)` or summary state). +Now since the two operators types are unified into one, we need a unified schema type as well. -We merge semantics of the above two types into one `Schema` type. -The struct of `Schema` stays, with two changes: +We implement the new schema type based on the original `Schema` type used in pre-ASAP operators, with two changes: - `Column` is renamed `Field`, and `Schema.columns` `Schema.fields`: the struct describes - a column and holds none of its data. Arrow and DataFusion use the same names. -- `Field.dtype` widens from `DataType` to an enum `FieldType`, so a field can describe either a - plain value or ASAP state. + a column and holds none of its data. (Arrow and DataFusion use the same names.) +- `Field.dtype` widens from `DataType` to an enum `FieldType`, which covers both plain data types and ASAP summary types. -`SummarySchema` / `SummaryField` are then redundant and deleted: +`SummarySchema` / `SummaryField` are then redundant and deleted. +Detailed code design is shown below. ```rust pub enum FieldType { DataType(DataType), ASAPType(ASAPType) } pub enum ASAPType { // SummaryFamilyType without Plain @@ -188,54 +215,106 @@ impl Field { } ``` -Today every `SummaryNode` stores its schema, built at construction. After, every node -computes it with `output_schema()`: - | Node | Today | After | |---|---|---| -| `NonASAPOp` | `QueryExpr::output_schema()` lifted to `SummarySchema` (`KeepPreAsap`), or stored on the `ValueOperation` / `BinaryOp` / `RelationalJoin` copy | today's `QueryExpr::output_schema()` logic | +| `NonASAPOp` | post-ASAP `KeepPreAsap`: `QueryExpr` schema lifted to `SummarySchema` and stored
post-ASAP `ValueOperation` / `BinaryOp` / `RelationalJoin` copies: stored at construction | using the same logic as `QueryExpr::output_schema()` | | `SummaryAgg` | the replaced `Aggregate`'s output with the measure column retyped to `family` | grouping columns + one `ASAPType(family)` column | | `SummaryEstimate` | the replaced operator's output schema | the child's grouping columns + the value columns of the `SketchQuery` | -| `FinalizeExactAccumulator` | the logical operator's output, lifted | the child's schema, `ASAPType(ExactAggregate ..)` columns turned into `DataType(..)` | +| `FinalizeExactAccumulator` | the logical operator's output, lifted | the child's schema, `ASAPType(ExactAggregate ..)` columns changed into `DataType(..)` | | `MaintainPopulation` / `ReadPopulation` | the source's schema / the replaced aggregate's output | the same rules, computed from the child and the `readout` | -| unexercised variants | one field typed `family` | unimplemented (§1.3) | +| unused variants | one field typed `family` | unimplemented | ### 2.2 Guarantee: always derived +A guarantee is filled in two steps: + +1. **Binding** records local accuracy guarantee: a `SummaryEstimate`'s `local_guarantee` (the sketch's error over an exact input) and an exact `SummaryAgg`'s `exact_rule`. No `guarantee` slot is set yet. +2. **`derive_guarantees`** fills every slot bottom-up: a node without an `ASAP` descendant is exact, and every other node composes its children's guarantees by its own rule. + +``` +Project p99 ±1% ← the child's + SummaryEstimate p99 ±1% ← local ±1%, composed with the child's + SummaryAgg(Kll) None ← state has no guarantee + Scan t exact ← no ASAP descendant +``` + +Per node kind: + | Node | Today | After | |---|---|---| -| `SummaryEstimate` | stored at binding: the sketch's own error composed with the child's (`compose_guarantee`) | **derived**: `local_guarantee` composed with the child's. `local_guarantee` is a field set at binding: the sketch's error over an exact input, `None` when the model has no error model for the family | +| `SummaryEstimate` | stored at binding: the sketch's own error composed with the child's (`compose_guarantee`) | **derived**: `local_guarantee` composed with the child's. `local_guarantee` is set at binding: the sketch's error over an exact input, `None` when the model has no error model for the family | | `SummaryAgg` | stored: ExactAggregate family composed with the child's; sketch families `None` | **derived**: ExactAggregate family: exact, composed with the child's under `exact_rule`; sketch families `Set(None)`, state has no guarantee | -| `NonASAPOp` | `KeepPreAsap`: exact; the `ValueOperation` / `BinaryOp` / `RelationalJoin` copies: composed at construction | **derived**: composed from the children (`relational_join_guarantee`, `exact_operation_rule`); exact if no `ASAP` descendant | +| `NonASAPOp` | pre-ASAP `QueryExpr`: none
post-ASAP `KeepPreAsap`: exact
post-ASAP `ValueOperation` / `BinaryOp` / `RelationalJoin` copies: composed at construction | **derived**: composed from the children; exact if no `ASAP` descendant | | `FinalizeExactAccumulator` | copies the child's | **derived**: the child's | | `MaintainPopulation` / `ReadPopulation` | stored: exact | **derived**: exact | -| unexercised variants | `None`: state has no guarantee of its own | unimplemented (§1.3) | - -- **Only the local part is stored.** Today binding stores the composed value, built - from the child it sees. Assembly can fill that child's hole with a different plan, and - a stored composition would go stale (`relink_agg_child` copies it today, relying on the - new child being exact). -- `derive_guarantees` uses the same `AccuracyModel` as binding, and the evidence for - `propagate`'s `PropagationStats`. It reads only the subtree, so search runs it on a - candidate to check its accuracy target (the candidate filter in - `search_workload_with_targets`). -- The value travels with the node through cloning, CSE and serialization, as - `SummaryNode.guarantee` does today. +| unused variants | `None`: state has no guarantee of its own | unimplemented (§1.3) | ### 2.3 Timing: set where position does not decide it +A timing is filled in two steps: + +1. **Binding** sets every `SummaryAgg` to the timing today's fallback derives: + `IngestionTime`, or `QueryTime` when binding built its child at query time (a + query-time `FinalizeExactAccumulator`). The planner sets every + `FinalizeExactAccumulator`. +2. **`derive_timings`** runs on each assembled root, sharing one memo, top-down: a root is query + time, a set node keeps its value, a node of fixed kind takes that kind's time, and + every other node takes its parent's. A node reached at two timings is copied (§4). + +``` + binding derive_timings +Project Unset QueryTime ← root + SummaryEstimate Unset QueryTime ← fixed by kind + SummaryAgg IngestionTime IngestionTime ← kept + Scan t Unset IngestionTime ← from parent +``` + +Exported timings are unchanged. Moving the `SummaryAgg` default to `QueryTime`, and +letting the lifecycle step choose ingestion time, is a separate PR. + +Per node kind: + | Node | Today | After | |---|---|---| -| `NonASAPOp` | `KeepPreAsap`: from the consuming edge; the `ValueOperation` / `BinaryOp` copies: a stored field | **derived** from the consuming edge (§5) | -| `SummaryAgg` | from the child; ingestion time under `KeepPreAsap` | **set**: binding sets `QueryTime`; a lifecycle decision may change it to `IngestionTime` | +| `NonASAPOp` | pre-ASAP `QueryExpr`: none
post-ASAP `KeepPreAsap`: from the consuming edge
post-ASAP `ValueOperation` / `BinaryOp` copies: a stored field | **derived** from the consuming edge (§5) | +| `SummaryAgg` | from the child; ingestion time under `KeepPreAsap` | **set** by binding, as today's fallback: `IngestionTime`, or `QueryTime` over a query-time child | | `FinalizeExactAccumulator` | a stored field, set by the planner | **set** by the planner: the same position allows either time | | `SummaryEstimate` | query time, fixed by the kind | **derived** from the kind: query time | | `MaintainPopulation` / `ReadPopulation` | a stored field, always ingestion / query time | **derived** from the kind: ingestion / query time | -| unexercised variants | `SummaryMerge`: a stored field; `Join` / `Subtract` / `Delete`: ingestion time | unimplemented (§1.3) | +| unused variants | `SummaryMerge`: a stored field; `Join` / `Subtract` / `Delete`: ingestion time | unimplemented (§1.3) | Unlike a guarantee, a timing depends on the parents, so `derive_timings` needs the whole DAG and runs only after assembly. +### 2.4 Workflow of setting up `guarantee` and `timing`: today vs. after + +Today: + +``` +search / binding each SummaryNode's guarantee is composed when the node is built; + BinaryOp / ValueOperation store their timing +selection reads each candidate's stored guarantee against its target +assembly assemble_residual builds kept nodes and composes their guarantee; + relink_summary copies the old guarantee onto a relinked SummaryAgg +lifecycle reads the root's guarantee +export validate_execution_data_states, per root, derives the remaining + timings into a side table and writes them onto the edges +``` + +After: + +``` +search / binding sets only what cannot be derived: SummaryAgg.timing, + local_guarantee, exact_rule; checks accuracy on a derived copy, + then drops the copy +assembly builds each root; kept NonASAP nodes stay as they are +derive_timings per root, one shared memo: fills timings top-down, copies a node + read at two timings +derive_guarantees per root, one shared memo: fills guarantees bottom-up +lifecycle reads the derived guarantee +export reads the slots; rejects an Unset one +``` + --- # II. Changes, in data-flow order @@ -243,8 +322,8 @@ DAG and runs only after assembly. ## 3. Optimizer entry Frontends and `resolve` build `NonASAP` trees only and access children with -`expect_non_asap()`. `ParsedWorkload::new` rejects a tree that `contains_asap()`, -next to its existing entry-count check: +`expect_non_asap()`. `search_cse_workload_with`, which every `search_workload*` entry +reaches, panics on a root that `contains_asap()`: an ASAP node there is a caller bug. ```rust impl Operator { @@ -256,8 +335,7 @@ impl Operator { A compile-time alternative — an associated type on `ColState` with `ColumnRef::ASAP = Never` — only protects frontend code before `resolve`: frontends already return `ColumnId` trees, where `ASAP` is allowed. The entry check covers every -input (frontends after `resolve`, deserialized plans, third-party frontends, test IR) -with simpler types. +input (frontends after `resolve`, deserialized plans, test IR) with simpler types. ## 4. Planner: search and assembly @@ -268,45 +346,57 @@ pub enum Replacement { } ``` -**Holes**: a subtree of a candidate that is `Rc::ptr_eq` to a target is filled by that -target's own choice. A candidate needing a specific child implementation (e.g. -`realize_temporal_average`) inlines a new node, so it is not a hole. -`realize_child_with` falls back to leaving a hole instead of `keep_pre_asap`. +**Candidates stay bottom-up, as today**: a candidate is built on a concrete child plan +(`realize_child_with`, or each child candidate in `prepare_compositions`), so a chosen +plan is complete. Where `realize_child_with` falls back to `keep_pre_asap` today, it +returns the child's original subtree, and assembly keeps it as is. + +**Accuracy check during search**: binding sets no `guarantee` slot (§2.2), so the +candidate filter in `search_workload_with_targets` and `prepare_compositions` run +`derive_guarantees` on the candidate alone, with a fresh memo, then check its accuracy target. The derived +copy is only read, then dropped: PlanSpace keeps the original candidate, whose nodes are +shared with other queries. **Assembly** — one rule replaces `assemble_residual`: ```rust fn assemble(&self, t: &Rc) -> Rc { memo by ptr; // shared children stay one Rc - let body = match self.chosen(t) { - Some(Subtree(r)) => r, // rewrites are assembled further down too; today assemble_target wraps them in keep_pre_asap + match self.chosen(t) { + Some(Subtree(r)) => r, // a complete plan, used as is Some(ExactComposition{..}) => composition.plan, - None => t, // keep the node, recurse — assemble_residual does this for four operators only - }; - body.map_children(|c| if is_target(c) { self.assemble(c) } else { c }) + None => t.map_children(|c| if is_target(c) { self.assemble(c) } else { c }), + // keep the node, assemble its children — assemble_residual does this for four operators only + } } ``` -Then run `derive_timings` (§5) once: - -- **Illegal fill** (e.g. a query-time `SummaryEstimate` under a `SummaryAgg`): - `derive_timings` returns an error, and the hole falls back to its original subtree — - `relink_summary`'s fallback, for every operator. +Then `GlobalSelection::assemble_selected_dag` runs `derive_timings` (§5) → +`derive_guarantees` (§2.2) on each root it assembles. The `DerivationMemo` lives on +`GlobalSelection` next to `assembled_nodes`, so roots assembled one call at a time still +share nodes. `assemble_selected_dag_with_summary_maintenance_lifecycles` plans lifecycles +on that result, as today: lifecycle planning holds `Rc`s into the plan and reads the +root's guarantee, so it must see the derived tree. + +- **Illegal child** (e.g. a query-time `SummaryEstimate` under a `SummaryAgg`): candidates + are checked when built, as `relink_summary` does today + (`validate_execution_data_states_at`). An error from `derive_timings` after assembly is + a bug, and planning fails with that error. - **A shared subtree read at two timings**, e.g. a query-time `Aggregate` and an ingestion-time `SummaryAgg` reading one `Scan`: both choices are legal, only the sharing is not. `derive_timings` memoizes by (pointer, timing), so it builds one copy per - timing; a subtree read at one timing stays one `Rc`. + timing; a subtree read at one timing stays one `Rc`, within a root or across roots. -Finally run `derive_guarantees` (§2.2). `map_children` is `rebuild_children` from +`map_children` is `rebuild_children` from `pre_asap/cse.rs`, dispatching to `NonASAPOp::map_children` / `ASAPOp::map_children`. -Deleted: `assemble_residual`, `relink_summary`, the `query_time_nested_sum` / -`contains_aggregate` special cases, `keep_pre_asap` / `keep_pre_asap_rc`, and the -`KeepPreAsap` branch of `finalize_exact_accumulator`. +Deleted: `assemble_residual`, `keep_pre_asap` / `keep_pre_asap_rc`, and the +`KeepPreAsap` branch of `finalize_exact_accumulator`. Kept: `relink_summary` and the +`query_time_nested_sum` special case, which pick a child after selection today. | #468 problem | Resolution | |---|---| | 1. A `Project` is a `QueryExpr` inside `KeepPreAsap` and a `ValueOperation` outside | one set of types | -| 2. Nothing outside `KeepPreAsap` can reference the `Scan` inside, so an exact aggregate and a sketch cannot share a scan | `Aggregate` and `SummaryAgg` can point to the same `Scan`. This holds when both run at the same time — always, without lifecycle decisions; otherwise the scan is duplicated (above). Splitting a multi-measure `Aggregate` into exact + sketch is a binding rule, out of scope (§9) | +| 2. Nothing outside `KeepPreAsap` can reference the `Scan` inside, so an exact aggregate and a sketch cannot share a scan | `Aggregate` and `SummaryAgg` can point to the same `Scan`. This holds when both run at the same time; otherwise the scan is copied (above). With today's defaults the sketch runs at ingestion time and the exact `Aggregate` at query time, so they share only after the `QueryTime` default (separate PR, §2.3). Splitting a multi-measure `Aggregate` into exact + sketch is a binding rule, out of scope (§9) | | 3. `SetOp` and similar have no post-ASAP copy, so no summary below them | `SetOp` takes `None => t`; both children are assembled | ## 5. Timing: execution data states @@ -362,6 +452,8 @@ lowering plus a `DagInput` arm (an incoming edge as a materialized table); the - **Timing and guarantee** are read from the node slots; an `Unset` slot is rejected. An edge's `data_state` is its producer's timing plus the primitive of its kind (§5). `compile_executable_dag` no longer re-runs data-state validation. +- **`SummaryMerge`** stays a wire payload, although its planner-side variant is + unimplemented (§1.3, §10). - **Phases** become per fragment. Switching phase inside a fragment would need a materialization point, and those are `ASAP` nodes, so nothing is lost. @@ -373,9 +465,9 @@ lowering plus a `DagInput` arm (an incoming edge as a materialized table); the | `dag_export.rs` | delete `build_summary` / `build_summary_hybrid` / `summary_kind_tag`; one exporter with an `ASAP` arm; update the viewer's `node-style.js` and the pin test `viewer_categorizes_exactly_the_exported_node_kinds` | | `summary_maintenance_cost/estimator.rs` (80 sites) | `KeepPreAsap` branches (`query_source_selections`, `retained_queries`) use the §6 fragment; `exact_binary` / `value_operation` costs fold into it. Also fixes the missing `RelationalJoin` arm in `summary_operation_evidence` | | `physical_plan_cost_model.rs::estimate_candidate` | every fragment goes through `lower_query_physical_dag` | -| `summary_maintenance_lifecycle.rs` | `selected_raw_recompute` becomes `!contains_asap(root)`; the `keep_pre_asap(target)` fallback in `assemble_selected_dag_with_summary_maintenance_lifecycles` becomes `target`; the lifecycle decision sets a `SummaryAgg` to `IngestionTime` with `with_timing` (§2.3) | +| `summary_maintenance_lifecycle.rs` | `selected_raw_recompute` becomes `!contains_asap(root)`; the `keep_pre_asap(target)` fallback in `assemble_selected_dag_with_summary_maintenance_lifecycles` becomes `target` | | `maintained_population.rs` | `KeepPreAsap(source)` becomes `source`; `population.matches_input` reads an `NonASAP` child directly | -| `exact_composition.rs` | `ExactOperation::Aggregate` becomes an `NonASAP(Aggregate)` whose child is a hole | +| `exact_composition.rs` | `ExactOperation::Aggregate` becomes a `NonASAP(Aggregate)`, built over each child candidate as `prepare_compositions` does today | | `RelationalJoin.pruning` | never set to `Some` in production; delete. Candidate pruning can return as an `ASAPOp` variant | --- @@ -390,7 +482,7 @@ lowering plus a `DagInput` arm (an incoming edge as a materialized table); the |---|---|---| | 0 Preparation | `Rc` for `Concat.children`; `rebuild_children` → `map_children`; `Column::plain` | `asap-types` | | 1 Split | [decoupling doc](decoupling_op_and_expr.md): `NonASAPOp` + `ScalarExpr`; children stay `Rc` | scalar code ([decoupling doc §3](decoupling_op_and_expr.md#3-changes)) | -| 2 Two levels | §1.1, §1.4: `Operator`, an empty `ASAPOp`, `contains_asap()`, `expect_non_asap()`; child slots become `Rc>`; every variant gets `timing` / `guarantee` slots, left `Unset` | every crate; the same mechanical change everywhere | +| 2 Two levels | §1.1, §1.4: `Operator`, an empty `ASAPOp`, `contains_asap()`, `expect_non_asap()`; child slots become `Rc>`; every variant gets `timing` / `guarantee` slots, and nodes are built through constructors that leave both `Unset` | every crate; the same mechanical change everywhere | | 3 One schema | §2.1: `Column` → `Field` and `Schema.columns` → `fields` (serde keeps the name `columns` until stage 4); `FieldType`, `ASAPType`, `PlainField`, `Schema` everywhere except the `executable_dag.rs` wire types, which keep `SummarySchema` until stage 4. **No wire change** | `asap-types` + schema construction in every crate | | 4 New types | fill `ASAPOp`; `ASAP` arms of `output_schema`; `derive_guarantees` and `derive_timings` (§2.2, §5); the entry check (§3); `flatten(&SummaryNode) -> Rc` so export runs on the new types; wire types become `Schema`, and `Schema.fields` serializes as `fields`. Wire → 6. **The only wire-breaking stage**; merged together with ASAPQuery-backend and ASAPCollector | `asap-types`, `devtools`, viewer | | 5 Planner | §4: candidates and assembly on `Rc`; §7 moves to the new types; delete `flatten` | `asap-aware-mapping` | @@ -403,13 +495,15 @@ the same early flat export, on the final types. - One integration test per #468 problem: 1. `WITH metric AS (SELECT avg(CASE WHEN l_quantity BETWEEN 1 AND 50 THEN 1.0 ELSE 0.0 END) AS in_range FROM lineitem) SELECT in_range, in_range = 1.0 AS ok FROM metric` — no post-ASAP-only node besides `ASAP`; all `Project`s are one variant. - 2. `SELECT avg(l_extendedprice), approx_percentile_cont(l_discount, 0.99) FROM lineitem` — the `avg` `Aggregate` and the KLL `SummaryAgg` share one `Scan` by `Rc::ptr_eq` (once a binding rule splits measures). + 2. `SELECT avg(l_extendedprice), approx_percentile_cont(l_discount, 0.99) FROM lineitem` — the `avg` `Aggregate` and the KLL `SummaryAgg` share one `Scan` by `Rc::ptr_eq` when both run at the same time (once a binding rule splits measures); with today's ingestion-time default the `Scan` is copied. 3. `SELECT approx_distinct(l_partkey) FROM lineitem UNION ALL SELECT approx_distinct(l_suppkey) FROM lineitem` — each side of the `SetOp` has a `SummaryEstimate`. - A shared `Scan` read at two timings is copied once per timing; read at one timing, it stays one `Rc`. +- A node shared by two roots, assembled in two calls, is still one `Rc` after `derive_*`. - After `derive_*`, no slot is `Unset`; export rejects a tree with one. -- A `SummaryEstimate` over a hole reports the error of the plan that fills the hole. -- Each unexercised variant (§1.3) returns `Unimplemented` from `output_schema`, `derive_*` and export. -- `ParsedWorkload::new` rejects a tree containing `ASAP`. +- Exported timings of today's plans are unchanged. +- A kept `NonASAP` node (e.g. a `SetOp`) reports the guarantee composed from its assembled children. +- Each unused branch (§1.3) returns `Unimplemented` from `output_schema`, `derive_*` and export. +- `search_workload*` panics on a root containing `ASAP`. - Rewrite the 117 `SummaryExpr::` assertions in `sql_to_post_asap.rs` / `promql_to_post_asap.rs` / `exact_composition.rs`. - Wire 6 round trip with a `DagInput` fragment; a version-5 document is rejected. - The 52 `execution_data_state.rs` tests keep their shapes; assertions read the `timing` slot instead of `ExecutionDataStateAssignment`. @@ -420,9 +514,13 @@ the same early flat export, on the final types. - Candidate pruning as an `ASAPOp` variant. - Accuracy through `SummaryMerge` / `Subtract` / `Delete` / `Join` (§2.2): an accuracy descriptor on state, or composing error along the state chain at readout. +- Holes: letting a chosen plan's child be filled by that child target's own choice at + assembly, instead of fixing it when the candidate is built. A search-strategy change, + independent of the types here. - Folding `ExactComposition` into `Subtree` (both of its forms become expressible); deferred until stage 5 is stable. ## 10. Open questions -None at present. +- Does ASAPQuery insert `SummaryMerge` only on the executable DAG, or through ASAPPlanner's + post-ASAP types? The planner-side variant is unimplemented (§1.3). From 7135c989be7fdf35bb968e830028980350f3f1de Mon Sep 17 00:00:00 2001 From: Selvomega Date: Tue, 29 Sep 2026 17:02:42 +0000 Subject: [PATCH 5/5] grilled one more time --- .../proposals/decoupling_op_and_expr.md | 3 +-- .../design_docs/proposals/operator-sharing.md | 23 +++++++++++++------ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/docs/design_docs/proposals/decoupling_op_and_expr.md b/docs/design_docs/proposals/decoupling_op_and_expr.md index b28727b4b..e24b87aa0 100644 --- a/docs/design_docs/proposals/decoupling_op_and_expr.md +++ b/docs/design_docs/proposals/decoupling_op_and_expr.md @@ -6,8 +6,7 @@ > approximate, measured on `main` at `8acb472`. **The idea.** `QueryExpr` holds two different kinds of node in one enum. This proposal -splits it into `NonASAPOp` (operators) and `ScalarExpr` (scalar expressions), so the -field a node sits in decides its type. +splits it into `NonASAPOp` (operators) and `ScalarExpr` (scalar expressions). ``` Today Proposed diff --git a/docs/design_docs/proposals/operator-sharing.md b/docs/design_docs/proposals/operator-sharing.md index 44254c4cd..6ad34e65f 100644 --- a/docs/design_docs/proposals/operator-sharing.md +++ b/docs/design_docs/proposals/operator-sharing.md @@ -71,11 +71,18 @@ pub fn derive_guarantees( evidence: &dyn AccuracyEvidenceProvider, // evidence provider used for derivation memo: &mut DerivationMemo, ) -> Result, AccuracyError>; -/// Build the execution timing of one DAG root by derivation +/// Build the execution timing of one DAG root by derivation; the root runs at query time pub fn derive_timings( root: &Rc, memo: &mut DerivationMemo, ) -> Result, ExecutionDataStateError>; +/// Same, with the root's timing given, e.g. `IngestionTime` for a maintenance candidate +/// (like today's `validate_execution_data_states_at`) +pub fn derive_timings_at( + root: &Rc, + root_timing: ExecutionTiming, + memo: &mut DerivationMemo, +) -> Result, ExecutionDataStateError>; ``` - **Derivation recomputes**: `derive_*` keep the values set at construction (§2.2, §2.3) @@ -228,7 +235,7 @@ impl Field { A guarantee is filled in two steps: -1. **Binding** records local accuracy guarantee: a `SummaryEstimate`'s `local_guarantee` (the sketch's error over an exact input) and an exact `SummaryAgg`'s `exact_rule`. No `guarantee` slot is set yet. +1. **Binding** records local accuracy guarantee: a `SummaryEstimate`'s `local_guarantee` (the sketch's error over an exact input) and an exact `SummaryAgg`'s `exact_rule`. No `guarantee` slot is set yet. To size a sketch and check its target, binding still needs the child's error, as today: it runs `derive_guarantees` on the child with a fresh memo, reads the result, and drops it. 2. **`derive_guarantees`** fills every slot bottom-up: a node without an `ASAP` descendant is exact, and every other node composes its children's guarantees by its own rule. ``` @@ -243,7 +250,7 @@ Per node kind: | Node | Today | After | |---|---|---| | `SummaryEstimate` | stored at binding: the sketch's own error composed with the child's (`compose_guarantee`) | **derived**: `local_guarantee` composed with the child's. `local_guarantee` is set at binding: the sketch's error over an exact input, `None` when the model has no error model for the family | -| `SummaryAgg` | stored: ExactAggregate family composed with the child's; sketch families `None` | **derived**: ExactAggregate family: exact, composed with the child's under `exact_rule`; sketch families `Set(None)`, state has no guarantee | +| `SummaryAgg` | stored: ExactAggregate family composed with the child's; sketch families `None` | **derived**: ExactAggregate family: exact, composed with the child's under `exact_rule`, except `ExactKind::Count`, exact whatever the child (as today); sketch families `Set(None)`, state has no guarantee | | `NonASAPOp` | pre-ASAP `QueryExpr`: none
post-ASAP `KeepPreAsap`: exact
post-ASAP `ValueOperation` / `BinaryOp` / `RelationalJoin` copies: composed at construction | **derived**: composed from the children; exact if no `ASAP` descendant | | `FinalizeExactAccumulator` | copies the child's | **derived**: the child's | | `MaintainPopulation` / `ReadPopulation` | stored: exact | **derived**: exact | @@ -362,7 +369,9 @@ shared with other queries. ```rust fn assemble(&self, t: &Rc) -> Rc { memo by ptr; // shared children stay one Rc - match self.chosen(t) { + let chosen = if query_time_nested_sum(t) { None } // as today: keep the outer SUM so the + else { self.chosen(t) }; // inner target's own choice is assembled + match chosen { Some(Subtree(r)) => r, // a complete plan, used as is Some(ExactComposition{..}) => composition.plan, None => t.map_children(|c| if is_target(c) { self.assemble(c) } else { c }), @@ -379,8 +388,8 @@ on that result, as today: lifecycle planning holds `Rc`s into the plan and reads root's guarantee, so it must see the derived tree. - **Illegal child** (e.g. a query-time `SummaryEstimate` under a `SummaryAgg`): candidates - are checked when built, as `relink_summary` does today - (`validate_execution_data_states_at`). An error from `derive_timings` after assembly is + are checked when built with `derive_timings_at(candidate, placement's timing)`, as + `relink_summary` does today with `validate_execution_data_states_at`. An error from `derive_timings` after assembly is a bug, and planning fails with that error. - **A shared subtree read at two timings**, e.g. a query-time `Aggregate` and an ingestion-time `SummaryAgg` reading one `Scan`: both choices are legal, only the sharing @@ -484,7 +493,7 @@ lowering plus a `DagInput` arm (an incoming edge as a materialized table); the | 1 Split | [decoupling doc](decoupling_op_and_expr.md): `NonASAPOp` + `ScalarExpr`; children stay `Rc` | scalar code ([decoupling doc §3](decoupling_op_and_expr.md#3-changes)) | | 2 Two levels | §1.1, §1.4: `Operator`, an empty `ASAPOp`, `contains_asap()`, `expect_non_asap()`; child slots become `Rc>`; every variant gets `timing` / `guarantee` slots, and nodes are built through constructors that leave both `Unset` | every crate; the same mechanical change everywhere | | 3 One schema | §2.1: `Column` → `Field` and `Schema.columns` → `fields` (serde keeps the name `columns` until stage 4); `FieldType`, `ASAPType`, `PlainField`, `Schema` everywhere except the `executable_dag.rs` wire types, which keep `SummarySchema` until stage 4. **No wire change** | `asap-types` + schema construction in every crate | -| 4 New types | fill `ASAPOp`; `ASAP` arms of `output_schema`; `derive_guarantees` and `derive_timings` (§2.2, §5); the entry check (§3); `flatten(&SummaryNode) -> Rc` so export runs on the new types; wire types become `Schema`, and `Schema.fields` serializes as `fields`. Wire → 6. **The only wire-breaking stage**; merged together with ASAPQuery-backend and ASAPCollector | `asap-types`, `devtools`, viewer | +| 4 New types | fill `ASAPOp`; `ASAP` arms of `output_schema`; `derive_guarantees` and `derive_timings` (§2.2, §5); the entry check (§3); `flatten(&SummaryNode) -> Rc` so export runs on the new types, copying each node's guarantee and today's derived timing into the slots, so the export is unchanged; wire types become `Schema`, and `Schema.fields` serializes as `fields`. Wire → 6. **The only wire-breaking stage**; merged together with ASAPQuery-backend and ASAPCollector | `asap-types`, `devtools`, viewer | | 5 Planner | §4: candidates and assembly on `Rc`; §7 moves to the new types; delete `flatten` | `asap-aware-mapping` | | 6 Cleanup | delete `SummaryExpr`, `SummaryNode`, extra `ValueOperation` variants, `ExactOperation`, `post_asap/cse.rs`; update `post-asap-ir.md`, `physical-plan-integration.md`, developer and viewer docs | docs |