Restrict SQL top-k detection to OnlyTemporal queries

Author: akanksha-akkihal

asap-query-engine/src/engines/simple_engine/sql.rs

@@ -606,12 +606,16 @@ impl SimpleEngine {
             }
         };
 
-        // Top-k detection takes precedence: `... ORDER BY <agg alias> DESC LIMIT k`
-        // is served by CountMinSketchWithHeap (Statistic::Topk) rather than the
-        // plain COUNT/SUM path, so the sketch heap drives the result set. Both
-        // COUNT(col) and SUM(col) map to Topk; they differ only in how the
-        // backing sketch is weighted (see TopkWeighting).
-        let topk = detect_sql_topk(&query_data);
+        // Top-k (CountMinSketchWithHeap) is defined only for flat temporal queries:
+        // one window, one GROUP BY key, COUNT/SUM ... ORDER BY <agg alias> 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.
+        let topk = match query_pattern_type {
+            QueryPatternType::OnlyTemporal => detect_sql_topk(&query_data),
+            QueryPatternType::OnlySpatial | QueryPatternType::OneTemporalOneSpatial => None,
+        };
         let statistic_to_compute = if topk.is_some() {
             Statistic::Topk
         } else {
@@ -961,6 +965,24 @@ mod detect_topk_tests {
         let qd = parse(&sql).expect("query should parse");
         assert_eq!(detect_sql_topk(&qd), None);
     }
+
+    #[test]
+    fn nested_outer_layer_would_match_detect_sql_topk() {
+        // Spatial-over-temporal: ORDER BY / LIMIT sit on the outer SELECT, so the
+        // parsed top-level `query_data` looks like SUM top-k even though the temporal
+        // aggregate is in the subquery. The engine must not promote this to Topk.
+        let sql = format!(
+            "SELECT srcip, SUM(bytes) AS rollup FROM ( \
+               SELECT srcip, dstip, SUM(pkt_len) AS bytes FROM netflow_table {WINDOW} \
+               GROUP BY srcip, dstip \
+             ) sub GROUP BY srcip ORDER BY rollup DESC LIMIT 10"
+        );
+        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 OnlyTemporal is gated)",
+        );
+    }
 }
 
 #[cfg(test)]

asap-query-engine/src/tests/sql_pattern_matching_tests.rs

@@ -12,6 +12,7 @@ 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;
@@ -176,6 +177,26 @@ mod tests {
         );
     }
 
+    #[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.
+        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);
+
+        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 is OnlyTemporal-only; nested outer ORDER BY LIMIT must stay on the plain SUM path",
+        );
+    }
+
     #[test]
     fn test_no_match_returns_none() {
         // Engine has a SUM template; incoming uses AVG — should never match