feat(sql-parser): Support half-open time >= A AND time < B in SQL WHERE clause for benchmark parity

asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs

@@ -1217,4 +1217,158 @@ mod tests {
             Some(QueryError::InvalidValueCol),
         );
     }
+
+    // ── Half-open range: `time >= A AND time < B` ────────────────────────────
+    //
+    // ClickHouse runs `>=`/`<` as a true half-open `[start, end)` scan, which is
+    // how ASAP selects precompute windows — so a query written this way is
+    // semantically equivalent on both backends (issue #401). We accept STRICTLY
+    // `>=` (lower) + `<` (upper); any other operator combination is rejected.
+
+    /// A half-open `>=/<` clause must produce the same `(start, duration)` as the
+    /// equivalent inclusive-looking `BETWEEN` (downstream window selection is
+    /// identical because both reduce to the same TimeInfo).
+    #[test]
+    fn test_half_open_matches_between_time_info() {
+        let between = parse_sql_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' \
+             GROUP BY L1, L2, L3, L4",
+        )
+        .expect("BETWEEN query should parse");
+
+        let half_open = parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time >= DATEADD(s, -10, '2025-10-01 00:00:10') AND time < '2025-10-01 00:00:10' \
+             GROUP BY L1, L2, L3, L4",
+        )
+        .expect("half-open query should parse");
+
+        assert_eq!(
+            half_open.time_info.get_time_col_name(),
+            between.time_info.get_time_col_name()
+        );
+        assert_eq!(
+            half_open.time_info.get_start(),
+            between.time_info.get_start()
+        );
+        assert_eq!(
+            half_open.time_info.get_duration(),
+            between.time_info.get_duration()
+        );
+        assert_eq!(half_open.time_info.get_duration(), 10.0);
+    }
+
+    /// The two comparisons may appear in either order.
+    #[test]
+    fn test_half_open_reversed_operand_order() {
+        let q = parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time < '2025-10-01 00:00:10' AND time >= DATEADD(s, -10, '2025-10-01 00:00:10') \
+             GROUP BY L1",
+        )
+        .expect("reversed-order half-open query should parse");
+        assert_eq!(q.time_info.get_duration(), 10.0);
+    }
+
+    /// The time column may sit on the right-hand side of each comparison
+    /// (`A <= time AND B > time` ≡ `time >= A AND time < B`).
+    #[test]
+    fn test_half_open_column_on_right() {
+        let q = parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE DATEADD(s, -10, '2025-10-01 00:00:10') <= time AND '2025-10-01 00:00:10' > time \
+             GROUP BY L1",
+        )
+        .expect("column-on-right half-open query should parse");
+        assert_eq!(q.time_info.get_duration(), 10.0);
+    }
+
+    /// NOW()-relative half-open ranges parse (used by query templates).
+    #[test]
+    fn test_half_open_now_relative() {
+        let q = parse_sql_query(
+            "SELECT AVG(value) FROM cpu_usage \
+             WHERE time >= DATEADD(s, -1, NOW()) AND time < NOW() \
+             GROUP BY L1",
+        )
+        .expect("NOW()-relative half-open query should parse");
+        assert_eq!(q.time_info.get_duration(), 1.0);
+    }
+
+    /// A half-open query classifies through the matcher exactly like its BETWEEN
+    /// counterpart (1s span over a label subset ⇒ Spatial).
+    #[test]
+    fn test_half_open_pattern_matching() {
+        check_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time >= DATEADD(s, -1, NOW()) AND time < NOW() \
+             GROUP BY L1",
+            vec![QueryType::Spatial],
+            None,
+        );
+    }
+
+    /// A half-open template and a half-open incoming query (absolute timestamps)
+    /// match via duration-based `TimeInfo::matches_pattern`.
+    #[test]
+    fn test_half_open_template_matches_absolute_query() {
+        let template = parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time >= DATEADD(s, -10, NOW()) AND time < NOW() \
+             GROUP BY L1, L2, L3, L4",
+        )
+        .unwrap();
+        let incoming = parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time >= DATEADD(s, -10, '2025-10-01 00:00:10') AND time < '2025-10-01 00:00:10' \
+             GROUP BY L1, L2, L3, L4",
+        )
+        .unwrap();
+        assert!(incoming.matches_sql_pattern(&template));
+    }
+
+    /// Strict rejection: `>` lower bound is not accepted.
+    #[test]
+    fn test_half_open_rejects_gt_lower_bound() {
+        assert!(parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time > DATEADD(s, -10, '2025-10-01 00:00:10') AND time < '2025-10-01 00:00:10' \
+             GROUP BY L1",
+        )
+        .is_none());
+    }
+
+    /// Strict rejection: `<=` upper bound is not accepted.
+    #[test]
+    fn test_half_open_rejects_lte_upper_bound() {
+        assert!(parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time >= DATEADD(s, -10, '2025-10-01 00:00:10') AND time <= '2025-10-01 00:00:10' \
+             GROUP BY L1",
+        )
+        .is_none());
+    }
+
+    /// Two lower bounds is not a valid range.
+    #[test]
+    fn test_half_open_rejects_two_lower_bounds() {
+        assert!(parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time >= DATEADD(s, -10, '2025-10-01 00:00:10') AND time >= '2025-10-01 00:00:10' \
+             GROUP BY L1",
+        )
+        .is_none());
+    }
+
+    /// Both comparisons must reference the same column.
+    #[test]
+    fn test_half_open_rejects_mismatched_columns() {
+        assert!(parse_sql_query(
+            "SELECT SUM(value) FROM cpu_usage \
+             WHERE time >= DATEADD(s, -10, '2025-10-01 00:00:10') AND L1 < '2025-10-01 00:00:10' \
+             GROUP BY L1",
+        )
+        .is_none());
+    }
 }

asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_parser.rs

@@ -6,6 +6,28 @@ use std::collections::HashSet;
 use parse_datetime::parse_datetime;
 use sqlparser::ast::Value::SingleQuotedString;
 
+/// One side of a half-open `time >= A AND time < B` range, carrying its
+/// resolved timestamp (seconds).
+enum TimeBound {
+    /// `time >= ts` — inclusive lower bound.
+    Lower(f64),
+    /// `time < ts` — exclusive upper bound.
+    Upper(f64),
+}
+
+/// Mirror a comparison operator for operand swaps (`A <= time` ≡ `time >= A`).
+/// Only the operators relevant to half-open time ranges are mirrored; anything
+/// else returns `None` and is rejected upstream.
+fn mirror_operator(op: &BinaryOperator) -> Option<BinaryOperator> {
+    match op {
+        BinaryOperator::Lt => Some(BinaryOperator::Gt),
+        BinaryOperator::LtEq => Some(BinaryOperator::GtEq),
+        BinaryOperator::Gt => Some(BinaryOperator::Lt),
+        BinaryOperator::GtEq => Some(BinaryOperator::LtEq),
+        _ => None,
+    }
+}
+
 pub struct SQLPatternParser {
     #[allow(dead_code)]
     schema: SQLSchema,
@@ -523,9 +545,10 @@ impl SQLPatternParser {
                 _ => None,
             },
 
-            _ => {
-                panic!("invalid time syntax {:?}", highlow);
-            }
+            // Unrecognized time syntax: return None so the query is treated as
+            // unmatched (and forwarded / reported as unsupported) rather than
+            // crashing the engine or planner.
+            _ => None,
         }
     }
 
@@ -556,10 +579,88 @@ impl SQLPatternParser {
 
                 Some(TimeInfo::new(col_name, start, duration))
             }
+
+            // Half-open range: `time >= <start> AND time < <end>`.
+            //
+            // ClickHouse executes `>=`/`<` as a true half-open `[start, end)`
+            // scan, which is exactly how ASAP selects precompute windows — so a
+            // query written this way answers the same question on both backends
+            // (unlike inclusive `BETWEEN`). We accept STRICTLY `>=` for the lower
+            // bound and `<` for the upper bound; any other operator combination
+            // (`>`, `<=`) returns None and is treated as unmatched, to avoid a
+            // silent off-by-one against the ClickHouse baseline.
+            Expr::BinaryOp {
+                left,
+                op: BinaryOperator::And,
+                right,
+            } => self.get_time_info_from_half_open(left, right),
+
             _ => None,
         }
     }
 
