Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions crates/frontend-promql/src/promql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,8 +601,9 @@ fn walk_histogram(call: &Call) -> Result<L2> {
/// (`HistogramQuantile`), native histograms / raw samples take the sketch-able
/// `Quantile` (issues #43 / #79) — so the two functions cannot diverge.
///
/// The vector argument is lowered once per branch. That is a duplicated subtree
/// by construction; `plan::cse` hoists it back into a single producer.
/// The vector argument is lowered once per branch, duplicating the subtree —
/// a future workload-level reuse pass could hoist it back into a single
/// producer.
///
/// Each branch aliases its value column to `value` rather than taking the
/// intent-keyed name (`quantile_0_5`, `quantile_0_9`, …). `Merge` derives its
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,10 @@ fn intents(e: &QueryExpr) -> Vec<AggIntent> {
go(rhs, out);
}
QueryExpr::Merge { children } => children.iter().for_each(|c| go(c, out)),
QueryExpr::LetBinding { expr, child, .. } => {
go(expr, out);
go(child, out);
}
QueryExpr::VectorFromScalar(inner) | QueryExpr::ScalarFromVector(inner) => {
go(inner, out)
}
QueryExpr::Scan { .. }
| QueryExpr::Scalar(_)
| QueryExpr::EvalTime
| QueryExpr::Ref { .. } => {}
QueryExpr::Scan { .. } | QueryExpr::Scalar(_) | QueryExpr::EvalTime => {}
}
}
go(e, &mut out);
Expand Down
9 changes: 1 addition & 8 deletions crates/frontend-promql/tests/promql_conformance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,10 @@ fn collect(e: &QueryExpr, out: &mut Vec<AggIntent>) {
collect(right, out);
}
QueryExpr::Merge { children } => children.iter().for_each(|c| collect(c, out)),
QueryExpr::LetBinding { expr, child, .. } => {
collect(expr, out);
collect(child, out);
}
QueryExpr::VectorFromScalar(inner) | QueryExpr::ScalarFromVector(inner) => {
collect(inner, out)
}
QueryExpr::Scan { .. }
| QueryExpr::Scalar(_)
| QueryExpr::EvalTime
| QueryExpr::Ref { .. } => {}
QueryExpr::Scan { .. } | QueryExpr::Scalar(_) | QueryExpr::EvalTime => {}
}
}

