Skip to content

Commit df39d3a

Browse files
zz_yclaude
andcommitted
refactor(ir): split L2 relational + converter into asap-l2, slim asap-ir to L3
Per @milind's #57 feedback ("a lot fits into asap-ir"). asap-ir was 3.9k LOC holding L2 relational + L3 canonical IR + the L2→L3 converter + binder + resolution. Split along the real (non-cyclic) seam: - asap-ir (2.3k, was 3.9k): the L3 canonical IR only — query_expr, agg_intent, expr_ir, schema, names (+ types, workload). - asap-l2 (1.6k, new): the L2 relational algebra + the L2→L3 converter — relational, lower (convert_root), binder, column_resolution. Depends on asap-ir. The apparent L2↔L3 module cycles were doc-comment links, not code deps; the real graph layers cleanly. Isolation wins: - asap-sketch and asap-plan now depend on asap-ir alone — they never pull the converter/binder machinery (cargo tree: 0 asap-l2 in either). - Front ends depend on asap-ir (L3 types) + asap-l2 (relational + convert_root). - Parser quarantine unchanged (promql frontend: 0 datafusion). Docs (README, design.md §5.1/§6.0/P1, migration-plan) updated: dependency table, directory tree, and the "why L2 and L3 are separate crates" rationale. No behavior change; full workspace suite green; clippy --all-targets clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c770165 commit df39d3a

20 files changed

Lines changed: 177 additions & 130 deletions

File tree

