Skip to content

refactor: slowly phase out QueryPatternType and QueryType enums #494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
milindsrivastava1997 wants to merge 4 commits into from
Closed

refactor: slowly phase out QueryPatternType and QueryType enums #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99bfac0
refactor: start_timestamp logic for OnlySpatial
milindsrivastava1997 Jul 1, 2026
786e037
updated start/end timestamp validation logic
milindsrivastava1997 Jul 1, 2026
0589abf
removed special build_spatiotemporal_context function
milindsrivastava1997 Jul 1, 2026
cca0d6f
removed QueryType::Spatial, replaced by either TemporalGeneric or Spa…
milindsrivastava1997 Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 31 additions & 31 deletions asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlparser_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::TemporalGeneric]);
}
}

Expand Down Expand Up @@ -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::TemporalGeneric],
None,
);
}
Expand All @@ -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,
);
}
Expand All @@ -145,7 +145,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand Down Expand Up @@ -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,
);
}
Expand All @@ -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,
);
}
Expand All @@ -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,
);
}
Expand All @@ -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::TemporalGeneric],
None,
);
}
Expand All @@ -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,
);
}
Expand All @@ -250,7 +250,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -259,7 +259,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -268,7 +268,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -277,7 +277,7 @@ 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],
vec![QueryType::TemporalGeneric, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -286,7 +286,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -295,7 +295,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -304,7 +304,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -313,7 +313,7 @@ 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],
vec![QueryType::TemporalGeneric, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -322,7 +322,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -331,7 +331,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -340,7 +340,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -349,7 +349,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -358,7 +358,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalQuantile],
None,
);
}
Expand All @@ -367,7 +367,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalQuantile],
None,
);
}
Expand Down Expand Up @@ -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,
);
}
Expand All @@ -452,7 +452,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand All @@ -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,
);
}
Expand All @@ -491,7 +491,7 @@ 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],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand Down Expand Up @@ -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,
);
}
Expand Down Expand Up @@ -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::TemporalGeneric],
None,
);
}
Expand Down Expand Up @@ -792,7 +792,7 @@ mod tests {
GROUP BY L1, L2, L3, L4) \
GROUP BY L1",
15.0,
vec![QueryType::Spatial, QueryType::TemporalGeneric],
vec![QueryType::SpatioTemporal, QueryType::TemporalGeneric],
None,
);
}
Expand Down Expand Up @@ -1304,7 +1304,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,
);
}
Expand Down
100 changes: 37 additions & 63 deletions asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ use std::collections::HashSet;

#[derive(Debug, Clone, PartialEq)]
pub enum QueryType {
/// Dead: `query_info_to_pattern` no longer constructs this variant (#487). A query
/// spanning exactly one scrape interval used to be forced into `Spatial` regardless
/// of its GROUP BY, distinct from `Temporal*`/`SpatioTemporal`, which checked labels.
/// That was never a real structural difference — `Spatial` was just `Temporal` with
/// `range == scrape_interval` — so it's now classified by the same label-coverage
/// check as everything else and lands in `TemporalGeneric`/`TemporalQuantile`
/// (full GROUP BY) or `SpatioTemporal` (partial GROUP BY) like any other duration.
/// Kept as a variant only because downstream `sql.rs` still matches on it; deleted
/// once that's migrated to structural checks.
Spatial,
TemporalGeneric,
TemporalQuantile,
Expand Down Expand Up @@ -245,70 +254,35 @@ impl SQLPatternMatcher {

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 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
sql_query.add_subquery(
QueryType::Spatial,
aggregation_info.clone(),
metric.clone(),
labels.clone(),
time_info.clone(),
);
// Every subquery — outer or inner, whatever its duration — is classified purely
// by aggregation + label coverage. Duration only gates validity above
// (SpatialDurationSmall); it no longer decides the shape. A query that covers
// all metadata columns is a plain temporal read (Quantile/Generic); anything
// grouping by a subset of labels is SpatioTemporal, regardless of how long a
// range it spans — a 1-interval query with a partial GROUP BY is exactly as
// "spatiotemporal" as a 10-interval one.
for (metric, aggregation_info, _scraped_intervals, labels, time_info) in query_data.iter() {
let has_all_labels = self
.schema
.get_metadata_columns(metric)
.map(|schema_metadata_columns| labels == schema_metadata_columns)
.unwrap_or(true);

let query_type = if !has_all_labels {
QueryType::SpatioTemporal
} else if aggregation_info.get_name() == "QUANTILE" {
QueryType::TemporalQuantile
} 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 {
// 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(),
);
}
}
}
QueryType::TemporalGeneric
};

sql_query.add_subquery(
query_type,
aggregation_info.clone(),
metric.clone(),
labels.clone(),
time_info.clone(),
);
}

sql_query
Expand Down
Loading
Loading