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 13d1c3bc..47996f68 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 @@ -145,8 +145,8 @@ mod tests { fn test_dated_spatial_of_temporal_quantile_max() { check_query( "SELECT QUANTILE(0.95, value) FROM (SELECT MAX(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) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -250,8 +250,8 @@ mod tests { fn test_spatial_of_temporal_sum_sum() { check_query( "SELECT SUM(result) FROM (SELECT SUM(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -259,8 +259,8 @@ mod tests { fn test_spatial_of_temporal_sum_min() { check_query( "SELECT SUM(result) FROM (SELECT MIN(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1, L2", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -268,8 +268,8 @@ mod tests { fn test_spatial_of_temporal_sum_max() { check_query( "SELECT SUM(result) FROM (SELECT MAX(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1, L2, L3", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -277,8 +277,8 @@ mod tests { fn test_spatial_of_temporal_sum_avg() { check_query( "SELECT SUM(result) FROM (SELECT AVG(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1, L2, L3, L4", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -286,8 +286,8 @@ mod tests { fn test_spatial_of_temporal_max_sum() { check_query( "SELECT MAX(result) FROM (SELECT SUM(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1, L2", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -295,8 +295,8 @@ mod tests { fn test_spatial_of_temporal_max_min() { check_query( "SELECT MAX(result) FROM (SELECT MIN(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -304,8 +304,8 @@ mod tests { fn test_spatial_of_temporal_max_max() { check_query( "SELECT MAX(result) FROM (SELECT MAX(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1, L2, L3", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -313,8 +313,8 @@ mod tests { fn test_spatial_of_temporal_max_avg() { check_query( "SELECT MAX(result) FROM (SELECT AVG(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1, L2, L3, L4", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -322,8 +322,8 @@ mod tests { fn test_spatial_of_temporal_quantile_max() { check_query( "SELECT QUANTILE(0.95, value) FROM (SELECT MAX(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -331,8 +331,8 @@ mod tests { fn test_spatial_of_temporal_quantile_min() { check_query( "SELECT QUANTILE(0.95, value) FROM (SELECT MIN(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -340,8 +340,8 @@ mod tests { fn test_spatial_of_temporal_quantile_sum() { check_query( "SELECT QUANTILE(0.95, value) FROM (SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -349,8 +349,8 @@ mod tests { fn test_spatial_of_temporal_quantile_avg() { check_query( "SELECT QUANTILE(0.95, value) FROM (SELECT AVG(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -358,8 +358,8 @@ mod tests { fn test_spatial_of_temporal_avg_quantile() { check_query( "SELECT AVG(result) FROM (SELECT QUANTILE(0.95, value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1, L2", - vec![QueryType::Spatial, QueryType::TemporalQuantile], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -367,8 +367,8 @@ mod tests { fn test_spatial_of_temporal_quantile_quantile() { check_query( "SELECT QUANTILE(0.95, value) FROM (SELECT QUANTILE(0.95, value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1, L2, L3", - vec![QueryType::Spatial, QueryType::TemporalQuantile], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -452,8 +452,8 @@ mod tests { fn test_spatial_of_temporal_percentile_max() { check_query( "SELECT PERCENTILE(value, 95) FROM (SELECT MAX(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -491,8 +491,8 @@ mod tests { fn test_clickhouse_spatial_of_temporal_quantile_max() { check_query( "SELECT quantile(0.95)(value) FROM (SELECT MAX(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1", - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } @@ -782,9 +782,10 @@ mod tests { ); } - /// Spatial-of-temporal: outer has UNUSED time (not checked), inner scraped_intervals = 30/15 = 2.0 + /// Nested query with a fine inner duration (30/15 = 2.0, so the bug-201 guard + /// wouldn't false-positive) still gets rejected as unsupported, not matched. #[test] - fn test_bug_201_spatial_of_temporal_not_rejected() { + fn test_bug_201_spatial_of_temporal_rejected_as_nested() { check_query_with_interval( "SELECT SUM(result) FROM \ (SELECT SUM(value) AS result FROM cpu_usage \ @@ -792,8 +793,8 @@ mod tests { GROUP BY L1, L2, L3, L4) \ GROUP BY L1", 15.0, - vec![QueryType::Spatial, QueryType::TemporalGeneric], - None, + vec![], + Some(QueryError::NestedQueryUnsupported), ); } 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 d7f8f340..073e79ba 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 @@ -21,6 +21,7 @@ pub enum QueryError { TemporalMissingLabels, // indistinguishable from too large scrape duration IllegalAggregationFn, SpatialDurationSmall, + NestedQueryUnsupported, } #[derive(Debug)] @@ -243,14 +244,25 @@ impl SQLPatternMatcher { }; println!("flattened QueryData: {query_data:?}"); + if query_data.len() > 1 { + println!("Returned QueryError::NestedQueryUnsupported"); + + return SQLQuery::new( + Vec::new(), + Some(QueryError::NestedQueryUnsupported), + Some(format!( + "nested SQL queries are not supported (n={})", + query_data.len() + )), + ); + } + let mut sql_query = SQLQuery::new(Vec::new(), None, None); - for (i, (metric, aggregation_info, scrape_duration, labels, time_info)) in - query_data.iter().enumerate() + if let Some((metric, aggregation_info, scrape_duration, labels, time_info)) = + query_data.first() { - if i < query_data.len() - 1 { - // Not the last query - // let time_info = TimeInfo::new("time".to_string(), *start, *scrape_duration); // You may need to adjust this + if (scrape_duration - 1.0).abs() < f64::EPSILON { sql_query.add_subquery( QueryType::Spatial, aggregation_info.clone(), @@ -258,55 +270,42 @@ impl SQLPatternMatcher { labels.clone(), time_info.clone(), ); - } else { - // Last query - // let time_info = TimeInfo::new("time".to_string(), *start, *scrape_duration); - - if (scrape_duration - 1.0).abs() < f64::EPSILON { - sql_query.add_subquery( - QueryType::Spatial, - aggregation_info.clone(), - metric.clone(), - labels.clone(), - time_info.clone(), - ); - } 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 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 { - // SpatioTemporal: spans multiple scrape intervals but groups by subset of labels sql_query.add_subquery( - QueryType::SpatioTemporal, + 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(), + ); } } } diff --git a/asap-query-engine/src/tests/query_equivalence_tests.rs b/asap-query-engine/src/tests/query_equivalence_tests.rs index bd9cbec2..add45048 100644 --- a/asap-query-engine/src/tests/query_equivalence_tests.rs +++ b/asap-query-engine/src/tests/query_equivalence_tests.rs @@ -317,59 +317,41 @@ mod tests { } #[test] - fn test_spatial_of_temporal_sum_equivalence() { + fn test_spatial_of_temporal_sql_is_rejected() { + // PromQL still supports (and collapses) sum(sum_over_time(...)) by (label); + // the SQL equivalent is a nested subquery, which the matcher now rejects + // outright with QueryError::NestedQueryUnsupported (#499) rather than + // mis-executing it, so there's no longer a SQL context to compare. let scrape_interval_ms = 1000; let promql_query = "sum(sum_over_time(cpu_usage[10s])) by (L1)"; let sql_query = "SELECT SUM(result) FROM (SELECT SUM(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1"; - // let all_labels = vec!["L1", "L2", "L3", "L4"]; let grouping_labels = vec!["L1"]; let rollup_labels = vec!["L2", "L3", "L4"]; let window_size_ms = 10_000; - // Setup test configuration - // Using SUM of SUM which is collapsable (spatial="sum", temporal="sum_over_time") - let (promql_config, sql_config, streaming_config) = TestConfigBuilder::new("cpu_usage") + let (_, sql_config, streaming_config) = TestConfigBuilder::new("cpu_usage") .with_grouping_labels(grouping_labels) .with_rollup_labels(rollup_labels) .with_scrape_interval_ms(scrape_interval_ms) .add_spatial_of_temporal_query(promql_query, sql_query, 3, window_size_ms) .build_both(); - // Create engines - let promql_engine = SimpleEngine::new( - Arc::new(NoOpStore), - // None, - promql_config, - streaming_config.clone(), - scrape_interval_ms, - QueryLanguage::promql, - ); - let sql_engine = SimpleEngine::new( Arc::new(NoOpStore), - // None, sql_config, streaming_config, scrape_interval_ms, QueryLanguage::sql, ); - // Extract contexts let query_time_sec: f64 = 1_000.0; // Arbitrary timestamp in seconds - let promql_context = promql_engine - .build_query_execution_context_promql(promql_query.to_string(), query_time_sec) - .expect("Failed to build PromQL context"); - - let sql_context = sql_engine - .build_query_execution_context_sql(sql_query.to_string(), query_time_sec) - .expect("Failed to build SQL context"); + let sql_context = + sql_engine.build_query_execution_context_sql(sql_query.to_string(), query_time_sec); - // Assert equivalence - assert_execution_context_equivalent( - &promql_context, - &sql_context, - "spatial_of_temporal_sum", + assert!( + sql_context.is_none(), + "Expected build_query_execution_context_sql to return None for a nested subquery." ); } 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 4c9f579c..e2492462 100644 --- a/asap-query-engine/src/tests/sql_pattern_matching_tests.rs +++ b/asap-query-engine/src/tests/sql_pattern_matching_tests.rs @@ -12,7 +12,6 @@ mod tests { use crate::engines::simple_engine::SimpleEngine; use crate::stores::simple_map_store::SimpleMapStore; use promql_utilities::data_model::KeyByLabelNames; - use promql_utilities::query_logics::enums::Statistic; use sql_utilities::sqlhelper::{SQLSchema, Table}; use std::collections::{HashMap, HashSet}; use std::sync::Arc; @@ -162,8 +161,9 @@ mod tests { } #[test] - fn test_spatial_of_temporal_subquery_matches_now_template() { - // Spatial-of-temporal: outer GROUP BY L1 (subset), inner GROUP BY all labels + fn test_spatial_of_temporal_subquery_is_rejected() { + // Nested SQL (spatial-of-temporal subquery) is unsupported: the matcher rejects + // it with QueryError::NestedQueryUnsupported instead of mis-executing it (#499). let template = "SELECT SUM(result) FROM (SELECT SUM(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) GROUP BY L1"; let engine = build_sql_engine(template, 1, 10_000); @@ -172,28 +172,25 @@ mod tests { let context = engine.build_query_execution_context_sql(incoming.to_string(), query_time); assert!( - context.is_some(), - "Expected build_query_execution_context_sql to return Some for spatial-of-temporal subquery, got None." + context.is_none(), + "Expected build_query_execution_context_sql to return None for a nested subquery." ); } #[test] - fn nested_order_by_limit_is_not_topk() { - // Outer ORDER BY + LIMIT on a spatial-over-temporal query must not be routed - // through CountMinSketchWithHeap; the inner temporal SUM is not a flat top-k. + fn nested_order_by_limit_query_is_rejected() { + // Nested SQL with an outer ORDER BY + LIMIT is unsupported for the same reason: + // the matcher rejects it before topk detection ever runs. let template = "SELECT L1, SUM(result) FROM (SELECT SUM(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1, L2, L3, L4) sub GROUP BY L1"; let engine = build_sql_engine(template, 1, 10_000); let incoming = "SELECT L1, SUM(result) AS rollup FROM (SELECT SUM(value) AS result FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, '2025-10-01 00:00:10') AND '2025-10-01 00:00:10' GROUP BY L1, L2, L3, L4) sub GROUP BY L1 ORDER BY rollup DESC LIMIT 10"; let query_time = 1727740810.0_f64; - let context = engine - .build_query_execution_context_sql(incoming.to_string(), query_time) - .expect("nested spatial-of-temporal query should build a context"); - assert_ne!( - context.metadata.statistic_to_compute, - Statistic::Topk, - "top-k detection skips nested OneTemporalOneSpatial; outer ORDER BY LIMIT must stay on the plain SUM path", + let context = engine.build_query_execution_context_sql(incoming.to_string(), query_time); + assert!( + context.is_none(), + "Expected build_query_execution_context_sql to return None for a nested subquery." ); }