+    /// Parse a `time >= A AND time < B` conjunction into `TimeInfo`.
+    ///
+    /// The two comparisons may appear in either order, and the time column may
+    /// be on either side of each comparison. Both comparisons must reference the
+    /// same column. Returns `None` unless there is exactly one `>=` lower bound
+    /// and exactly one `<` upper bound.
+    fn get_time_info_from_half_open(&self, left: &Expr, right: &Expr) -> Option<TimeInfo> {
+        let (lcol, lbound) = self.parse_time_comparison(left)?;
+        let (rcol, rbound) = self.parse_time_comparison(right)?;
+
+        if lcol != rcol {
+            return None;
+        }
+
+        let (start, end) = match (lbound, rbound) {
+            (TimeBound::Lower(start), TimeBound::Upper(end)) => (start, end),
+            (TimeBound::Upper(end), TimeBound::Lower(start)) => (start, end),
+            // Two lower bounds or two upper bounds is not a valid range.
+            _ => return None,
+        };
+
+        let duration = end - start;
+
+        Some(TimeInfo::new(lcol, start, duration))
+    }
+
+    /// Parse a single comparison of the form `time >= <expr>` or `time < <expr>`
+    /// (column on either side) into `(column_name, TimeBound)`.
+    ///
+    /// Strictly accepts only `>=` (lower) and `<` (upper); every other operator
+    /// returns `None`.
+    fn parse_time_comparison(&self, expr: &Expr) -> Option<(String, TimeBound)> {
+        let Expr::BinaryOp { left, op, right } = expr else {
+            return None;
+        };
+
+        // Normalize so the column identifier is on the left. If the column is on
+        // the right instead, flip the operator to its mirror so the bound
+        // classification below stays correct (e.g. `A <= time` ≡ `time >= A`).
+        let (col_expr, op, ts_expr) = match (left.as_ref(), right.as_ref()) {
+            (Expr::Identifier(_), _) => (left.as_ref(), op.clone(), right.as_ref()),
+            (_, Expr::Identifier(_)) => (right.as_ref(), mirror_operator(op)?, left.as_ref()),
+            _ => return None,
+        };
+
+        let col_name = match col_expr {
+            Expr::Identifier(ident) => ident.value.clone(),
+            _ => return None,
+        };
+
+        let ts = self.get_timestamp_from_between_highlow(ts_expr)?;
+
+        let bound = match op {
+            BinaryOperator::GtEq => TimeBound::Lower(ts),
+            BinaryOperator::Lt => TimeBound::Upper(ts),
+            // `>` and `<=` are intentionally rejected (see get_time_info).
+            _ => return None,
+        };
+
+        Some((col_name, bound))
+    }
+
     fn parse_dateadd(&self, func: &Function) -> Option<f64> {
         let args = match &func.args {
             FunctionArguments::List(args) => &args.args,

asap-planner-rs/tests/sql_integration.rs

@@ -285,6 +285,65 @@ fn temporal_quantile() {
     assert_eq!(out.inference_cleanup_param(q), Some(1));
 }
 
+// ── Half-open time range: `time >= A AND time < B` (issue #408) ───────────────
+//
+// The planner shares the SQL time-clause parser with the query engine, so the
+// half-open form must produce a plan identical to the equivalent `BETWEEN`.
+// These tests close the end-to-end loop at the planner layer and guard against
+// future parser refactors silently dropping half-open support.
+
+/// Half-open `time >= DATEADD(s, -300, NOW()) AND time < NOW()` must generate
+/// the exact same plan as the `BETWEEN`-based `temporal_quantile` test
+/// (DatasketchesKLL, window = 300 s, grouping = [datacenter],
+/// rollup = [hostname, region], cleanup = 1).
+#[test]
+fn temporal_quantile_half_open() {
+    let q = "SELECT quantile(0.95)(cpu_usage) FROM metrics_table WHERE time >= DATEADD(s, -300, NOW()) AND time < NOW() GROUP BY datacenter";
+    let out = SQLController::from_yaml(&one_query_config(q, 300), sql_opts())
+        .unwrap()
+        .generate()
+        .unwrap();
+
+    assert_eq!(out.streaming_aggregation_count(), 1);
+    assert_eq!(out.inference_query_count(), 1);
+    assert!(out.has_aggregation_type("DatasketchesKLL"));
+    assert!(!out.has_aggregation_type("DeltaSetAggregator"));
+    assert!(out.all_tumbling_window_sizes_eq(300));
+    assert_eq!(
+        out.aggregation_table_name("DatasketchesKLL"),
+        Some("metrics_table".to_string())
+    );
+    assert_eq!(
+        out.aggregation_value_column("DatasketchesKLL"),
+        Some("cpu_usage".to_string())
+    );
+    let mut rollup = out.aggregation_labels("DatasketchesKLL", "rollup");
+    rollup.sort();
+    assert_eq!(rollup, vec!["hostname".to_string(), "region".to_string()]);
+    assert_eq!(
+        out.aggregation_labels("DatasketchesKLL", "grouping"),
+        vec!["datacenter".to_string()]
+    );
+    assert_eq!(
+        out.aggregation_labels("DatasketchesKLL", "aggregated"),
+        Vec::<String>::new()
+    );
+    assert_eq!(out.inference_cleanup_param(q), Some(1));
+}
+
+/// Strict operator policy at the planner layer: a `>` lower bound / `<=` upper
+/// bound combination is not a recognized half-open range, so the time clause
+/// fails to parse and the controller surfaces a `SqlParse` error rather than
+/// generating a plan with the wrong (silently half-open) window.
+#[test]
+fn half_open_gt_lte_returns_sql_parse_error() {
+    let q = "SELECT quantile(0.95)(cpu_usage) FROM metrics_table WHERE time > DATEADD(s, -300, NOW()) AND time <= NOW() GROUP BY datacenter";
+    let result = SQLController::from_yaml(&one_query_config(q, 300), sql_opts())
+        .unwrap()
+        .generate();
+    assert!(matches!(result, Err(ControllerError::SqlParse(_))));
+}
+
 // ── Elastic SQL syntax variants ───────────────────────────────────────────────
 //
 // These three tests exercise syntactic forms used by Elastic: