You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
L3Expr = Expr<ColumnId> — positional, produced by the Binder resolving every ColumnRef against the in-scope schema.
They're a separate type family from the canonical QueryExpr operator tree, only reachable through it via wrapper fields: Scan.predicates: Vec<Predicate>, Filter.pred: Predicate(L3Expr), Aggregate.having: Option<Predicate>, Project.items: Vec<ProjectItem { expr: L3Expr, .. }>, Sort.keys: Vec<SortKey { expr: L3Expr, .. }>, and BinaryOp's scalar operand.
Proposal
Drop the separate Expr<C> family. Fold its variants (Column/Literal/Compare/BoolAnd/BoolOr/Not/IsNull/IsNotNull/Cast/InList/FunctionCall/Arith/Case) directly into QueryExpr, and make QueryExpr itself generic over the column-reference type the same way Expr<C> already is — one tree type instead of two, with a name-based version (QueryExpr<ColumnRef>, what front ends build) and a positional version (QueryExpr<ColumnId>, what the Binder produces) instead of today's split between a name-based/positional scalar pair sitting inside a single canonical operator tree.
Motivation
One tree type to learn/extend instead of two — fewer wrapper structs (Predicate, ProjectItem, SortKey) whose only job is "carry an L3Expr plus a bit of metadata."
BinaryOpKind already reuses L3Expr's ArithOp/CompareOp operator enums (query_expr.rs:186-192, explicitly commented as "shared with L3Expr::Arith" / "shared with L3Expr::Compare") — the operator vocabulary is already shared between the two trees; this closes the remaining gap by sharing the tree shape too.
Open questions / risks (flagging before work starts, not yet assessed)
Today, typing Filter.pred/ProjectItem.expr/SortKey.expr/Aggregate.having as L3Expr makes it a compile error to nest a relational node (e.g. a whole Aggregate or Scan subtree) inside a WHERE clause or projection — L3Expr's variant set is closed to scalar shapes only. If scalar variants move into QueryExpr, every one of those positions becomes representable as anyQueryExpr variant, so that invariant needs a replacement (a sub-enum/marker-trait bound enforced at construction, or runtime validation in every consumer) or it's lost.
QueryExpr nodes carry a row schema (output_schema()); Expr<C> nodes carry a scalar type (infer_expr_type()). These are different attached properties over different tree shapes (row-producing operators vs. row-to-single-scalar expressions). Need to decide whether a merged type carries both kinds of typing per node, or whether output_schema()/infer_expr_type() stay as two functions pattern-matching disjoint subsets of one larger enum.
Blast radius: L3Expr currently appears in ~14 files — crates/types/src/intent_algebra/{query_expr.rs,expr_ir.rs,mod.rs}, crates/types/src/dag_export.rs, crates/l2/src/{relational.rs,column_resolution.rs,canonicalize.rs,lower.rs}, crates/asap-aware-mapping/src/bind.rs, both front ends' test suites, and crates/integration-tests. Every one needs updating regardless of which resolution above is picked.
Touches asap-types (the crate every other crate depends on) — expr_ir.rs, query_expr.rs, dag_export.rs — plus every current consumer of L2Expr/L3Expr/Predicate/ProjectItem/SortKey listed above. Expect a multi-PR migration, same caveat as #179.
Summary
L2Expr/L3Expr(crates/types/src/intent_algebra/expr_ir.rs) are one generic scalar-expression tree,Expr<C>, aliased twice:L2Expr = Expr<ColumnRef>— name-based, front-end-emitted.L3Expr = Expr<ColumnId>— positional, produced by the Binder resolving everyColumnRefagainst the in-scope schema.They're a separate type family from the canonical
QueryExproperator tree, only reachable through it via wrapper fields:Scan.predicates: Vec<Predicate>,Filter.pred: Predicate(L3Expr),Aggregate.having: Option<Predicate>,Project.items: Vec<ProjectItem { expr: L3Expr, .. }>,Sort.keys: Vec<SortKey { expr: L3Expr, .. }>, andBinaryOp's scalar operand.Proposal
Drop the separate
Expr<C>family. Fold its variants (Column/Literal/Compare/BoolAnd/BoolOr/Not/IsNull/IsNotNull/Cast/InList/FunctionCall/Arith/Case) directly intoQueryExpr, and makeQueryExpritself generic over the column-reference type the same wayExpr<C>already is — one tree type instead of two, with a name-based version (QueryExpr<ColumnRef>, what front ends build) and a positional version (QueryExpr<ColumnId>, what the Binder produces) instead of today's split between a name-based/positional scalar pair sitting inside a single canonical operator tree.Motivation
Predicate,ProjectItem,SortKey) whose only job is "carry anL3Exprplus a bit of metadata."QueryExpritself carry an "unresolved column-reference state... or a generic parameter" that the Binder resolves via a single generic substitution walk, rather than a dedicated lowering pass. This issue proposes extending that same generic parameter to the currently-separate scalar sublanguage too, so the whole tree — operators and scalar expressions alike — swapsColumnRefforColumnIdin one pass.BinaryOpKindalready reusesL3Expr'sArithOp/CompareOpoperator enums (query_expr.rs:186-192, explicitly commented as "shared withL3Expr::Arith" / "shared withL3Expr::Compare") — the operator vocabulary is already shared between the two trees; this closes the remaining gap by sharing the tree shape too.Open questions / risks (flagging before work starts, not yet assessed)
Filter.pred/ProjectItem.expr/SortKey.expr/Aggregate.havingasL3Exprmakes it a compile error to nest a relational node (e.g. a wholeAggregateorScansubtree) inside a WHERE clause or projection —L3Expr's variant set is closed to scalar shapes only. If scalar variants move intoQueryExpr, every one of those positions becomes representable as anyQueryExprvariant, so that invariant needs a replacement (a sub-enum/marker-trait bound enforced at construction, or runtime validation in every consumer) or it's lost.QueryExprnodes carry a row schema (output_schema());Expr<C>nodes carry a scalar type (infer_expr_type()). These are different attached properties over different tree shapes (row-producing operators vs. row-to-single-scalar expressions). Need to decide whether a merged type carries both kinds of typing per node, or whetheroutput_schema()/infer_expr_type()stay as two functions pattern-matching disjoint subsets of one larger enum.L3Exprcurrently appears in ~14 files —crates/types/src/intent_algebra/{query_expr.rs,expr_ir.rs,mod.rs},crates/types/src/dag_export.rs,crates/l2/src/{relational.rs,column_resolution.rs,canonicalize.rs,lower.rs},crates/asap-aware-mapping/src/bind.rs, both front ends' test suites, andcrates/integration-tests. Every one needs updating regardless of which resolution above is picked.QueryExpr's three predicate homes:Scan.predicates/Filter.pred/Aggregate.having, allPredicate-typed today) — worth sequencing rather than landing simultaneously, per Delete crates/l2 (asap-l2) entirely — the original L2 relational tree + its binder/lower/canonicalize plumbing; front ends emit canonical QueryExpr shapes directly #179's own note that its migration should get "a real blast-radius pass... before any code is deleted."Scope
Touches
asap-types(the crate every other crate depends on) —expr_ir.rs,query_expr.rs,dag_export.rs— plus every current consumer ofL2Expr/L3Expr/Predicate/ProjectItem/SortKeylisted above. Expect a multi-PR migration, same caveat as #179.Related
asap_l2::relational::QueryExpr); this issue is the analogous question one level down, for the scalar tree.QueryExpr's threePredicate-typed filter homes; anyL3Expr→QueryExprfold touches all three.🤖 Generated with Claude Code