From 2381b80b181271b60312192d3ecd6582e363fca8 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Fri, 3 Jul 2026 11:54:04 -0400 Subject: [PATCH 1/6] align end timestamp for SQL queries, for all query types --- .../src/engines/simple_engine/sql.rs | 228 +++++++++++++++--- 1 file changed, 192 insertions(+), 36 deletions(-) diff --git a/asap-query-engine/src/engines/simple_engine/sql.rs b/asap-query-engine/src/engines/simple_engine/sql.rs index 2947bbc..25351d3 100644 --- a/asap-query-engine/src/engines/simple_engine/sql.rs +++ b/asap-query-engine/src/engines/simple_engine/sql.rs @@ -171,6 +171,27 @@ impl SimpleEngine { .cloned() } + /// Aligns `end_timestamp` down to the nearest data-ingestion-interval + /// boundary, unconditionally. Unlike the shared, PromQL-oriented + /// `validate_and_align_end_timestamp` (which only snaps for + /// `OnlySpatial`), SQL end timestamps come from explicit `BETWEEN` + /// clauses and should already be interval-aligned, so this is a no-op + /// in the common case and a safety net otherwise — for every SQL query + /// shape, not just a subset. + fn align_end_timestamp_sql(&self, end_timestamp: u64) -> u64 { + let interval_ms = self.data_ingestion_interval_ms; + if end_timestamp.is_multiple_of(interval_ms) { + return end_timestamp; + } + let aligned = (end_timestamp / interval_ms) * interval_ms; + warn!( + "SQL query end timestamp {} is not aligned with data ingestion interval of {} ms; \ + aligning down to {}.", + end_timestamp, interval_ms, aligned + ); + aligned + } + /// Calculates start timestamp for SQL queries fn calculate_start_timestamp_sql( &self, @@ -264,10 +285,14 @@ impl SimpleEngine { /// Extract QueryRequirements from a parsed SQL match result. /// Used as the fallback path when no query_configs entry is found. + /// + /// `data_range_ms` is always the query's own requested duration: for a + /// single-scrape-interval query this equals `data_ingestion_interval_ms` + /// by construction (that's exactly the matcher's classification + /// boundary), so this is a plain identity, not a special case. fn build_query_requirements_sql( &self, match_result: &SQLQuery, - query_pattern_type: QueryPatternType, topk: Option, ) -> QueryRequirements { let query_data = match_result @@ -275,15 +300,7 @@ impl SimpleEngine { .expect("build_query_requirements_sql called on valid SQLQuery"); let metric = query_data.metric.clone(); - let statistic_name = match query_pattern_type { - QueryPatternType::OneTemporalOneSpatial => match_result - .inner_data() - .expect("OneTemporalOneSpatial pattern guarantees inner_data is present") - .aggregation_info - .get_name() - .to_lowercase(), - _ => query_data.aggregation_info.get_name().to_lowercase(), - }; + let statistic_name = query_data.aggregation_info.get_name().to_lowercase(); // For top-k the requirement is `Statistic::Topk` (→ CountMinSketchWithHeap) // and the grouping is empty: the GROUP BY column is the sketch's @@ -298,22 +315,7 @@ impl SimpleEngine { .collect() }; - let data_range_ms = match query_pattern_type { - QueryPatternType::OnlySpatial => self.data_ingestion_interval_ms, - QueryPatternType::OnlyTemporal => { - let duration_secs = query_data.time_info.clone().get_duration() as u64; - duration_secs * 1000 - } - QueryPatternType::OneTemporalOneSpatial => { - let duration_secs = match_result - .inner_data() - .expect("OneTemporalOneSpatial pattern guarantees inner_data is present") - .time_info - .clone() - .get_duration() as u64; - duration_secs * 1000 - } - }; + let data_range_ms = (query_data.time_info.clone().get_duration() * 1000.0).round() as u64; let grouping_labels = if is_topk { KeyByLabelNames::empty() @@ -592,8 +594,7 @@ impl SimpleEngine { .ok()? } else { warn!("No query_config entry for SQL query. Attempting capability-based matching."); - let requirements = - self.build_query_requirements_sql(&match_result, query_pattern_type, topk); + let requirements = self.build_query_requirements_sql(&match_result, topk); self.streaming_config .read() .unwrap() @@ -730,9 +731,8 @@ impl SimpleEngine { query_kwargs: query_kwargs.clone(), }; - // Calculate timestamps - similar to OnlyTemporal - let end_timestamp = - self.validate_and_align_end_timestamp(query_time, QueryPatternType::OnlyTemporal); + // Calculate timestamps + let end_timestamp = self.align_end_timestamp_sql(query_time); let duration_secs = match_result.outer_data()?.time_info.get_duration() as u64; let start_timestamp = end_timestamp - (duration_secs * 1000); @@ -755,11 +755,7 @@ impl SimpleEngine { warn!( "No query_config entry for SQL spatio-temporal query. Attempting capability-based matching." ); - let requirements = self.build_query_requirements_sql( - match_result, - QueryPatternType::OnlyTemporal, - topk, - ); + let requirements = self.build_query_requirements_sql(match_result, topk); self.streaming_config .read() .unwrap() @@ -1790,3 +1786,163 @@ mod topk_pipeline_tests { ); } } + +/// `build_spatiotemporal_context`'s end_timestamp snap (issue #500 / A1): +/// unlike the old hardcoded `QueryPatternType::OnlyTemporal` call into the +/// shared `validate_and_align_end_timestamp` (which only snaps for +/// `OnlySpatial`), SQL now always snaps a misaligned end_timestamp down to +/// the nearest data-ingestion-interval boundary, for every SQL query shape +/// including genuine multi-interval SpatioTemporal queries. +#[cfg(test)] +mod spatiotemporal_timestamp_alignment_tests { + use super::SimpleEngine; + use crate::data_model::{ + AggregationConfig, AggregationReference, AggregationType, CleanupPolicy, InferenceConfig, + QueryConfig, QueryLanguage, SchemaConfig, StreamingConfig, WindowType, + }; + use crate::stores::simple_map_store::SimpleMapStore; + use chrono::{Local, TimeZone}; + use promql_utilities::data_model::KeyByLabelNames; + use sql_utilities::sqlhelper::{SQLSchema, Table}; + use std::collections::{HashMap, HashSet}; + use std::sync::Arc; + + /// The SQL literal-date parser reads timestamps in the local timezone, so + /// the wall-clock second value that lands on (or off) a 300ms boundary + /// depends on the machine's TZ offset. Rather than hardcode an + /// assumed-UTC epoch, compute each candidate second's epoch-ms directly + /// via `chrono::Local`, so the test is correct under any CI timezone. + fn local_epoch_ms(second: u32) -> i64 { + Local + .with_ymd_and_hms(2025, 10, 1, 0, 0, second) + .single() + .expect("2025-10-01 00:00:xx is unambiguous in any timezone") + .timestamp() + * 1000 + } + + /// A SpatioTemporal SQL engine (GROUP BY a subset of labels, 2s window) + /// with a 300ms scrape interval — deliberately not a divisor of 1000ms, + /// so a whole-second literal timestamp is misaligned unless its seconds + /// value happens to be a multiple of 0.3s. + fn build_engine() -> SimpleEngine { + let labels: HashSet = ["L1", "L2"].iter().map(|s| s.to_string()).collect(); + let value_cols: HashSet = ["value"].iter().map(|s| s.to_string()).collect(); + let table = Table::new( + "cpu_usage".to_string(), + "time".to_string(), + value_cols, + labels, + ); + let sql_schema = SQLSchema::new(vec![table]); + + const AGG_ID: u64 = 1; + let template = "SELECT L1, SUM(value) FROM cpu_usage \ + WHERE time BETWEEN DATEADD(s, -2, NOW()) AND NOW() GROUP BY L1"; + let query_config = QueryConfig::new(template.to_string()) + .add_aggregation(AggregationReference::new(AGG_ID, None)); + + let inference_config = InferenceConfig { + schema: SchemaConfig::SQL(sql_schema), + query_configs: vec![query_config], + cleanup_policy: CleanupPolicy::NoCleanup, + }; + + let agg_config = AggregationConfig { + aggregation_id: AGG_ID, + aggregation_type: AggregationType::Sum, + aggregation_sub_type: String::new(), + parameters: HashMap::new(), + grouping_labels: KeyByLabelNames::new(vec!["L1".to_string()]), + aggregated_labels: KeyByLabelNames::empty(), + rollup_labels: KeyByLabelNames::empty(), + original_yaml: String::new(), + window_size_ms: 2000, + slide_interval_ms: 2000, + window_type: WindowType::Tumbling, + spatial_filter: String::new(), + spatial_filter_normalized: String::new(), + metric: "cpu_usage".to_string(), + num_aggregates_to_retain: None, + read_count_threshold: None, + table_name: None, + value_column: None, + }; + + let mut agg_configs = HashMap::new(); + agg_configs.insert(AGG_ID, agg_config); + let streaming_config = Arc::new(StreamingConfig { + aggregation_configs: agg_configs, + }); + let store = Arc::new(SimpleMapStore::new( + streaming_config.clone(), + CleanupPolicy::NoCleanup, + )); + + SimpleEngine::new( + store, + inference_config, + streaming_config, + 300, // scrape interval, ms — not a divisor of 1000 + QueryLanguage::sql, + ) + } + + #[test] + fn misaligned_end_timestamp_is_snapped_down() { + // Find a whole second in [0, 12) whose epoch-ms is NOT a multiple of + // 300ms (any TZ offset used by the local-date parser is itself a + // multiple of 300ms, so such a second exists in every timezone). + let (second, end_ms) = (0..12u32) + .map(|s| (s, local_epoch_ms(s))) + .find(|(_, ms)| ms % 300 != 0) + .expect("a misaligned second must exist in any timezone"); + let expected_end_ms = (end_ms / 300) * 300; + + let query = format!( + "SELECT L1, SUM(value) FROM cpu_usage \ + WHERE time BETWEEN DATEADD(s, -2, '2025-10-01 00:00:{second:02}') \ + AND '2025-10-01 00:00:{second:02}' GROUP BY L1" + ); + let context = build_engine() + .build_query_execution_context_sql(query, 0.0) + .expect("SpatioTemporal query should build a context"); + + let window = &context.store_plan.values_query; + assert_eq!( + window.end_timestamp, expected_end_ms as u64, + "misaligned end_timestamp must be snapped down to the nearest 300ms boundary" + ); + assert_eq!( + window.start_timestamp, + (expected_end_ms - 2000) as u64, + "start_timestamp must be the snapped end_timestamp minus the query's own 2s duration" + ); + } + + #[test] + fn already_aligned_end_timestamp_is_unchanged() { + // Find a whole second in [0, 12) whose epoch-ms already lands on a + // 300ms boundary. + let (second, end_ms) = (0..12u32) + .map(|s| (s, local_epoch_ms(s))) + .find(|(_, ms)| ms % 300 == 0) + .expect("an aligned second must exist in any timezone"); + + let query = format!( + "SELECT L1, SUM(value) FROM cpu_usage \ + WHERE time BETWEEN DATEADD(s, -2, '2025-10-01 00:00:{second:02}') \ + AND '2025-10-01 00:00:{second:02}' GROUP BY L1" + ); + let context = build_engine() + .build_query_execution_context_sql(query, 0.0) + .expect("SpatioTemporal query should build a context"); + + let window = &context.store_plan.values_query; + assert_eq!( + window.end_timestamp, end_ms as u64, + "an already-aligned end_timestamp must be left unchanged (snap is a no-op)" + ); + assert_eq!(window.start_timestamp, (end_ms - 2000) as u64); + } +} From b25690fd1a7c8f0caa66fde48aa6125223ab0500 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Fri, 3 Jul 2026 12:15:39 -0400 Subject: [PATCH 2/6] redirected all build context functions to SpatioTemporal --- .../src/engines/simple_engine/sql.rs | 283 +----------------- 1 file changed, 11 insertions(+), 272 deletions(-) diff --git a/asap-query-engine/src/engines/simple_engine/sql.rs b/asap-query-engine/src/engines/simple_engine/sql.rs index 25351d3..09bf0bf 100644 --- a/asap-query-engine/src/engines/simple_engine/sql.rs +++ b/asap-query-engine/src/engines/simple_engine/sql.rs @@ -9,12 +9,11 @@ use crate::engines::query_result::{InstantVector, InstantVectorElement, QueryRes use asap_types::query_requirements::QueryRequirements; use asap_types::utils::normalize_spatial_filter; use promql_utilities::data_model::KeyByLabelNames; -use promql_utilities::query_logics::enums::{QueryPatternType, Statistic}; -use sql_utilities::ast_matching::QueryType; +use promql_utilities::query_logics::enums::Statistic; use sql_utilities::ast_matching::{ detect_sql_topk, SQLPatternMatcher, SQLPatternParser, SQLQuery, SqlTopk, TopkWeighting, }; -use sql_utilities::sqlhelper::{AggregationInfo, OrderByItem, SQLQueryData}; +use sql_utilities::sqlhelper::{OrderByItem, SQLQueryData}; use sqlparser::dialect::*; use sqlparser::parser::Parser as parser; use std::collections::HashMap; @@ -192,54 +191,6 @@ impl SimpleEngine { aligned } - /// Calculates start timestamp for SQL queries - fn calculate_start_timestamp_sql( - &self, - end_timestamp: u64, - query_pattern_type: QueryPatternType, - match_result: &SQLQuery, - ) -> u64 { - match query_pattern_type { - QueryPatternType::OnlyTemporal => { - let duration_secs = match_result - .outer_data() - .expect("OnlyTemporal pattern guarantees outer_data is present") - .time_info - .clone() - .get_duration() as u64; - end_timestamp - (duration_secs * 1000) - } - QueryPatternType::OneTemporalOneSpatial => { - let duration_secs = match_result - .inner_data() - .expect("OneTemporalOneSpatial pattern guarantees inner_data is present") - .time_info - .clone() - .get_duration() as u64; - end_timestamp - (duration_secs * 1000) - } - QueryPatternType::OnlySpatial => end_timestamp - self.data_ingestion_interval_ms, - } - } - - /// Calculates and validates query timestamps for SQL - fn calculate_query_timestamps_sql( - &self, - query_time: u64, - query_pattern_type: QueryPatternType, - match_result: &SQLQuery, - ) -> QueryTimestamps { - let mut end_timestamp = query_time; - end_timestamp = self.validate_and_align_end_timestamp(end_timestamp, query_pattern_type); - let start_timestamp = - self.calculate_start_timestamp_sql(end_timestamp, query_pattern_type, match_result); - - QueryTimestamps { - start_timestamp, - end_timestamp, - } - } - /// Extracts quantile parameter from SQL match result fn extract_quantile_param_sql(&self, match_result: &SQLQuery) -> Option { match_result @@ -267,22 +218,6 @@ impl SimpleEngine { Ok(query_kwargs) } - fn sql_get_is_collapsable( - &self, - temporal_aggregation: &AggregationInfo, - spatial_aggregation: &AggregationInfo, - ) -> bool { - match spatial_aggregation.get_name() { - "SUM" => matches!( - temporal_aggregation.get_name(), - "SUM" | "COUNT" // Note: "increase" and "rate" are commented out in Python - ), - "MIN" => temporal_aggregation.get_name() == "MIN", - "MAX" => temporal_aggregation.get_name() == "MAX", - _ => false, - } - } - /// Extract QueryRequirements from a parsed SQL match result. /// Used as the fallback path when no query_configs entry is found. /// @@ -419,211 +354,15 @@ impl SimpleEngine { // every successful path below. let post = SqlPostProcessing::from_query_data(&query_data); - // Handle SpatioTemporal queries separately - they bypass QueryPatternType mapping - if match_result.query_type == vec![QueryType::SpatioTemporal] { - let query_time = Self::convert_query_time_to_data_time( - query_data.time_info.get_start() + query_data.time_info.get_duration(), - ); - let ctx = self.build_spatiotemporal_context(&match_result, query_time, &query_data)?; - return Some((ctx, post)); - } - - let query_pattern_type = match &match_result.query_type[..] { - [x] => match x { - QueryType::Spatial => QueryPatternType::OnlySpatial, - QueryType::TemporalGeneric => QueryPatternType::OnlyTemporal, - QueryType::TemporalQuantile => QueryPatternType::OnlyTemporal, - QueryType::SpatioTemporal => unreachable!("SpatioTemporal handled above"), - }, - [x, y] => match (x, y) { - (QueryType::Spatial, QueryType::TemporalGeneric) => { - QueryPatternType::OneTemporalOneSpatial - } - (QueryType::Spatial, QueryType::TemporalQuantile) => { - QueryPatternType::OneTemporalOneSpatial - } - _ => return None, - }, - _ => return None, - }; - - // For nested queries (spatial of temporal), the outer query has no time clause, - // so we need to use the inner (temporal) query's time_info to compute query_time - let query_time = match query_pattern_type { - QueryPatternType::OneTemporalOneSpatial => { - let inner_time_info = &match_result.inner_data()?.time_info; - Self::convert_query_time_to_data_time( - inner_time_info.get_start() + inner_time_info.get_duration(), - ) - } - _ => Self::convert_query_time_to_data_time( - query_data.time_info.get_start() + query_data.time_info.get_duration(), - ), - }; - - // self.handle_sql_temporal_aggregation( - // query_config, - // &match_result, - // query_time, - // query_pattern_type, - // ) - // } - - // fn handle_sql_temporal_aggregation( - // &self, - // query_config: &QueryConfig, - // match_result: &SQLQuery, - // query_time: u64, - // query_pattern_type: QueryPatternType, - // ) -> Option<(KeyByLabelNames, QueryResult)> { - // Labels - - let query_output_labels = match &match_result.query_type.len() { - // Potentially change SQLQueryType - 1 => { - // For non-nested queries, output associated labels - let labels = &match_result.outer_data()?.labels; - - KeyByLabelNames::new(labels.clone().into_iter().collect()) - } - 2 => { - // Extract spatial aggregation output labels using AST-based approach - let temporal_labels = &match_result.inner_data()?.labels; - let spatial_labels = &match_result.outer_data()?.labels; - - let temporal_aggregation = &match_result.inner_data()?.aggregation_info; - let spatial_aggregation = &match_result.outer_data()?.aggregation_info; - - match self.sql_get_is_collapsable(temporal_aggregation, spatial_aggregation) { - // If false: get all labels, which are all temporal labels. If true, get only spatial labels - false => KeyByLabelNames::new(temporal_labels.clone().into_iter().collect()), - true => KeyByLabelNames::new(spatial_labels.clone().into_iter().collect()), - } - } - _ => { - warn!("Invalid query type: {}", query_pattern_type); - KeyByLabelNames::new(Vec::new()) - } - }; - - // Statistic - determine based on query pattern type - let statistic_name = match query_pattern_type { - QueryPatternType::OnlyTemporal => { - // Use the temporal aggregation (first subquery) - match_result - .outer_data()? - .aggregation_info - .get_name() - .to_lowercase() - } - QueryPatternType::OneTemporalOneSpatial => { - // Use the temporal aggregation (second subquery contains temporal) - match_result - .inner_data()? - .aggregation_info - .get_name() - .to_lowercase() - } - QueryPatternType::OnlySpatial => { - // Use the spatial aggregation (first subquery) - match_result - .outer_data()? - .aggregation_info - .get_name() - .to_lowercase() - } - }; - - // Top-k (CountMinSketchWithHeap) applies to flat single-layer queries: - // COUNT/SUM ... GROUP BY ORDER BY DESC LIMIT k. - // Nested patterns attach ORDER BY / LIMIT to the outer SELECT; `query_data` - // from parse is the outer layer, while the temporal aggregate lives in - // `inner_data` for OneTemporalOneSpatial. Running detect_sql_topk on the - // outer layer would mis-classify spatial rollups as top-k. - // - // Single-interval windows (duration == scrape interval) classify as - // `OnlySpatial` in the pattern matcher even though they are flat temporal - // reads, so both `OnlyTemporal` and `OnlySpatial` must run detection. - let topk = match query_pattern_type { - QueryPatternType::OnlyTemporal | QueryPatternType::OnlySpatial => { - detect_sql_topk(&query_data) - } - QueryPatternType::OneTemporalOneSpatial => None, - }; - if topk.is_some_and(|t| t.weighting == TopkWeighting::Sum) { - warn!( - "SUM top-k assumes non-negative values; results are undefined for columns with negative entries" - ); - } - let statistic_to_compute = if topk.is_some() { - Statistic::Topk - } else { - Self::parse_single_statistic(&statistic_name)? - }; - - let mut query_kwargs = self - .build_query_kwargs_sql(&statistic_to_compute, &match_result) - .map_err(|e| { - warn!("{}", e); - e - }) - .ok()?; - if let Some(topk) = topk { - query_kwargs.insert("k".to_string(), topk.k.to_string()); - } - - // Create query metadata - let metadata = QueryMetadata { - query_output_labels: query_output_labels.clone(), - statistic_to_compute, - query_kwargs: query_kwargs.clone(), - }; - - // Time - let timestamps = - self.calculate_query_timestamps_sql(query_time, query_pattern_type, &match_result); - - // Resolve aggregation: try pre-configured query_configs first, fall back to capability matching. - let agg_info: AggregationIdInfo = - if let Some(config) = self.find_query_config_sql(&query_data) { - self.get_aggregation_id_info(&config) - .map_err(|e| { - warn!("{}", e); - e - }) - .ok()? - } else { - warn!("No query_config entry for SQL query. Attempting capability-based matching."); - let requirements = self.build_query_requirements_sql(&match_result, topk); - self.streaming_config - .read() - .unwrap() - .clone() - .find_compatible_aggregation(&requirements)? - }; - - let metric = &match_result.outer_data()?.metric; - - let spatial_filter = if query_pattern_type == QueryPatternType::OneTemporalOneSpatial { - match_result - .outer_data()? - .labels - .iter() - .cloned() - .collect::>() - .join(",") - } else { - String::new() - }; - - let ctx = self.build_sql_execution_context_tail( - metric, - ×tamps, - metadata, - agg_info, - spatial_filter, - query_time, - )?; + // Every valid (non-nested) SQL query is handled uniformly by + // `build_spatiotemporal_context`, regardless of duration or GROUP BY + // shape. An unmatched query (e.g. no time column) leaves `match_result` + // empty, so `outer_data()` inside `build_spatiotemporal_context` + // returns `None` and this propagates via `?` — no separate check needed. + let query_time = Self::convert_query_time_to_data_time( + query_data.time_info.get_start() + query_data.time_info.get_duration(), + ); + let ctx = self.build_spatiotemporal_context(&match_result, query_time, &query_data)?; Some((ctx, post)) } From 0ad3034783721ba1b4bfa8197a08abf973d65f89 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Fri, 3 Jul 2026 15:14:08 -0400 Subject: [PATCH 3/6] Updated window logic --- asap-planner-rs/src/planner/sql.rs | 60 ++++++++++++++++-------- asap-planner-rs/tests/sql_integration.rs | 41 ++++------------ 2 files changed, 50 insertions(+), 51 deletions(-) diff --git a/asap-planner-rs/src/planner/sql.rs b/asap-planner-rs/src/planner/sql.rs index 950a09d..a1106ca 100644 --- a/asap-planner-rs/src/planner/sql.rs +++ b/asap-planner-rs/src/planner/sql.rs @@ -3,8 +3,8 @@ use std::collections::HashSet; use asap_types::enums::{CleanupPolicy, WindowType}; use promql_utilities::data_model::KeyByLabelNames; use promql_utilities::query_logics::enums::{AggregationType, QueryTreatmentType, Statistic}; -use sql_utilities::ast_matching::sqlhelper::{detect_sql_topk, Table}; -use sql_utilities::ast_matching::sqlpattern_matcher::{QueryType, SQLPatternMatcher}; +use sql_utilities::ast_matching::sqlhelper::{detect_sql_topk, Table, TimeInfo}; +use sql_utilities::ast_matching::sqlpattern_matcher::SQLPatternMatcher; use sql_utilities::ast_matching::sqlpattern_parser::SQLPatternParser; use sql_utilities::ast_matching::SQLSchema; use sqlparser::dialect::ClickHouseDialect; @@ -94,7 +94,6 @@ impl SQLSingleQueryProcessor { // Determine fields from query vecs let agg_info = &sql_query.query_data[0].aggregation_info; - let query_type = &sql_query.query_type[0]; let labels = &sql_query.query_data[0].labels; let table_name = &sql_query.query_data[0].metric; @@ -102,10 +101,10 @@ impl SQLSingleQueryProcessor { // Compute window let window_cfg = compute_sql_window( - query_type, + &sql_query.query_data[0].time_info, self.data_ingestion_interval_ms, self.t_repeat_ms, - ); + )?; // Get all metadata columns for the table let all_metadata = get_all_metadata_columns(&self.table_definitions, table_name)?; @@ -164,11 +163,12 @@ impl SQLSingleQueryProcessor { } } - let t_lookback_ms = match query_type { - QueryType::Spatial => self.data_ingestion_interval_ms, - // SQLPatternParser always produces second-based durations; convert to ms. - _ => (sql_query.query_data[0].time_info.get_duration() * 1000.0).round() as u64, - }; + // SQLPatternParser always produces second-based durations; convert to ms. + // For a single-scrape-interval query this equals data_ingestion_interval_ms + // by construction (the matcher's classification boundary), so this is a + // plain unconditional formula, not a special case. + let t_lookback_ms = + (sql_query.query_data[0].time_info.get_duration() * 1000.0).round() as u64; let cleanup_param = if self.cleanup_policy == CleanupPolicy::NoCleanup { None @@ -221,20 +221,40 @@ fn get_sql_statistics(name: &str) -> Result, ControllerError> { } } +/// Behavior change from #500: enforces two invariants — +/// `t_repeat_ms >= data_ingestion_interval_ms` (can't refresh faster than +/// raw ingestion) and `duration_ms >= t_repeat_ms` (a precompute window must +/// not outlive the query range it's sized for) — rather than silently +/// picking `data_ingestion_interval_ms` or `t_repeat_ms` depending on query +/// shape. Once both hold, `window_size_ms` is simply `t_repeat_ms`: at the +/// single-scrape-interval boundary `duration_ms == data_ingestion_interval_ms` +/// exactly, so both invariants together force `t_repeat_ms == data_ingestion_interval_ms` +/// there, reproducing the old `Spatial` branch without a special case. +/// +/// Old code used `t_repeat_ms` uncapped even when it exceeded the query's +/// own duration (see `temporal_sum_t600` in `sql_integration.rs`, updated +/// alongside this change to expect a `PlannerError` instead). fn compute_sql_window( - query_type: &QueryType, + time_info: &TimeInfo, data_ingestion_interval_ms: u64, t_repeat_ms: u64, -) -> IntermediateWindowConfig { - let window_size_ms = match query_type { - QueryType::Spatial => data_ingestion_interval_ms, - _ => t_repeat_ms, - }; - IntermediateWindowConfig { - window_size_ms, - slide_interval_ms: window_size_ms, - window_type: WindowType::Tumbling, +) -> Result { + if t_repeat_ms < data_ingestion_interval_ms { + return Err(ControllerError::PlannerError(format!( + "t_repeat_ms ({t_repeat_ms}ms) must be >= data_ingestion_interval_ms ({data_ingestion_interval_ms}ms)" + ))); } + let duration_ms = (time_info.get_duration() * 1000.0).round() as u64; + if duration_ms < t_repeat_ms { + return Err(ControllerError::PlannerError(format!( + "query duration ({duration_ms}ms) must be >= t_repeat_ms ({t_repeat_ms}ms)" + ))); + } + Ok(IntermediateWindowConfig { + window_size_ms: t_repeat_ms, + slide_interval_ms: t_repeat_ms, + window_type: WindowType::Tumbling, + }) } fn get_all_metadata_columns( diff --git a/asap-planner-rs/tests/sql_integration.rs b/asap-planner-rs/tests/sql_integration.rs index 465f144..0dcb3e4 100644 --- a/asap-planner-rs/tests/sql_integration.rs +++ b/asap-planner-rs/tests/sql_integration.rs @@ -578,41 +578,20 @@ fn temporal_sum_t30() { } /// T = 300 s == range: covered by temporal_sum above (cleanup = 1). -/// T = 600 s > range: window = 600, cleanup = ceil(300 / 600) = 1. -/// T larger than the query range is valid; the result is still 1 retained window. +/// +/// T = 600 s > range (300s): as of #500, a precompute window is not allowed +/// to outlive the query range it's sized for — `duration_ms >= t_repeat_ms` +/// is now an enforced invariant, so this returns `PlannerError` instead of +/// silently building a 600s window (the old behavior, when this test +/// asserted `window == 600` and `cleanup == ceil(300 / 600) == 1`). #[test] -fn temporal_sum_t600() { +fn temporal_sum_t600_exceeds_duration_is_planner_error() { let q = "SELECT SUM(cpu_usage) FROM metrics_table WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() GROUP BY datacenter"; - let out = SQLController::from_yaml(&one_query_config(q, 600_000), sql_opts()) + let result = SQLController::from_yaml(&one_query_config(q, 600_000), sql_opts()) .unwrap() - .generate() - .unwrap(); + .generate(); - assert_eq!(out.streaming_aggregation_count(), 2); - assert_eq!(out.inference_query_count(), 1); - assert!(out.has_aggregation_type("CountMinSketch")); - assert!(out.has_aggregation_type("DeltaSetAggregator")); - assert!(out.all_tumbling_window_sizes_eq(600_000)); - assert_eq!( - out.aggregation_table_name("CountMinSketch"), - Some("metrics_table".to_string()) - ); - assert_eq!( - out.aggregation_value_column("CountMinSketch"), - Some("cpu_usage".to_string()) - ); - let mut rollup = out.aggregation_labels("CountMinSketch", "rollup"); - rollup.sort(); - assert_eq!(rollup, vec!["hostname".to_string(), "region".to_string()]); - assert_eq!( - out.aggregation_labels("CountMinSketch", "grouping"), - Vec::::new() - ); - assert_eq!( - out.aggregation_labels("CountMinSketch", "aggregated"), - vec!["datacenter".to_string()] - ); - assert_eq!(out.inference_cleanup_param(q), Some(1)); + assert!(matches!(result, Err(ControllerError::PlannerError(_)))); } // ── multi-query tests ───────────────────────────────────────────────────────── From 54e86b1377b8e43cc49a1ed9d0e9076ac1475a83 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Fri, 3 Jul 2026 15:58:36 -0400 Subject: [PATCH 4/6] refactor: classify every query as SpatioTemporal --- .../src/ast_matching/sqlparser_test.rs | 54 +++++++-------- .../src/ast_matching/sqlpattern_matcher.rs | 69 +++++++------------ 2 files changed, 52 insertions(+), 71 deletions(-) diff --git a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs index 47996f6..a795122 100644 --- a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs +++ b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs @@ -99,7 +99,7 @@ mod tests { if let Some(query_data) = SQLPatternParser::new(&schema, time).parse_query(&statements) { let result = matcher.query_info_to_pattern(&query_data); assert!(result.is_valid()); - assert_eq!(result.query_type, vec![QueryType::Spatial]); + assert_eq!(result.query_type, vec![QueryType::SpatioTemporal]); } } @@ -109,7 +109,7 @@ mod tests { fn test_dated_temporal_sum() { check_query( "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, '2025-10-01 00:00:00') AND '2025-10-01 00:00:00' GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalGeneric], + vec![QueryType::SpatioTemporal], None, ); } @@ -118,7 +118,7 @@ mod tests { fn test_dated_temporal_quantile() { check_query( "SELECT QUANTILE(0.95, value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, '2025-10-01 00:00:00') AND '2025-10-01 00:00:00' GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalQuantile], + vec![QueryType::SpatioTemporal], None, ); } @@ -127,7 +127,7 @@ mod tests { fn test_dated_spatial_avg() { check_query( "SELECT AVG(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, '2025-10-01 00:00:00') AND '2025-10-01 00:00:00' GROUP BY L1, L2, L3, L4", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -136,7 +136,7 @@ mod tests { fn test_dated_spatial_quantile() { check_query( "SELECT QUANTILE(0.95, value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, '2025-10-01 00:00:00') AND '2025-10-01 00:00:00' GROUP BY L1", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -156,7 +156,7 @@ mod tests { fn test_temporal_quantile() { check_query( "SELECT QUANTILE(0.95, value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalQuantile], + vec![QueryType::SpatioTemporal], None, ); } @@ -165,7 +165,7 @@ mod tests { fn test_temporal_sum() { check_query( "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalGeneric], + vec![QueryType::SpatioTemporal], None, ); } @@ -174,7 +174,7 @@ mod tests { fn test_temporal_max() { check_query( "SELECT MAX(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalGeneric], + vec![QueryType::SpatioTemporal], None, ); } @@ -183,7 +183,7 @@ mod tests { fn test_temporal_min() { check_query( "SELECT MIN(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalGeneric], + vec![QueryType::SpatioTemporal], None, ); } @@ -192,7 +192,7 @@ mod tests { fn test_temporal_avg() { check_query( "SELECT AVG(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalGeneric], + vec![QueryType::SpatioTemporal], None, ); } @@ -203,7 +203,7 @@ mod tests { fn test_spatial_sum() { check_query( "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY L1", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -212,7 +212,7 @@ mod tests { fn test_spatial_max() { check_query( "SELECT MAX(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY L1, L2", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -221,7 +221,7 @@ mod tests { fn test_spatial_min() { check_query( "SELECT MIN(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY L1, L2, L3", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -230,7 +230,7 @@ mod tests { fn test_spatial_avg() { check_query( "SELECT AVG(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY L1, L2, L3, L4", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -239,7 +239,7 @@ mod tests { fn test_spatial_quantile() { check_query( "SELECT QUANTILE(0.95, value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY L1", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -425,7 +425,7 @@ mod tests { fn test_temporal_percentile() { check_query( "SELECT PERCENTILE(value, 95) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalQuantile], + vec![QueryType::SpatioTemporal], None, ); } @@ -434,7 +434,7 @@ mod tests { fn test_spatial_percentile() { check_query( "SELECT PERCENTILE(value, 95) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY L1", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -464,7 +464,7 @@ mod tests { fn test_clickhouse_temporal_quantile() { check_query( "SELECT quantile(0.95)(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalQuantile], + vec![QueryType::SpatioTemporal], None, ); } @@ -473,7 +473,7 @@ mod tests { fn test_clickhouse_spatial_quantile() { check_query( "SELECT quantile(0.95)(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY L1", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -504,7 +504,7 @@ mod tests { fn test_clickhouse_explicit_datetime_temporal_quantile() { check_query( "SELECT quantile(0.95)(value) FROM cpu_usage WHERE time BETWEEN '2025-10-01 00:00:00' AND '2025-10-01 00:00:10' GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalQuantile], + vec![QueryType::SpatioTemporal], None, ); } @@ -516,7 +516,7 @@ mod tests { fn test_asap_only_iso_z_temporal_quantile() { check_query( "SELECT quantile(0.95)(value) FROM cpu_usage WHERE time BETWEEN '2025-10-01T00:00:00Z' AND '2025-10-01T00:00:10Z' GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalQuantile], + vec![QueryType::SpatioTemporal], None, ); } @@ -528,7 +528,7 @@ mod tests { fn test_iso_no_z_treated_as_local_time_temporal_quantile() { check_query( "SELECT quantile(0.95)(value) FROM cpu_usage WHERE time BETWEEN '2025-10-01T00:00:00' AND '2025-10-01T00:00:10' GROUP BY L1, L2, L3, L4", - vec![QueryType::TemporalQuantile], + vec![QueryType::SpatioTemporal], None, ); } @@ -537,7 +537,7 @@ mod tests { fn test_clickhouse_explicit_datetime_spatial_quantile() { check_query( "SELECT quantile(0.95)(value) FROM cpu_usage WHERE time BETWEEN '2025-10-01 00:00:00' AND '2025-10-01 00:00:01' GROUP BY L1", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -738,7 +738,7 @@ mod tests { WHERE time BETWEEN DATEADD(s, -15, '2025-10-01 00:00:00') AND '2025-10-01 00:00:00' \ GROUP BY L1, L2, L3, L4", 15.0, - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } @@ -751,7 +751,7 @@ mod tests { WHERE time BETWEEN DATEADD(s, -30, '2025-10-01 00:00:00') AND '2025-10-01 00:00:00' \ GROUP BY L1, L2, L3, L4", 15.0, - vec![QueryType::TemporalGeneric], + vec![QueryType::SpatioTemporal], None, ); } @@ -764,7 +764,7 @@ mod tests { WHERE time BETWEEN DATEADD(s, -30, '2025-10-01 00:00:00') AND '2025-10-01 00:00:00' \ GROUP BY L1, L2, L3, L4", 15.0, - vec![QueryType::TemporalQuantile], + vec![QueryType::SpatioTemporal], None, ); } @@ -1305,7 +1305,7 @@ mod tests { "SELECT SUM(value) FROM cpu_usage \ WHERE time >= DATEADD(s, -1, NOW()) AND time < NOW() \ GROUP BY L1", - vec![QueryType::Spatial], + vec![QueryType::SpatioTemporal], None, ); } diff --git a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs index 073e79b..9d7f95c 100644 --- a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs +++ b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs @@ -22,6 +22,7 @@ pub enum QueryError { IllegalAggregationFn, SpatialDurationSmall, NestedQueryUnsupported, + MissingTimeRange, } #[derive(Debug)] @@ -262,52 +263,32 @@ impl SQLPatternMatcher { if let Some((metric, aggregation_info, scrape_duration, labels, time_info)) = query_data.first() { - if (scrape_duration - 1.0).abs() < f64::EPSILON { - sql_query.add_subquery( - QueryType::Spatial, - aggregation_info.clone(), - metric.clone(), - labels.clone(), - time_info.clone(), + // scrape_duration is 0.0 only for the UNUSED-time-column marker + // (the outer layer of a nested query, which the len() > 1 check + // above already rejects) — not reachable for a flat query, since + // any real time range short enough to compute a 0.0 (or < 1 + // scrape interval) duration already errors out above via + // SpatialDurationSmall. Kept as an explicit error rather than + // silently leaving the query unmatched, in case that invariant + // ever changes. + if *scrape_duration == 0.0 { + println!("Returned QueryError::MissingTimeRange"); + + return SQLQuery::new( + Vec::new(), + Some(QueryError::MissingTimeRange), + Some("query has no resolvable time range".to_string()), ); - } else if *scrape_duration > 1.0 { - // Check if labels match all metadata columns - let has_all_labels = self - .schema - .get_metadata_columns(metric) - .map(|schema_metadata_columns| labels == schema_metadata_columns) - .unwrap_or(true); - - if has_all_labels { - // Full temporal query with all labels (PromQL-equivalent) - if aggregation_info.get_name() == "QUANTILE" { - sql_query.add_subquery( - QueryType::TemporalQuantile, - aggregation_info.clone(), - metric.clone(), - labels.clone(), - time_info.clone(), - ); - } else { - sql_query.add_subquery( - QueryType::TemporalGeneric, - aggregation_info.clone(), - metric.clone(), - labels.clone(), - time_info.clone(), - ); - } - } else { - // SpatioTemporal: spans multiple scrape intervals but groups by subset of labels - sql_query.add_subquery( - QueryType::SpatioTemporal, - aggregation_info.clone(), - metric.clone(), - labels.clone(), - time_info.clone(), - ); - } } + + // Every valid query classifies as SpatioTemporal + sql_query.add_subquery( + QueryType::SpatioTemporal, + aggregation_info.clone(), + metric.clone(), + labels.clone(), + time_info.clone(), + ); } sql_query From 9a39c69f29ecec73408496747b9a9c83508badbe Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Fri, 3 Jul 2026 17:35:39 -0400 Subject: [PATCH 5/6] Removed all query types apart from SpatioTemporal --- .../rs/sql_utilities/src/ast_matching/sqlparser_test.rs | 2 +- .../sql_utilities/src/ast_matching/sqlpattern_matcher.rs | 7 ++++--- asap-planner-rs/src/planner/sql.rs | 2 +- asap-planner-rs/tests/sql_integration.rs | 2 +- asap-query-engine/src/engines/simple_engine/sql.rs | 2 +- asap-query-engine/src/tests/sql_pattern_matching_tests.rs | 6 +++--- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs index a795122..d5925e8 100644 --- a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs +++ b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs @@ -756,7 +756,7 @@ mod tests { ); } - /// scraped_intervals = 30/15 = 2.0 with QUANTILE agg → TemporalQuantile + /// scraped_intervals = 30/15 = 2.0 with QUANTILE agg → SpatioTemporal #[test] fn test_bug_201_temporal_quantile_not_rejected() { check_query_with_interval( diff --git a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs index 9d7f95c..d24a22b 100644 --- a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs +++ b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs @@ -5,11 +5,12 @@ use crate::sqlhelper::TimeInfo; use std::collections::HashSet; +/// Every valid SQL query classifies as SpatioTemporal — query-type-agnostic, +/// so this is a single-variant enum. Kept (rather than removed outright) so +/// `SQLQuery.query_type: Vec` doesn't need reshaping here — see +/// #512 for that follow-up. #[derive(Debug, Clone, PartialEq)] pub enum QueryType { - Spatial, - TemporalGeneric, - TemporalQuantile, SpatioTemporal, } diff --git a/asap-planner-rs/src/planner/sql.rs b/asap-planner-rs/src/planner/sql.rs index a1106ca..5aef496 100644 --- a/asap-planner-rs/src/planner/sql.rs +++ b/asap-planner-rs/src/planner/sql.rs @@ -221,7 +221,7 @@ fn get_sql_statistics(name: &str) -> Result, ControllerError> { } } -/// Behavior change from #500: enforces two invariants — +/// Enforces two invariants — /// `t_repeat_ms >= data_ingestion_interval_ms` (can't refresh faster than /// raw ingestion) and `duration_ms >= t_repeat_ms` (a precompute window must /// not outlive the query range it's sized for) — rather than silently diff --git a/asap-planner-rs/tests/sql_integration.rs b/asap-planner-rs/tests/sql_integration.rs index 0dcb3e4..28a6107 100644 --- a/asap-planner-rs/tests/sql_integration.rs +++ b/asap-planner-rs/tests/sql_integration.rs @@ -579,7 +579,7 @@ fn temporal_sum_t30() { /// T = 300 s == range: covered by temporal_sum above (cleanup = 1). /// -/// T = 600 s > range (300s): as of #500, a precompute window is not allowed +/// T = 600 s > range (300s): a precompute window is not allowed /// to outlive the query range it's sized for — `duration_ms >= t_repeat_ms` /// is now an enforced invariant, so this returns `PlannerError` instead of /// silently building a 600s window (the old behavior, when this test diff --git a/asap-query-engine/src/engines/simple_engine/sql.rs b/asap-query-engine/src/engines/simple_engine/sql.rs index 09bf0bf..de7fd7d 100644 --- a/asap-query-engine/src/engines/simple_engine/sql.rs +++ b/asap-query-engine/src/engines/simple_engine/sql.rs @@ -1526,7 +1526,7 @@ mod topk_pipeline_tests { } } -/// `build_spatiotemporal_context`'s end_timestamp snap (issue #500 / A1): +/// `build_spatiotemporal_context`'s end_timestamp snap: /// unlike the old hardcoded `QueryPatternType::OnlyTemporal` call into the /// shared `validate_and_align_end_timestamp` (which only snaps for /// `OnlySpatial`), SQL now always snaps a misaligned end_timestamp down to diff --git a/asap-query-engine/src/tests/sql_pattern_matching_tests.rs b/asap-query-engine/src/tests/sql_pattern_matching_tests.rs index e249246..66a867f 100644 --- a/asap-query-engine/src/tests/sql_pattern_matching_tests.rs +++ b/asap-query-engine/src/tests/sql_pattern_matching_tests.rs @@ -129,9 +129,9 @@ mod tests { #[test] fn test_spatial_query_matches_now_template() { - // Spatial: window equals the scrape interval (1s), GROUP BY all labels + // Window equals the scrape interval (1s), GROUP BY all labels — classifies + // the same as every other shape: SpatioTemporal. let template = "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY L1, L2, L3, L4"; - // scrape_interval=1, window=1 → classified as Spatial by the matcher let engine = build_sql_engine(template, 1, 1000); let incoming = "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, '2025-10-01 00:00:10') AND '2025-10-01 00:00:10' GROUP BY L1, L2, L3, L4"; @@ -146,7 +146,7 @@ mod tests { #[test] fn test_temporal_quantile_query_matches_now_template() { - // TemporalQuantile: QUANTILE aggregation, window > scrape interval, GROUP BY all labels + // QUANTILE aggregation, window > scrape interval, GROUP BY all labels let template = "SELECT QUANTILE(0.95, value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4"; let engine = build_sql_engine(template, 1, 10_000); From 762d37d34a4e6d7fc38849eefb6f920545a0fe18 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Fri, 3 Jul 2026 17:45:33 -0400 Subject: [PATCH 6/6] small fix --- asap-query-engine/src/engines/simple_engine/sql.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/asap-query-engine/src/engines/simple_engine/sql.rs b/asap-query-engine/src/engines/simple_engine/sql.rs index de7fd7d..daf7b31 100644 --- a/asap-query-engine/src/engines/simple_engine/sql.rs +++ b/asap-query-engine/src/engines/simple_engine/sql.rs @@ -702,7 +702,8 @@ mod detect_topk_tests { let qd = parse(&sql).expect("nested query should parse"); assert!( detect_sql_topk(&qd).is_some(), - "outer SELECT alone matches the top-k shape (this is why OneTemporalOneSpatial is gated)", + "outer SELECT alone matches the top-k shape — this is why nested queries must be \ + rejected (NestedQueryUnsupported) before topk detection ever runs on them", ); } }