From 8a9c8391db23a1252919978c7e01aaa11a4a5e97 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Sun, 11 Oct 2026 16:18:06 -0400 Subject: [PATCH 1/4] fix(sql-parser): accept global aggregates without group by --- .../src/ast_matching/sqlparser_test.rs | 22 +++++++++++++++++ .../src/ast_matching/sqlpattern_parser.rs | 6 +---- .../tests/sql_query_support/aggregations.yaml | 12 +++++----- .../tests/sql_query_support/basic.yaml | 14 +++++------ .../native-dag-aggregations.yaml | 2 +- .../tests/sql_query_support/olly-bench.yaml | 4 ++-- .../tests/sql_query_support/quantiles.yaml | 24 +++++++++---------- 7 files changed, 51 insertions(+), 33 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 d5925e86..a69d5c95 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 @@ -82,6 +82,28 @@ mod tests { assert!(query.labels.contains("L1")); } + #[test] + fn test_global_aggregate_parses_with_empty_grouping_labels() { + let query = parse_sql_query( + "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW()", + ) + .expect("global aggregate parses"); + + assert!(query.labels.is_empty()); + } + + #[test] + fn test_nested_global_aggregates_parse_with_empty_grouping_labels() { + let query = parse_sql_query( + "SELECT SUM(total) FROM (SELECT SUM(value) AS total FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW())", + ) + .expect("nested global aggregates parse"); + + // Matching and execution may reject nested queries, but parsing retains both layers. + assert!(query.labels.is_empty()); + assert!(query.subquery.expect("inner query").labels.is_empty()); + } + #[test] fn test_pattern_matching() { let schema = create_test_schema(); diff --git a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_parser.rs b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_parser.rs index 659ac325..1b22f105 100644 --- a/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_parser.rs +++ b/asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_parser.rs @@ -847,11 +847,7 @@ impl SQLPatternParser { } } - if group_bys.is_empty() { - None - } else { - Some(group_bys) - } + Some(group_bys) } _ => None, } diff --git a/asap-query-engine/tests/sql_query_support/aggregations.yaml b/asap-query-engine/tests/sql_query_support/aggregations.yaml index 83c5553a..1c1101e2 100644 --- a/asap-query-engine/tests/sql_query_support/aggregations.yaml +++ b/asap-query-engine/tests/sql_query_support/aggregations.yaml @@ -127,7 +127,7 @@ cases: - name: sum query: SELECT SUM(value) FROM data WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: sum-by-job query: SELECT job, SUM(value) FROM data WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY job expected: supported @@ -136,7 +136,7 @@ cases: expected: supported - name: count query: SELECT COUNT(value) FROM data WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: count-by-job query: SELECT job, COUNT(value) FROM data WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY job expected: supported @@ -152,13 +152,13 @@ cases: expected: supported - name: sum-sum-over-time query: SELECT SUM(value) FROM data WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() - expected: unsupported + expected: supported - name: sum-without-job-sum-over-time query: SELECT instance, SUM(value) FROM data WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() GROUP BY instance expected: supported - name: sum-count-over-time query: SELECT COUNT(value) FROM data WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() - expected: unsupported + expected: supported - name: sum-by-job-count-over-time query: SELECT job, COUNT(value) FROM data WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() GROUP BY job expected: supported @@ -170,7 +170,7 @@ cases: expected: supported - name: min-min-over-time query: SELECT MIN(value) FROM data WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() - expected: unsupported + expected: supported - name: min-by-job-min-over-time query: SELECT job, MIN(value) FROM data WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() GROUP BY job expected: supported @@ -182,7 +182,7 @@ cases: expected: supported - name: max-max-over-time query: SELECT MAX(value) FROM data WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() - expected: unsupported + expected: supported - name: max-by-job-max-over-time query: SELECT job, MAX(value) FROM data WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() GROUP BY job expected: supported diff --git a/asap-query-engine/tests/sql_query_support/basic.yaml b/asap-query-engine/tests/sql_query_support/basic.yaml index 8390a696..e88238c0 100644 --- a/asap-query-engine/tests/sql_query_support/basic.yaml +++ b/asap-query-engine/tests/sql_query_support/basic.yaml @@ -35,25 +35,25 @@ cases: query: SELECT L1, COUNT(DISTINCT L2) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1 expected: supported - # No GROUP BY: the pattern parser rejects an empty GROUP BY. + # No GROUP BY: global aggregates use an empty grouping-label set. - name: sum_no_group_by query: SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() - expected: unsupported + expected: supported - name: count_no_group_by query: SELECT COUNT(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() - expected: unsupported + expected: supported - name: min_no_group_by query: SELECT MIN(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() - expected: unsupported + expected: supported - name: max_no_group_by query: SELECT MAX(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile_no_group_by query: SELECT QUANTILE(0.95, value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() - expected: unsupported + expected: supported - name: count_distinct_no_group_by query: SELECT COUNT(DISTINCT L2) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() - expected: unsupported + expected: supported # Time-range forms, with GROUP BY - name: absolute_between_group_by diff --git a/asap-query-engine/tests/sql_query_support/native-dag-aggregations.yaml b/asap-query-engine/tests/sql_query_support/native-dag-aggregations.yaml index 301bda6d..d9b27ddd 100644 --- a/asap-query-engine/tests/sql_query_support/native-dag-aggregations.yaml +++ b/asap-query-engine/tests/sql_query_support/native-dag-aggregations.yaml @@ -11,7 +11,7 @@ repetition_delay_ms: 60000 cases: - name: tumbling-sum query: SELECT SUM(value) FROM data WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: keyed-count query: SELECT job, COUNT(value) FROM data WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY job expected: supported diff --git a/asap-query-engine/tests/sql_query_support/olly-bench.yaml b/asap-query-engine/tests/sql_query_support/olly-bench.yaml index 566bb518..09f61e86 100644 --- a/asap-query-engine/tests/sql_query_support/olly-bench.yaml +++ b/asap-query-engine/tests/sql_query_support/olly-bench.yaml @@ -39,7 +39,7 @@ repetition_delay_ms: 60000 cases: - name: olly-09-sum-memory query: SELECT SUM(value) FROM backend_process_resident_memory_bytes WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: olly-05-max-over-time-cache-lag query: SELECT job, instance, MAX(value) FROM cache_refresh_lag_seconds WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() GROUP BY job, instance expected: supported @@ -51,7 +51,7 @@ cases: expected: supported - name: olly-21-histogram-quantile-0.95 query: SELECT QUANTILE(0.95, value) FROM order_service_http_request_duration_seconds WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() - expected: unsupported + expected: supported - name: olly-23-topk-max-over-time-backlog query: SELECT job, instance, MAX(value) AS v FROM backend_retry_backlog_depth WHERE time BETWEEN DATEADD(s, -300, NOW()) AND NOW() GROUP BY job, instance ORDER BY v DESC LIMIT 2 expected: supported diff --git a/asap-query-engine/tests/sql_query_support/quantiles.yaml b/asap-query-engine/tests/sql_query_support/quantiles.yaml index 98f56112..109fc87b 100644 --- a/asap-query-engine/tests/sql_query_support/quantiles.yaml +++ b/asap-query-engine/tests/sql_query_support/quantiles.yaml @@ -56,40 +56,40 @@ cases: - name: quantile-0.1 query: SELECT QUANTILE(0.1, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.2 query: SELECT QUANTILE(0.2, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.3 query: SELECT QUANTILE(0.3, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.4 query: SELECT QUANTILE(0.4, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.5 query: SELECT QUANTILE(0.5, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.6 query: SELECT QUANTILE(0.6, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.7 query: SELECT QUANTILE(0.7, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.75 query: SELECT QUANTILE(0.75, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.8 query: SELECT QUANTILE(0.8, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.9 query: SELECT QUANTILE(0.9, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.95 query: SELECT QUANTILE(0.95, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.99 query: SELECT QUANTILE(0.99, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() - expected: unsupported + expected: supported - name: quantile-0.5-by-job query: SELECT job, QUANTILE(0.5, value) FROM quantile_wide WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW() GROUP BY job From c9bc66606fbee3722dc4826f8422b1230edc73da Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Sun, 11 Oct 2026 20:11:27 -0400 Subject: [PATCH 2/4] fix(query-engine): execute global SQL aggregates --- .../src/ast_matching/sqlparser_test.rs | 9 ++ .../src/engines/simple_engine/mod.rs | 11 ++- .../src/engines/simple_engine/sql.rs | 6 ++ .../src/tests/sql_pattern_matching_tests.rs | 91 +++++++++++++++---- 4 files changed, 94 insertions(+), 23 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 a69d5c95..5c332376 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 @@ -104,6 +104,15 @@ mod tests { assert!(query.subquery.expect("inner query").labels.is_empty()); } + #[test] + fn test_nested_global_aggregates_remain_unsupported_by_matcher() { + check_query( + "SELECT SUM(total) FROM (SELECT SUM(value) AS total FROM cpu_usage WHERE time BETWEEN DATEADD(s, -1, NOW()) AND NOW())", + vec![], + Some(QueryError::NestedQueryUnsupported), + ); + } + #[test] fn test_pattern_matching() { let schema = create_test_schema(); diff --git a/asap-query-engine/src/engines/simple_engine/mod.rs b/asap-query-engine/src/engines/simple_engine/mod.rs index c0bb98bd..4c9866ee 100644 --- a/asap-query-engine/src/engines/simple_engine/mod.rs +++ b/asap-query-engine/src/engines/simple_engine/mod.rs @@ -1361,6 +1361,9 @@ impl SimpleEngine { } }, None => match value_precompute.get_keys() { + Some(keys) if keys.is_empty() => { + vec![(fallback_key.clone(), fallback_key.clone())] + } Some(keys) => keys .into_iter() .filter_map(|self_key| { @@ -3116,11 +3119,9 @@ impl SimpleEngine { Some(&query_bounds), ) .into_iter() - // A fully unlabeled result (fallback_key was None and - // the value accumulator has no self-keys) has no - // RangeVectorElement representation -- matches today's - // behavior of producing no sample for this combination. - .filter_map(|(key, value)| key.map(|key| (key, value))) + // A missing store key denotes the single scalar row of + // a global aggregate; represent it with empty labels. + .map(|(key, value)| (key.unwrap_or_else(KeyByLabelValues::new), value)) .collect(); // Rank and truncate to k within this group only -- a diff --git a/asap-query-engine/src/engines/simple_engine/sql.rs b/asap-query-engine/src/engines/simple_engine/sql.rs index 11b71320..b385a037 100644 --- a/asap-query-engine/src/engines/simple_engine/sql.rs +++ b/asap-query-engine/src/engines/simple_engine/sql.rs @@ -503,6 +503,12 @@ impl SimpleEngine { "No query_config entry for SQL spatio-temporal query. Attempting capability-based matching." ); let requirements = self.build_query_requirements_sql(match_result, topk); + // A global aggregate has no output grouping labels. Capability + // requirements cannot express whether a candidate is internally + // keyed, so only an exact query configuration can safely serve it. + if topk.is_none() && requirements.grouping_labels.labels.is_empty() { + return None; + } self.streaming_config .read() .unwrap() 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 66a867fa..7401492a 100644 --- a/asap-query-engine/src/tests/sql_pattern_matching_tests.rs +++ b/asap-query-engine/src/tests/sql_pattern_matching_tests.rs @@ -7,10 +7,12 @@ mod tests { use crate::data_model::{ AggregationConfig, AggregationReference, AggregationType, CleanupPolicy, InferenceConfig, - QueryConfig, QueryLanguage, SchemaConfig, StreamingConfig, WindowType, + PrecomputedOutput, QueryConfig, QueryLanguage, SchemaConfig, StreamingConfig, WindowType, }; use crate::engines::simple_engine::SimpleEngine; + use crate::precompute_operators::sum_accumulator::SumAccumulator; use crate::stores::simple_map_store::SimpleMapStore; + use crate::stores::Store; use promql_utilities::data_model::KeyByLabelNames; use sql_utilities::sqlhelper::{SQLSchema, Table}; use std::collections::{HashMap, HashSet}; @@ -21,7 +23,11 @@ mod tests { /// * `template_sql` — the NOW()-based query stored in inference_config /// * `agg_id` — aggregation id /// * `window_size_ms` — window size in ms - fn build_sql_engine(template_sql: &str, agg_id: u64, window_size_ms: u64) -> SimpleEngine { + fn build_sql_engine( + template_sql: &str, + agg_id: u64, + window_size_ms: u64, + ) -> (SimpleEngine, Arc) { // Schema: cpu_usage table let labels: HashSet = ["L1", "L2", "L3", "L4"] .iter() @@ -52,12 +58,16 @@ mod tests { aggregation_type: AggregationType::Sum, aggregation_sub_type: String::new(), parameters: HashMap::new(), - grouping_labels: KeyByLabelNames::new( - ["L1", "L2", "L3", "L4"] - .iter() - .map(|s| s.to_string()) - .collect(), - ), + grouping_labels: if template_sql.contains("GROUP BY") { + KeyByLabelNames::new( + ["L1", "L2", "L3", "L4"] + .iter() + .map(|s| s.to_string()) + .collect(), + ) + } else { + KeyByLabelNames::empty() + }, aggregated_labels: KeyByLabelNames::empty(), rollup_labels: KeyByLabelNames::empty(), original_yaml: String::new(), @@ -84,20 +94,21 @@ mod tests { CleanupPolicy::NoCleanup, )); - SimpleEngine::new( - store, + let engine = SimpleEngine::new( + store.clone(), inference_config, streaming_config, 1000, QueryLanguage::sql, - ) + ); + (engine, store) } #[test] fn test_temporal_query_matches_now_template() { // Template in inference_config uses NOW() let template = "SELECT SUM(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); + let (engine, _) = build_sql_engine(template, 1, 10_000); // Incoming query uses absolute timestamps for the same 10s window let incoming = "SELECT SUM(value) 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"; @@ -115,7 +126,7 @@ mod tests { fn test_spatiotemporal_query_matches_now_template() { // SpatioTemporal: same metric, spans multiple intervals, GROUP BY subset of labels let template = "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1"; - let engine = build_sql_engine(template, 1, 10_000); + let (engine, _) = build_sql_engine(template, 1, 10_000); let incoming = "SELECT SUM(value) 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"; let query_time = 1727740810.0_f64; @@ -127,12 +138,56 @@ mod tests { ); } + #[test] + fn global_aggregate_does_not_fallback_to_grouped_config() { + let template = "SELECT L1, SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1"; + let (engine, _) = build_sql_engine(template, 1, 10_000); + let query = "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, '2025-10-01 00:00:10') AND '2025-10-01 00:00:10'"; + + assert!( + engine + .build_query_execution_context_sql(query.to_string(), 1_727_740_810.0) + .is_none(), + "a global aggregate must not use capability fallback to select a grouped config" + ); + } + + #[test] + fn global_sum_returns_one_unlabeled_result() { + let template = + "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -2, NOW()) AND NOW()"; + let (engine, store) = build_sql_engine(template, 1, 1_000); + let context = engine + .build_query_execution_context_sql(template.to_string(), 1_000_000.0) + .expect("global sum context builds"); + + // The configured aggregation is global, so each stored window has no label key. + for (start, end, value) in [ + (999_998_000, 999_999_000, 3.0), + (999_999_000, 1_000_000_000, 5.0), + ] { + store + .insert_precomputed_output( + PrecomputedOutput::new(start, end, None, 1), + Box::new(SumAccumulator::with_sum(value)), + ) + .expect("insert global sum window"); + } + + let results = engine + .execute_query_pipeline(&context, false, false) + .expect("global sum executes"); + assert_eq!(results.len(), 1); + assert!(results[0].labels.labels.is_empty()); + assert_eq!(results[0].value, 8.0); + } + #[test] fn test_spatial_query_matches_now_template() { // 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"; - let engine = build_sql_engine(template, 1, 1000); + 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"; let query_time = 1727740810.0_f64; @@ -148,7 +203,7 @@ mod tests { fn test_temporal_quantile_query_matches_now_template() { // 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); + let (engine, _) = build_sql_engine(template, 1, 10_000); let incoming = "SELECT QUANTILE(0.95, value) 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"; let query_time = 1727740810.0_f64; @@ -165,7 +220,7 @@ mod tests { // 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); + let (engine, _) = build_sql_engine(template, 1, 10_000); let incoming = "SELECT SUM(result) 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) GROUP BY L1"; let query_time = 1727740810.0_f64; @@ -182,7 +237,7 @@ mod tests { // 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 (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; @@ -198,7 +253,7 @@ mod tests { fn test_no_match_returns_none() { // Engine has a SUM template; incoming uses AVG — should never match let template = "SELECT SUM(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); + let (engine, _) = build_sql_engine(template, 1, 10_000); let incoming = "SELECT AVG(value) 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"; let query_time = 1727740810.0_f64; From 2aaed9ab54a297a5a652c7ccfd2823c473174f92 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Mon, 12 Oct 2026 09:41:32 -0400 Subject: [PATCH 3/4] fix(query-engine): stub benchmark binary in Docker build --- asap-query-engine/Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/asap-query-engine/Dockerfile b/asap-query-engine/Dockerfile index ea778113..ae28cb37 100644 --- a/asap-query-engine/Dockerfile +++ b/asap-query-engine/Dockerfile @@ -31,6 +31,7 @@ RUN mkdir -p asap-query-engine/src/bin \ && echo "fn main() {}" > asap-query-engine/src/bin/precompute_engine.rs \ && echo "fn main() {}" > asap-query-engine/src/bin/test_e2e_precompute.rs \ && echo "fn main() {}" > asap-query-engine/src/bin/bench_precompute_sketch.rs \ + && echo "fn main() {}" > asap-query-engine/src/bin/bench_ingest_sender.rs \ && echo "fn main() {}" > asap-query-engine/src/bin/e2e_quickstart_resource_test.rs \ && mkdir -p asap-planner-rs/src && echo "fn main() {}" > asap-planner-rs/src/main.rs \ && echo "pub fn placeholder() {}" >> asap-planner-rs/src/lib.rs From 5d9b36e628465c76a1f61a1ab736ee95822a96ad Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 10:15:35 -0400 Subject: [PATCH 4/4] fix(query-engine): reject partitioned scalar fallbacks --- .../rs/asap_types/src/capability_matching.rs | 24 ++++ .../src/engines/simple_engine/mod.rs | 3 - .../src/engines/simple_engine/sql.rs | 6 - .../src/tests/native_range_query_tests.rs | 73 ++++++++++++- .../src/tests/sql_pattern_matching_tests.rs | 103 ++++++++++++++---- 5 files changed, 172 insertions(+), 37 deletions(-) diff --git a/asap-common/dependencies/rs/asap_types/src/capability_matching.rs b/asap-common/dependencies/rs/asap_types/src/capability_matching.rs index c6f754ff..4ab4b9b5 100644 --- a/asap-common/dependencies/rs/asap_types/src/capability_matching.rs +++ b/asap-common/dependencies/rs/asap_types/src/capability_matching.rs @@ -157,6 +157,15 @@ pub fn labels_compatible(config_labels: &KeyByLabelNames, req_labels: &KeyByLabe config_labels == req_labels } +fn scalar_requirement_compatible( + config: &AggregationConfig, + requirements: &QueryRequirements, +) -> bool { + !requirements.grouping_labels.labels.is_empty() + || requirements.statistics.contains(&Statistic::Topk) + || config.aggregated_labels.labels.is_empty() +} + /// Spatial filter compatibility. /// - Both empty → compatible. /// - Config non-empty and matches query → compatible. @@ -290,6 +299,7 @@ pub fn find_compatible_aggregation( && sub_type.is_none_or(|st| c.aggregation_sub_type.eq_ignore_ascii_case(st)) && window_compatible(c, requirements.data_range_ms) && labels_compatible(&c.grouping_labels, &requirements.grouping_labels) + && scalar_requirement_compatible(c, requirements) && spatial_filter_compatible( &c.spatial_filter_normalized, &requirements.spatial_filter_normalized, @@ -552,6 +562,20 @@ mod tests { assert_eq!(result.unwrap().aggregation_id_for_value, 1); } + #[test] + fn scalar_sum_does_not_match_an_internally_partitioned_config() { + let mut value = make_config(1, "cpu", "MultipleSum", "sum", 300_000, "tumbling", &[], ""); + value.aggregated_labels = KeyByLabelNames::new(vec!["host".to_string()]); + let key = make_config(2, "cpu", "SetAggregator", "", 300_000, "tumbling", &[], ""); + let configs = HashMap::from([(1, value), (2, key)]); + + assert!( + find_compatible_aggregation(&configs, &req("cpu", &[Statistic::Sum], 300_000, &[], "")) + .is_none(), + "a scalar query cannot be served by an internally partitioned aggregation" + ); + } + #[test] fn plain_cms_matching_respects_sum_and_count_subtypes() { let mut configs = HashMap::new(); diff --git a/asap-query-engine/src/engines/simple_engine/mod.rs b/asap-query-engine/src/engines/simple_engine/mod.rs index 4c9866ee..03a4a24b 100644 --- a/asap-query-engine/src/engines/simple_engine/mod.rs +++ b/asap-query-engine/src/engines/simple_engine/mod.rs @@ -1361,9 +1361,6 @@ impl SimpleEngine { } }, None => match value_precompute.get_keys() { - Some(keys) if keys.is_empty() => { - vec![(fallback_key.clone(), fallback_key.clone())] - } Some(keys) => keys .into_iter() .filter_map(|self_key| { diff --git a/asap-query-engine/src/engines/simple_engine/sql.rs b/asap-query-engine/src/engines/simple_engine/sql.rs index b385a037..11b71320 100644 --- a/asap-query-engine/src/engines/simple_engine/sql.rs +++ b/asap-query-engine/src/engines/simple_engine/sql.rs @@ -503,12 +503,6 @@ impl SimpleEngine { "No query_config entry for SQL spatio-temporal query. Attempting capability-based matching." ); let requirements = self.build_query_requirements_sql(match_result, topk); - // A global aggregate has no output grouping labels. Capability - // requirements cannot express whether a candidate is internally - // keyed, so only an exact query configuration can safely serve it. - if topk.is_none() && requirements.grouping_labels.labels.is_empty() { - return None; - } self.streaming_config .read() .unwrap() diff --git a/asap-query-engine/src/tests/native_range_query_tests.rs b/asap-query-engine/src/tests/native_range_query_tests.rs index c900a7e9..209426b3 100644 --- a/asap-query-engine/src/tests/native_range_query_tests.rs +++ b/asap-query-engine/src/tests/native_range_query_tests.rs @@ -2202,7 +2202,11 @@ mod tests { fn create_range_engine_self_keyed( metric: &str, aggregated_label: &str, - data: Vec<(u64, CountMinSketchWithHeapAccumulator)>, + data: Vec<( + u64, + Option, + CountMinSketchWithHeapAccumulator, + )>, promql_query: &str, ) -> SimpleEngine { let mut aggregation_configs = HashMap::new(); @@ -2239,8 +2243,8 @@ mod tests { CleanupPolicy::NoCleanup, )); - for (timestamp, acc) in data { - let output = PrecomputedOutput::new(timestamp - WINDOW_MS, timestamp, None, 1); + for (timestamp, key, acc) in data { + let output = PrecomputedOutput::new(timestamp - WINDOW_MS, timestamp, key, 1); store .insert_precomputed_output(output, Box::new(acc)) .unwrap(); @@ -2287,8 +2291,12 @@ mod tests { sketch.inner.update("host-c", 10.0); let query = "topk(5, transfer_events)"; - let engine = - create_range_engine_self_keyed("transfer_events", "srcip", vec![(1000, sketch)], query); + let engine = create_range_engine_self_keyed( + "transfer_events", + "srcip", + vec![(1000, None, sketch)], + query, + ); let result = engine .handle_range_query_promql(query.to_string(), 1.0, 1.5, 1.0) @@ -2333,6 +2341,61 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread")] + async fn range_query_empty_self_keyed_topk_does_not_emit_outer_key() { + let query = "topk(5, transfer_events)"; + let engine = create_range_engine_self_keyed( + "transfer_events", + "srcip", + vec![( + 1000, + Some(KeyByLabelValues::new()), + CountMinSketchWithHeapAccumulator::new(3, 1024, 32), + )], + query, + ); + + let result = engine + .handle_range_query_promql(query.to_string(), 1.0, 1.5, 1.0) + .expect("native query execution should not fail"); + let (_, query_result) = result.expect("range query failed"); + + assert!( + matrix_values(query_result).is_empty(), + "an empty self-keyed heap must not become an unlabeled result" + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn range_query_global_sum_returns_one_unlabeled_series() { + let query = "sum(requests)"; + let engine = create_engine_multi_timestamp_with_window( + "requests", + AggregationType::Sum, + vec![], + vec![( + 1000, + None, + Box::new(SumAccumulator::with_sum(42.0)) as Box, + )], + query, + 1000, + 1000, + WindowType::Tumbling, + ); + + let result = engine + .handle_range_query_promql(query.to_string(), 1.0, 1.5, 1.0) + .expect("native query execution should not fail"); + let (_, query_result) = result.expect("range query failed"); + let elements = matrix_values(query_result); + + assert_eq!(elements.len(), 1); + assert_eq!(elements[0].labels, KeyByLabelValues::new()); + assert_eq!(elements[0].samples.len(), 1); + assert_eq!(elements[0].samples[0].value, 42.0); + } + #[tokio::test(flavor = "multi_thread")] async fn range_query_set_aggregator_merges_multiple_buckets_within_one_window() { // Every other SetAggregator test uses window_size_ms == bucket 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 7401492a..8204cefc 100644 --- a/asap-query-engine/src/tests/sql_pattern_matching_tests.rs +++ b/asap-query-engine/src/tests/sql_pattern_matching_tests.rs @@ -27,6 +27,9 @@ mod tests { template_sql: &str, agg_id: u64, window_size_ms: u64, + aggregation_type: AggregationType, + grouping_labels: &[&str], + aggregated_labels: &[&str], ) -> (SimpleEngine, Arc) { // Schema: cpu_usage table let labels: HashSet = ["L1", "L2", "L3", "L4"] @@ -55,20 +58,25 @@ mod tests { // Streaming config let agg_config = AggregationConfig { aggregation_id: agg_id, - aggregation_type: AggregationType::Sum, - aggregation_sub_type: String::new(), - parameters: HashMap::new(), - grouping_labels: if template_sql.contains("GROUP BY") { - KeyByLabelNames::new( - ["L1", "L2", "L3", "L4"] - .iter() - .map(|s| s.to_string()) - .collect(), - ) + aggregation_type, + aggregation_sub_type: if aggregation_type == AggregationType::MultipleSum { + "sum".to_string() } else { - KeyByLabelNames::empty() + String::new() }, - aggregated_labels: KeyByLabelNames::empty(), + parameters: HashMap::new(), + grouping_labels: KeyByLabelNames::new( + grouping_labels + .iter() + .map(|label| label.to_string()) + .collect(), + ), + aggregated_labels: KeyByLabelNames::new( + aggregated_labels + .iter() + .map(|label| label.to_string()) + .collect(), + ), rollup_labels: KeyByLabelNames::empty(), original_yaml: String::new(), window_size_ms, @@ -108,7 +116,14 @@ mod tests { fn test_temporal_query_matches_now_template() { // Template in inference_config uses NOW() let template = "SELECT SUM(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); + let (engine, _) = build_sql_engine( + template, + 1, + 10_000, + AggregationType::Sum, + &["L1", "L2", "L3", "L4"], + &[], + ); // Incoming query uses absolute timestamps for the same 10s window let incoming = "SELECT SUM(value) 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"; @@ -126,7 +141,7 @@ mod tests { fn test_spatiotemporal_query_matches_now_template() { // SpatioTemporal: same metric, spans multiple intervals, GROUP BY subset of labels let template = "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1"; - let (engine, _) = build_sql_engine(template, 1, 10_000); + let (engine, _) = build_sql_engine(template, 1, 10_000, AggregationType::Sum, &["L1"], &[]); let incoming = "SELECT SUM(value) 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"; let query_time = 1727740810.0_f64; @@ -139,16 +154,37 @@ mod tests { } #[test] - fn global_aggregate_does_not_fallback_to_grouped_config() { + fn global_aggregate_does_not_fallback_to_internally_partitioned_config() { let template = "SELECT L1, SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1"; - let (engine, _) = build_sql_engine(template, 1, 10_000); + let (engine, _) = build_sql_engine( + template, + 1, + 10_000, + AggregationType::MultipleSum, + &[], + &["L1"], + ); let query = "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, '2025-10-01 00:00:10') AND '2025-10-01 00:00:10'"; assert!( engine .build_query_execution_context_sql(query.to_string(), 1_727_740_810.0) .is_none(), - "a global aggregate must not use capability fallback to select a grouped config" + "a global aggregate must not use capability fallback to select an internally partitioned config" + ); + } + + #[test] + fn global_aggregate_falls_back_to_scalar_config() { + let template = "SELECT L1, SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, NOW()) AND NOW() GROUP BY L1"; + let (engine, _) = build_sql_engine(template, 1, 10_000, AggregationType::Sum, &[], &[]); + let query = "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -10, '2025-10-01 00:00:10') AND '2025-10-01 00:00:10'"; + + assert!( + engine + .build_query_execution_context_sql(query.to_string(), 1_727_740_810.0) + .is_some(), + "a global aggregate can use capability fallback to select a scalar config" ); } @@ -156,7 +192,7 @@ mod tests { fn global_sum_returns_one_unlabeled_result() { let template = "SELECT SUM(value) FROM cpu_usage WHERE time BETWEEN DATEADD(s, -2, NOW()) AND NOW()"; - let (engine, store) = build_sql_engine(template, 1, 1_000); + let (engine, store) = build_sql_engine(template, 1, 1_000, AggregationType::Sum, &[], &[]); let context = engine .build_query_execution_context_sql(template.to_string(), 1_000_000.0) .expect("global sum context builds"); @@ -187,7 +223,14 @@ mod tests { // 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"; - let (engine, _) = build_sql_engine(template, 1, 1000); + let (engine, _) = build_sql_engine( + template, + 1, + 1000, + AggregationType::Sum, + &["L1", "L2", "L3", "L4"], + &[], + ); 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"; let query_time = 1727740810.0_f64; @@ -203,7 +246,14 @@ mod tests { fn test_temporal_quantile_query_matches_now_template() { // 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); + let (engine, _) = build_sql_engine( + template, + 1, + 10_000, + AggregationType::Sum, + &["L1", "L2", "L3", "L4"], + &[], + ); let incoming = "SELECT QUANTILE(0.95, value) 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"; let query_time = 1727740810.0_f64; @@ -220,7 +270,7 @@ mod tests { // 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); + let (engine, _) = build_sql_engine(template, 1, 10_000, AggregationType::Sum, &["L1"], &[]); let incoming = "SELECT SUM(result) 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) GROUP BY L1"; let query_time = 1727740810.0_f64; @@ -237,7 +287,7 @@ mod tests { // 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 (engine, _) = build_sql_engine(template, 1, 10_000, AggregationType::Sum, &["L1"], &[]); 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; @@ -253,7 +303,14 @@ mod tests { fn test_no_match_returns_none() { // Engine has a SUM template; incoming uses AVG — should never match let template = "SELECT SUM(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); + let (engine, _) = build_sql_engine( + template, + 1, + 10_000, + AggregationType::Sum, + &["L1", "L2", "L3", "L4"], + &[], + ); let incoming = "SELECT AVG(value) 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"; let query_time = 1727740810.0_f64;