Expand Down
5 changes: 3 additions & 2 deletions crates/frontend-sql/src/sql/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,9 @@ impl<'a> SqlLowerer<'a> {
/// the branches union-compatible, which `Merge` requires (it derives its
/// schema from the first child).
///
/// `Aggregate.child` is duplicated per level. `plan::cse` hoists it back into
/// a single producer — the same trade `histogram_quantiles` makes (#109).
/// `Aggregate.child` is duplicated per level — the same trade
/// `histogram_quantiles` makes (#109); a future workload-level reuse pass
/// could hoist it back into a single producer.
///
/// DataFusion's `__grouping_id` discriminator is dropped: it only exists to
/// tell a subtotal's `NULL` apart from a data `NULL`, which is observable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,10 @@ fn intents(e: &QueryExpr) -> Vec<AggIntent> {
go(rhs, out);
}
QueryExpr::Merge { children } => children.iter().for_each(|c| go(c, out)),
QueryExpr::LetBinding { expr, child, .. } => {
go(expr, out);
go(child, out);
}
QueryExpr::VectorFromScalar(inner) | QueryExpr::ScalarFromVector(inner) => {
go(inner, out)
}
QueryExpr::Scan { .. }
| QueryExpr::Scalar(_)
| QueryExpr::EvalTime
| QueryExpr::Ref { .. } => {}
QueryExpr::Scan { .. } | QueryExpr::Scalar(_) | QueryExpr::EvalTime => {}
}
}
go(e, &mut out);
Expand Down Expand Up @@ -152,9 +145,6 @@ fn has_window_func(qe: &QueryExpr) -> bool {
| QueryExpr::Sort { child, .. }
| QueryExpr::Limit { child, .. }
| QueryExpr::Subquery { child, .. } => has_window_func(child),
QueryExpr::LetBinding { expr, child, .. } => {
has_window_func(expr) || has_window_func(child)
}
_ => false,
}
}
Expand Down
9 changes: 1 addition & 8 deletions crates/frontend-sql/tests/netflow/netflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,14 +293,7 @@ fn visit(qe: &QueryExpr, f: &mut impl FnMut(&QueryExpr)) {
visit(child, f);
}
}
QueryExpr::LetBinding { expr, child, .. } => {
visit(expr, f);
visit(child, f);
}
QueryExpr::VectorFromScalar(child) | QueryExpr::ScalarFromVector(child) => visit(child, f),
QueryExpr::Scan { .. }
| QueryExpr::Scalar(_)
| QueryExpr::EvalTime
| QueryExpr::Ref { .. } => {}
QueryExpr::Scan { .. } | QueryExpr::Scalar(_) | QueryExpr::EvalTime => {}
}
}
5 changes: 3 additions & 2 deletions crates/frontend-sql/tests/sql_lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,8 +909,9 @@ async fn a_correlated_in_subquery_is_rejected() {
#[tokio::test]
async fn quantile_carries_its_input_column() {
// `metrics(ts=0, service=1, latency=2, bytes=3)`. Two quantiles over
// different columns must not compare equal — `plan::cse` dedupes on
// `AggIntent` equality, so a col-less intent would collapse them.
// different columns must not compare equal — a workload-level dedupe pass
// would compare on `AggIntent` equality, so a col-less intent would
// collapse them.
let qe = lower(
"SELECT approx_percentile_cont(latency, 0.5), \
approx_percentile_cont(bytes, 0.5) FROM metrics",
Expand Down
34 changes: 3 additions & 31 deletions crates/ir/src/dag_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@
//! structural hash per node, so a caller with several exported queries can
//! spot identical subtrees (a shared `Scan`, a repeated `Aggregate` shape,
//! …) by comparing hashes rather than re-implementing `QueryExpr: PartialEq`
//! structural comparison client-side. This is a proxy for real CSE output
//! (`asap_plan::cse::dedupe_subtrees`), not a replacement for it — that pass
//! isn't wired into any end-to-end multi-root pipeline today, so its
//! `bindings`/`Ref` shape isn't something an exported single query exposes.
//! structural comparison client-side.

use serde::Serialize;
use std::hash::{Hash, Hasher};
Expand All @@ -31,7 +28,7 @@ pub struct DagNode {
pub label: String,
pub detail: serde_json::Value,
/// Child node ids, in the variant's field order (e.g. `Join` is
/// `[left, right]`, `LetBinding` is `[expr, child]`).
/// `[left, right]`).
pub children: Vec<u32>,
/// Bottom-up structural hash: two nodes hash equally iff their `kind`,
/// `detail`, and (recursively) their children's hashes all match.
Expand Down Expand Up @@ -124,7 +121,7 @@ fn source_label(source: &Source) -> String {
/// (children pushed before their parent), and return the id of the pushed
/// root node. Exhaustive over every `QueryExpr` variant — a new variant
/// fails to compile here until this match is extended, matching the rest of
/// the IR's exhaustive-match style (e.g. `output_schema_in`, `plan::cse`).
/// the IR's exhaustive-match style (e.g. `output_schema`).
fn build(expr: &QueryExpr, nodes: &mut Vec<DagNode>) -> u32 {
match expr {
QueryExpr::Scan {
Expand All @@ -140,16 +137,6 @@ fn build(expr: &QueryExpr, nodes: &mut Vec<DagNode>) -> u32 {
});
push_node(nodes, "Scan", label, detail, vec![])
}
QueryExpr::Ref { name } => {
let detail = serde_json::json!({ "name": name });
push_node(
nodes,
"Ref",
format!("Ref({})", name.as_str()),
detail,
vec![],
)
}
QueryExpr::Scalar(v) => {
let detail = serde_json::json!({ "value": v });
push_node(nodes, "Scalar", format!("Scalar({v})"), detail, vec![])
Expand Down Expand Up @@ -330,21 +317,6 @@ fn build(expr: &QueryExpr, nodes: &mut Vec<DagNode>) -> u32 {
let detail = serde_json::json!({ "n": n, "offset": offset });
push_node(nodes, "Limit", format!("Limit({n})"), detail, vec![c])
}
QueryExpr::LetBinding { name, expr, child } => {
// Two children with asymmetric roles: `expr` (the bound producer)
// and `child` (the body); order is [expr, child] so an index into
// `children` always means the same thing across `LetBinding` nodes.
let e = build(expr, nodes);
let c = build(child, nodes);
let detail = serde_json::json!({ "name": name });
push_node(
nodes,
"LetBinding",
format!("LetBinding({})", name.as_str()),
detail,
vec![e, c],
)
}
QueryExpr::Subquery {
range,
resolution,
Expand Down
11 changes: 4 additions & 7 deletions crates/ir/src/intent_algebra/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,14 @@
//! - [`expr_ir`] — the scalar expression IR ([`L2Expr`] / [`L3Expr`] /
//! [`ColumnRef`]) shared by L2 and L3.
//! - [`schema`] — the per-edge [`Schema`] every L3 node carries.
//! - [`names`] — binding / query identifiers.
//!
//! The Layer-2 relational tree and the L2→L3 converter (`convert_root`, the
//! `Binder`, column resolution) live in the `asap-l2` crate — front ends need
//! them, but L3-only consumers (optimizer, sketch) do not, so they stay out of
//! this crate. Workload-level CSE lives in `asap-plan`.
//! this crate.

pub mod agg_intent;
pub mod expr_ir;
pub mod names;
pub mod query_expr;
pub mod schema;

Expand All @@ -25,11 +23,10 @@ pub use agg_intent::{
is_frequency_heavy_hitter, ranking_measure, AggIntent, MathFunc, RankingMeasure, TimeFunc,
};
pub use expr_ir::{ArithOp, ColumnRef, CompareOp, Expr, L2Expr, L3Expr, L3Scalar};
pub use names::{BindingName, QueryId};
pub use query_expr::{
aggregate_output_schema, AtModifier, BinaryOpKind, BindingScope, DataModel, GroupKeys,
GroupSide, InfoMatcher, JoinKind, Predicate, ProjectItem, QueryExpr, QueryExprError, Reduction,
aggregate_output_schema, AtModifier, BinaryOpKind, DataModel, GroupKeys, GroupSide,
InfoMatcher, JoinKind, Predicate, ProjectItem, QueryExpr, QueryExprError, Reduction,
SampleKind, SetOpKind, SortKey, Source, TimeShift, VectorGrouping, VectorMatch,
VectorMatchKind, WindowFuncKind, WindowKind,
};
pub use schema::{cse_reuse_is_legal, Column, ColumnId, CseError, DataType, Schema};
pub use schema::{Column, ColumnId, DataType, Schema};
27 changes: 0 additions & 27 deletions crates/ir/src/intent_algebra/names.rs

This file was deleted.

Loading
Loading