‎Cargo.lock‎

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
[workspace]
22
members = [
33
"crates/ir",
4+
"crates/l2",
45
"crates/sketch",
56
"crates/plan",
67
"crates/frontend-promql",

‎README.md‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,38 +39,40 @@ runtime coupling.
3939

4040
| Crate | Role | Depends on | ~LOC |
4141
|---|---|---|---|
42-
| **`asap-ir`** | L2 relational + L3 canonical IR + L2→L3 converter, binder, resolution, schema, expr, workload types | *(nothing — no query-language deps)* | 3,900 |
42+
| **`asap-ir`** | **L3 canonical IR only** — `QueryExpr` + `AggIntent` + scalar expr IR + schema + names | *(nothing — no query-language deps)* | 2,300 |
43+
| `asap-l2` | L2 per-language relational algebra + the L2→L3 converter (`convert_root`, binder, column resolution) | `asap-ir` | 1,600 |
4344
| `asap-sketch` | L4 sketch-bound IR (`SketchExpr`) | `asap-ir` | 240 |
4445
| `asap-plan` | optimizer layer — CSE (landed); cost-model / boundary / canonicalize (stubs) | `asap-ir` | 290 |
45-
| `asap-frontend-promql` | PromQL L1→L2 | `asap-ir`, **promql-parser** | 1,030 |
46-
| `asap-frontend-sql` | SQL L1→L2 | `asap-ir`, **datafusion** | 1,130 |
46+
| `asap-frontend-promql` | PromQL L1→L2 | `asap-ir`, `asap-l2`, **promql-parser** | 1,030 |
47+
| `asap-frontend-sql` | SQL L1→L2 | `asap-ir`, `asap-l2`, **datafusion** | 1,130 |
4748
| `asap-lower` | facade re-exporting both front ends | the two `frontend-*` crates | 15 |
4849
| `asap-e2e` | cross-language integration tests | `asap-frontend-promql` | 40 |
4950

50-
Splitting the front ends **quarantines their parsers**: a caller that needs
51-
only PromQL depends on `asap-frontend-promql` and never compiles DataFusion,
52-
and vice-versa (verified with `cargo tree`). `asap-lower` is the convenience
53-
facade for callers that want both.
51+
Two isolation wins fall out of this:
52+
- **The front ends quarantine their parsers** — a caller that needs only PromQL depends on `asap-frontend-promql` and never compiles DataFusion, and vice-versa (verified with `cargo tree`). `asap-lower` is the facade for callers that want both.
53+
- **L3-only consumers stay lean** — `asap-sketch` and `asap-plan` depend on `asap-ir` alone, so they never pull the L2 relational tree, the converter, or the binder (`asap-l2`). Only the front ends, which actually *lower* queries, need `asap-l2`.
5454

5555
### Directory structure
5656

5757
```
5858
crates/
59-
├── ir/ # asap-ir — the shared IR (largest crate)
59+
├── ir/ # asap-ir — L3 canonical IR (the shared vocabulary)
6060
│ └── src/
6161
│ ├── lib.rs
6262
│ ├── types.rs # AccuracyTarget, …
6363
│ ├── workload.rs # QueryWorkload / QueryLanguage / SqlDialect (front-end input)
6464
│ └── intent_algebra/
65-
│ ├── relational.rs # L2: per-language relational tree the front ends emit
6665
│ ├── query_expr.rs # L3: canonical QueryExpr — the IR everything pivots on
6766
│ ├── agg_intent.rs # L3: AggIntent vocabulary (Sum/Quantile/Rate/TopK/…)
68-
│ ├── expr_ir.rs # scalar expr IR (L2Expr / L3Expr)
69-
│ ├── lower.rs # L2→L3 converter (convert_root)
70-
│ ├── binder.rs # positional name-resolution seed
71-
│ ├── column_resolution.rs
67+
│ ├── expr_ir.rs # scalar expr IR (L2Expr / L3Expr / ColumnRef)
7268
│ ├── schema.rs # per-edge Schema + unique-keys
73-
│ └── names.rs
69+
│ └── names.rs # BindingName / QueryId
70+
├── l2/ # asap-l2 — L2 relational algebra + L2→L3 converter
71+
│ └── src/
72+
│ ├── relational.rs # L2: per-language relational tree the front ends emit
73+
│ ├── lower.rs # L2→L3 converter (convert_root)
74+
│ ├── binder.rs # positional name-resolution seed
75+
│ └── column_resolution.rs
7476
├── sketch/ # asap-sketch — L4 sketch IR: expr / schema / sketch
7577
├── plan/ # asap-plan — optimizer: cse.rs (landed)
7678
│ └── src/ # + cost_model.rs / boundary.rs / canonicalize.rs (stubs)
@@ -83,13 +85,14 @@ crates/
8385
# deployment-model-* crates, control-proto, and the bin/ entrypoints.
8486
```
8587

86-
**Why `asap-ir` holds the most.** L2 (the per-language relational tree) and L3
87-
(the canonical intent algebra) live in one crate because the **L2→L3 converter
88-
needs both** — front ends only *emit* L2, they don't own it. Its modules
89-
(`relational` / `query_expr` / `agg_intent` / `schema` / `binder` / …) are
90-
cleanly separated and could split into their own crates later if the crate
91-
grows unwieldy; for now they share one compilation unit to keep the converter's
92-
tight coupling in-crate.
88+
**Why L2 and L3 are separate crates.** `asap-ir` is the canonical L3 IR — the
89+
vocabulary every downstream layer pivots on. The L2 relational tree and the
90+
L2→L3 converter live in `asap-l2` because only the *front ends* need them: they
91+
emit L2 and call `convert_root`. Keeping them out of `asap-ir` means the
92+
optimizer (`asap-plan`), the sketch IR (`asap-sketch`), and any future
93+
L3-consuming layer compile against a lean core without the converter/binder
94+
machinery. (The converter co-locates with L2 rather than L3 because it owns the
95+
L2 tree definition and only *reads* L3.)
9396

9497
*(Planned.)* A new deployment model will land by adding one crate with `rules.rs` (pick L4 rules) + `topology.rs` + an emitter, plus one line in `bin/asap-controller/main.rs` — no changes to the IR crates.
9598

‎crates/frontend-promql/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ edition = "2021"
77
# in asap-ir. Pulls the PromQL parser only — never DataFusion.
88
[dependencies]
99
asap-ir = { path = "../ir" }
10+
asap-l2 = { path = "../l2" }
1011

1112
# Private mirror of GreptimeTeam/promql-parser (Apache-2.0). `main` tracks
1213
# upstream; our `asap` branch carries the local patches.

‎crates/frontend-promql/src/error.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::fmt;
22

3-
use asap_ir::intent_algebra::ConvertError;
3+
use asap_l2::ConvertError;
44

55
/// Errors from lowering a PromQL query (L1 parse → L2 → shared L2→L3 convert).
66
///

‎crates/frontend-promql/src/lib.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
//! PromQL front end: L1 (parse via `promql-parser`) → L2 relational, then the
2-
//! shared L2→L3 [`convert_root`](asap_ir::intent_algebra::convert_root).
2+
//! shared L2→L3 [`convert_root`](asap_l2::convert_root).
33
//!
44
//! Emits the per-language
5-
//! [`relational::QueryExpr`](asap_ir::intent_algebra::relational); the shared
6-
//! converter runs the [`Binder`](asap_ir::intent_algebra::Binder) for
5+
//! [`relational::QueryExpr`](asap_l2::relational); the shared
6+
//! converter runs the [`Binder`](asap_l2::Binder) for
77
//! positional name resolution. Depends on the PromQL parser only — never on the
88
//! SQL / DataFusion stack.
99
1010
pub mod error;
1111
pub mod promql;
1212

13-
use asap_ir::intent_algebra::{convert_root, QueryExpr};
13+
use asap_ir::intent_algebra::QueryExpr;
14+
use asap_l2::convert_root;
1415
use asap_ir::types::AccuracyTarget;
1516
use asap_ir::workload::{QueryLanguage, QueryWorkload};
1617

‎crates/frontend-promql/src/promql.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//! - **L2 (per-language tree)** is built here: the walk interprets PromQL
55
//! semantics (range vectors, aggregate operators, label matchers) and emits
66
//! the language-flavored [`relational::QueryExpr`] the controller's L2→L3
7-
//! converter ([`convert_root`](asap_ir::intent_algebra::convert_root))
7+
//! converter ([`convert_root`](asap_l2::convert_root))
88
//! consumes. Canonicalisation (window-over-aggregate fold, GROUP-BY →
99
//! positional `Aggregate.by`, positional name binding) happens in that
1010
//! converter, not here.
@@ -44,7 +44,7 @@ use promql_parser::parser::{
4444
use asap_ir::intent_algebra::query_expr::{
4545
BinaryOpKind, GroupSide, VectorGrouping, VectorMatch, VectorMatchKind,
4646
};
47-
use asap_ir::intent_algebra::relational::{
47+
use asap_l2::relational::{
4848
AggFunc, AggItem, L2SortKey, QueryExpr as L2, SourceSpec,
4949
};
5050
use asap_ir::intent_algebra::{ArithOp, ColumnRef, CompareOp, L2Expr, L3Scalar};

‎crates/frontend-sql/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ edition = "2021"
77
# shared L2→L3 converter in asap-ir. Pulls DataFusion only — never promql-parser.
88
[dependencies]
99
asap-ir = { path = "../ir" }
10+
asap-l2 = { path = "../l2" }
1011
datafusion = "43"
1112

1213
[dev-dependencies]

‎crates/frontend-sql/src/error.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::fmt;
22

3-
use asap_ir::intent_algebra::ConvertError;
3+
use asap_l2::ConvertError;
44

55
/// Errors from lowering a SQL query (L1 parse + plan via DataFusion → L2 →
66
/// shared L2→L3 convert).

‎crates/frontend-sql/src/lib.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
//! SQL front end: L1 (parse + plan via DataFusion) → L2 relational, then the
2-
//! shared L2→L3 [`convert_root`](asap_ir::intent_algebra::convert_root).
2+
//! shared L2→L3 [`convert_root`](asap_l2::convert_root).
33
//!
44
//! Emits the per-language
5-
//! [`relational::QueryExpr`](asap_ir::intent_algebra::relational); the shared
6-
//! converter runs the [`Binder`](asap_ir::intent_algebra::Binder) for
5+
//! [`relational::QueryExpr`](asap_l2::relational); the shared
6+
//! converter runs the [`Binder`](asap_l2::Binder) for
77
//! positional name resolution. Depends on DataFusion only — never on the PromQL
88
//! parser.
99
1010
pub mod error;
1111
pub mod sql;
1212

13-
use asap_ir::intent_algebra::{convert_root, QueryExpr};
13+
use asap_ir::intent_algebra::QueryExpr;
14+
use asap_l2::convert_root;
1415
use asap_ir::types::AccuracyTarget;
1516
use asap_ir::workload::{QueryLanguage, QueryWorkload, SqlDialect};
1617

0 commit comments

Comments
 (0)