From ea0bc7487d29265c591e72ec6affee0171432538 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Fri, 14 Aug 2026 13:23:12 -0400 Subject: [PATCH] =?UTF-8?q?Remove=20QueryExpr::Window=20=E2=80=94=20no=20p?= =?UTF-8?q?roducer=20exists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L2→L3 conversion collapses every L2 Window (bare range vector) into canonical TimeRange unconditionally (crates/l2/src/lower.rs); nothing ever constructs canonical QueryExpr::Window. Confirmed empirically by walking every lowered query across all 7 corpora (~2600 queries). Drops the variant, its output_schema arm, WindowKind (its only field type, now itself unused), and every match arm across dag_export, canonicalize's children walker, variant_coverage, and test files. L2's own relational::QueryExpr::Window (a distinct, still-live type) is untouched. Fixes #182 Co-Authored-By: Claude Sonnet 5 --- .../tests/histogram_metadata.rs | 3 +- .../awesome_prometheus_alerts.rs | 3 +- .../tests/promql_conformance.rs | 13 ++-- .../frontend-promql/tests/promql_lowering.rs | 7 +-- .../synthetic_packet_trace.rs | 3 +- crates/frontend-sql/tests/netflow/netflow.rs | 2 - crates/frontend-sql/tests/sql_lowering.rs | 5 -- crates/ir/src/dag_export.rs | 16 ----- crates/ir/src/intent_algebra/mod.rs | 2 +- crates/ir/src/intent_algebra/query_expr.rs | 60 +------------------ crates/l2/src/canonicalize.rs | 1 - crates/lower/src/bin/variant_coverage.rs | 5 -- 12 files changed, 12 insertions(+), 108 deletions(-) diff --git a/crates/frontend-promql/tests/histogram_metadata.rs b/crates/frontend-promql/tests/histogram_metadata.rs index cb2ce65e..03c2b910 100644 --- a/crates/frontend-promql/tests/histogram_metadata.rs +++ b/crates/frontend-promql/tests/histogram_metadata.rs @@ -24,8 +24,7 @@ fn quantile_kind(qe: &QueryExpr) -> &'static str { _ => None, }) .or_else(|| walk(child)), - QueryExpr::Window { child, .. } - | QueryExpr::TimeRange { child, .. } + QueryExpr::TimeRange { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } diff --git a/crates/frontend-promql/tests/observability/awesome_prometheus_alerts.rs b/crates/frontend-promql/tests/observability/awesome_prometheus_alerts.rs index 59ad64db..3b573c01 100644 --- a/crates/frontend-promql/tests/observability/awesome_prometheus_alerts.rs +++ b/crates/frontend-promql/tests/observability/awesome_prometheus_alerts.rs @@ -55,8 +55,7 @@ fn intents(e: &QueryExpr) -> Vec { out.extend(aggs.iter().cloned()); go(child, out); } - QueryExpr::Window { child, .. } - | QueryExpr::TimeRange { child, .. } + QueryExpr::TimeRange { child, .. } | QueryExpr::TimeShift { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Sort { child, .. } diff --git a/crates/frontend-promql/tests/promql_conformance.rs b/crates/frontend-promql/tests/promql_conformance.rs index c0927467..90eca026 100644 --- a/crates/frontend-promql/tests/promql_conformance.rs +++ b/crates/frontend-promql/tests/promql_conformance.rs @@ -70,8 +70,7 @@ fn collect(e: &QueryExpr, out: &mut Vec) { out.extend(aggs.iter().cloned()); collect(child, out); } - QueryExpr::Window { child, .. } - | QueryExpr::TimeRange { child, .. } + QueryExpr::TimeRange { child, .. } | QueryExpr::TimeShift { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Sort { child, .. } @@ -112,8 +111,7 @@ fn first_scan(e: &QueryExpr) -> (String, usize) { }; (name, predicates.len()) } - QueryExpr::Window { child, .. } - | QueryExpr::TimeRange { child, .. } + QueryExpr::TimeRange { child, .. } | QueryExpr::TimeShift { child, .. } | QueryExpr::Aggregate { child, .. } | QueryExpr::Filter { child, .. } @@ -211,8 +209,8 @@ fn name_regex_matcher_is_rejected__GAP() { #[test] fn range_vector_selector_is_time_range() { - // SEMANTICS: `[5m]` turns an instant vector into a range vector. - // In L3 this is a dedicated `TimeRange` node (not a streaming `Window`). + // SEMANTICS: `[5m]` turns an instant vector into a range vector, + // represented in L3 as a dedicated `TimeRange` node. let qe = ok("node_cpu_seconds_total[5m]"); let QueryExpr::TimeRange { range, .. } = &qe else { panic!("expected TimeRange for a range-vector selector, got {qe:?}"); @@ -2010,8 +2008,7 @@ fn first_relabel(e: &QueryExpr) -> &QueryExpr { QueryExpr::Aggregate { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::TimeRange { child, .. } - | QueryExpr::TimeShift { child, .. } - | QueryExpr::Window { child, .. } => first_relabel(child), + | QueryExpr::TimeShift { child, .. } => first_relabel(child), other => panic!("no Relabel reachable from {other:?}"), } } diff --git a/crates/frontend-promql/tests/promql_lowering.rs b/crates/frontend-promql/tests/promql_lowering.rs index 51df670a..3a8b62d3 100644 --- a/crates/frontend-promql/tests/promql_lowering.rs +++ b/crates/frontend-promql/tests/promql_lowering.rs @@ -516,8 +516,7 @@ fn collect_intents(e: &QueryExpr, out: &mut Vec) { out.extend(aggs.iter().cloned()); collect_intents(child, out); } - QueryExpr::Window { child, .. } - | QueryExpr::TimeRange { child, .. } + QueryExpr::TimeRange { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } => collect_intents(child, out), @@ -539,7 +538,6 @@ fn scan_columns(e: &QueryExpr) -> Vec { match e { QueryExpr::Scan { schema, .. } => schema.columns.iter().map(|c| c.name.clone()).collect(), QueryExpr::Aggregate { child, .. } - | QueryExpr::Window { child, .. } | QueryExpr::TimeRange { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Sort { child, .. } @@ -666,8 +664,7 @@ fn scan_schema_carries_ts_value_and_group_keys() { fn find_scan(n: &QueryExpr) -> &QueryExpr { match n { QueryExpr::Scan { .. } => n, - QueryExpr::Window { child, .. } - | QueryExpr::TimeRange { child, .. } + QueryExpr::TimeRange { child, .. } | QueryExpr::Aggregate { child, .. } | QueryExpr::Filter { child, .. } => find_scan(child), other => panic!("unexpected node {other:?}"), diff --git a/crates/frontend-sql/tests/data_quality_check/synthetic_packet_trace.rs b/crates/frontend-sql/tests/data_quality_check/synthetic_packet_trace.rs index e5b16d36..f751f533 100644 --- a/crates/frontend-sql/tests/data_quality_check/synthetic_packet_trace.rs +++ b/crates/frontend-sql/tests/data_quality_check/synthetic_packet_trace.rs @@ -77,8 +77,7 @@ fn intents(e: &QueryExpr) -> Vec { out.extend(aggs.iter().cloned()); go(child, out); } - QueryExpr::Window { child, .. } - | QueryExpr::TimeRange { child, .. } + QueryExpr::TimeRange { child, .. } | QueryExpr::TimeShift { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Sort { child, .. } diff --git a/crates/frontend-sql/tests/netflow/netflow.rs b/crates/frontend-sql/tests/netflow/netflow.rs index f4e1d89d..8f7c7c19 100644 --- a/crates/frontend-sql/tests/netflow/netflow.rs +++ b/crates/frontend-sql/tests/netflow/netflow.rs @@ -197,7 +197,6 @@ fn first_aggregate(qe: &QueryExpr) -> Option<(&GroupKeys, &Vec)> { } => Some((reduction.expect_reduce(), aggs)), QueryExpr::Project { child, .. } | QueryExpr::Filter { child, .. } - | QueryExpr::Window { child, .. } | QueryExpr::Distinct { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } @@ -263,7 +262,6 @@ fn visit(qe: &QueryExpr, f: &mut impl FnMut(&QueryExpr)) { QueryExpr::Project { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Aggregate { child, .. } - | QueryExpr::Window { child, .. } | QueryExpr::TimeRange { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } diff --git a/crates/frontend-sql/tests/sql_lowering.rs b/crates/frontend-sql/tests/sql_lowering.rs index 385c6072..acca7c86 100644 --- a/crates/frontend-sql/tests/sql_lowering.rs +++ b/crates/frontend-sql/tests/sql_lowering.rs @@ -55,7 +55,6 @@ fn find_aggregate(qe: &QueryExpr) -> Option<(&GroupKeys, &Vec)> { } => Some((reduction.expect_reduce(), aggs)), QueryExpr::Project { child, .. } | QueryExpr::Filter { child, .. } - | QueryExpr::Window { child, .. } | QueryExpr::Distinct { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } @@ -101,7 +100,6 @@ fn find_join(qe: &QueryExpr) -> Option<&QueryExpr> { QueryExpr::Project { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Aggregate { child, .. } - | QueryExpr::Window { child, .. } | QueryExpr::Distinct { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } @@ -116,7 +114,6 @@ fn find_filter(qe: &QueryExpr) -> Option<&QueryExpr> { QueryExpr::Filter { .. } => Some(qe), QueryExpr::Project { child, .. } | QueryExpr::Aggregate { child, .. } - | QueryExpr::Window { child, .. } | QueryExpr::Distinct { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } @@ -656,7 +653,6 @@ fn find_windowfunc(qe: &QueryExpr) -> Option<&QueryExpr> { QueryExpr::Project { child, .. } | QueryExpr::Filter { child, .. } | QueryExpr::Aggregate { child, .. } - | QueryExpr::Window { child, .. } | QueryExpr::Distinct { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } @@ -727,7 +723,6 @@ fn all_intents(qe: &QueryExpr) -> Vec { } QueryExpr::Project { child, .. } | QueryExpr::Filter { child, .. } - | QueryExpr::Window { child, .. } | QueryExpr::Distinct { child, .. } | QueryExpr::Sort { child, .. } | QueryExpr::Limit { child, .. } diff --git a/crates/ir/src/dag_export.rs b/crates/ir/src/dag_export.rs index a4c87b39..a1625145 100644 --- a/crates/ir/src/dag_export.rs +++ b/crates/ir/src/dag_export.rs @@ -237,22 +237,6 @@ fn build(expr: &QueryExpr, nodes: &mut Vec) -> u32 { vec![c], ) } - QueryExpr::Window { - kind, - size, - slide, - child, - } => { - let c = build(child, nodes); - let detail = serde_json::json!({ "kind": kind, "size": size, "slide": slide }); - push_node( - nodes, - "Window", - format!("Window({kind:?})"), - detail, - vec![c], - ) - } QueryExpr::Distinct { cols, child } => { let c = build(child, nodes); let detail = serde_json::json!({ "cols": cols }); diff --git a/crates/ir/src/intent_algebra/mod.rs b/crates/ir/src/intent_algebra/mod.rs index 6611bb52..f4047f16 100644 --- a/crates/ir/src/intent_algebra/mod.rs +++ b/crates/ir/src/intent_algebra/mod.rs @@ -27,6 +27,6 @@ pub use query_expr::{ 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, + VectorMatchKind, WindowFuncKind, }; pub use schema::{Column, ColumnId, DataType, Schema}; diff --git a/crates/ir/src/intent_algebra/query_expr.rs b/crates/ir/src/intent_algebra/query_expr.rs index 372c4750..205050fe 100644 --- a/crates/ir/src/intent_algebra/query_expr.rs +++ b/crates/ir/src/intent_algebra/query_expr.rs @@ -151,47 +151,6 @@ impl<'de> Deserialize<'de> for GroupKeys { } } -/// Lifecycle / flush semantics of a streaming time window. -/// -/// `Copy`/`Default`/`Hash`/`Display`/`FromStr` and `#[serde(rename_all = -/// "snake_case")]` (matching this file's `WindowFuncKind` neighbor) were -/// added so a downstream backend's own `WindowType`-shaped serving-time -/// enum (data-plane accumulator config: `Tumbling` default, -/// `"window_type": "tumbling"` wire format, string round-trip via -/// `Display`/`FromStr`) could be retired in favor of this type directly, -/// rather than keeping two independently-maintained enums in sync by hand. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, Default)] -#[serde(rename_all = "snake_case")] -pub enum WindowKind { - #[default] - Tumbling, - Sliding, - Session, -} - -impl std::fmt::Display for WindowKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - WindowKind::Tumbling => write!(f, "tumbling"), - WindowKind::Sliding => write!(f, "sliding"), - WindowKind::Session => write!(f, "session"), - } - } -} - -impl std::str::FromStr for WindowKind { - type Err = String; - - fn from_str(s: &str) -> Result { - match s.to_lowercase().as_str() { - "tumbling" => Ok(WindowKind::Tumbling), - "sliding" => Ok(WindowKind::Sliding), - "session" => Ok(WindowKind::Session), - _ => Err(format!("Unknown window kind: '{s}'")), - } - } -} - /// Which data model a `Source` / `AggIntent` operates over. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum DataModel { @@ -582,16 +541,6 @@ pub enum QueryExpr { child: Box, }, - /// ψ — tumbling / sliding / session window over the time axis. Window - /// over Aggregate is the canonical windowed-aggregate shape. - Window { - kind: WindowKind, - size: Duration, - #[serde(default)] - slide: Option, - child: Box, - }, - /// δ — SQL `DISTINCT` / row deduplication. Positional like every other L3 /// column reference; empty = dedup on all columns (`SELECT DISTINCT *`). Distinct { @@ -665,8 +614,7 @@ pub enum QueryExpr { /// Temporal range selection — "look back `range` of history for this /// computation." Used for all range-vector functions: `rate`, `increase`, - /// `*_over_time`. The range is distinct from both a streaming `Window` - /// (which is for query-repetition) and a row-level `Filter`. + /// `*_over_time`. The range is distinct from a row-level `Filter`. /// /// Structural marker: an `Aggregate` whose direct child is a `TimeRange` /// is a *per-series* reduction (label-preserving); one whose child is a @@ -721,12 +669,6 @@ impl QueryExpr { match self { QueryExpr::Scan { schema, .. } => Ok(schema.clone()), - // ψ — streaming window (tumbling / sliding / session) for query - // repetition. Does not change the column schema; passes through. - // Per-series range reductions (`rate`, `*_over_time`) now use the - // `TimeRange` node instead, so this arm is a simple pass-through. - QueryExpr::Window { child, .. } => child.output_schema(), - QueryExpr::Aggregate { reduction, aggs, diff --git a/crates/l2/src/canonicalize.rs b/crates/l2/src/canonicalize.rs index cc9e28bc..be66dd2e 100644 --- a/crates/l2/src/canonicalize.rs +++ b/crates/l2/src/canonicalize.rs @@ -65,7 +65,6 @@ fn children_mut(expr: &mut QueryExpr) -> Vec<&mut QueryExpr> { | Filter { child, .. } | Project { child, .. } | Aggregate { child, .. } - | Window { child, .. } | Distinct { child, .. } | Subquery { child, .. } | TimeRange { child, .. } diff --git a/crates/lower/src/bin/variant_coverage.rs b/crates/lower/src/bin/variant_coverage.rs index e0843a81..001674ba 100644 --- a/crates/lower/src/bin/variant_coverage.rs +++ b/crates/lower/src/bin/variant_coverage.rs @@ -24,7 +24,6 @@ const ALL_VARIANTS: &[&str] = &[ "Filter", "Project", "Aggregate", - "Window", "Distinct", "Merge", "Join", @@ -81,10 +80,6 @@ fn walk(e: &QueryExpr, seen: &mut BTreeSet<&'static str>) { seen.insert("Aggregate"); walk(child, seen); } - QueryExpr::Window { child, .. } => { - seen.insert("Window"); - walk(child, seen); - } QueryExpr::Distinct { child, .. } => { seen.insert("Distinct"); walk(child, seen);