Summary
The temporal-vs-spatial distinction is encoded as two separate enums (QueryPatternType in promql_utilities, QueryType::Spatial in the SQL planner) that don't correspond to anything in SQL or Elastic, where a time range is always just a time range. Code branches on these enums in 8+ places across promql_utilities, asap-planner-rs, and asap-query-engine, duplicating logic that could instead be derived structurally from the parsed query.
Problems this causes
get_statistics_to_compute, get_spatial_aggregation_output_labels, and others take QueryPatternType as a parameter purely to re-derive information already present in the parsed match result (e.g. whether a range duration or aggregation op is present).controller_patterns: HashMap<QueryPatternType, Vec<PromQLPattern>> in asap-query-engine/src/engines/simple_engine/mod.rs:140 iterates a HashMap keyed by the enum — iteration order is non-deterministic, and which pattern matches first can depend on that order.- Every new query shape needs the enum-branching logic updated in multiple places (
promql.rs, sql.rs, mod.rs in the query engine, plus the planner side) instead of one place.
Evidence
promql_utilities/src/query_logics/enums.rs — QueryPatternType definitionasap-query-engine/src/engines/simple_engine/mod.rs:140 — HashMap-keyed pattern registryasap-query-engine/src/engines/simple_engine/promql.rs, .../sql.rs — enum match sitesasap-planner-rs/src/planner/window.rs, cleanup.rs — enum match sites in the planner
Dependency
Cleaning this up depends on #486 — time ranges need to be explicit before the enum can be safely removed; otherwise the implicit-default logic the enum currently encodes has nowhere to go.
Summary
The temporal-vs-spatial distinction is encoded as two separate enums (
QueryPatternTypeinpromql_utilities,QueryType::Spatialin the SQL planner) that don't correspond to anything in SQL or Elastic, where a time range is always just a time range. Code branches on these enums in 8+ places acrosspromql_utilities,asap-planner-rs, andasap-query-engine, duplicating logic that could instead be derived structurally from the parsed query.Problems this causes
get_statistics_to_compute,get_spatial_aggregation_output_labels, and others takeQueryPatternTypeas a parameter purely to re-derive information already present in the parsed match result (e.g. whether a range duration or aggregation op is present).controller_patterns: HashMap<QueryPatternType, Vec<PromQLPattern>>inasap-query-engine/src/engines/simple_engine/mod.rs:140iterates aHashMapkeyed by the enum — iteration order is non-deterministic, and which pattern matches first can depend on that order.promql.rs,sql.rs,mod.rsin the query engine, plus the planner side) instead of one place.Evidence
promql_utilities/src/query_logics/enums.rs—QueryPatternTypedefinitionasap-query-engine/src/engines/simple_engine/mod.rs:140— HashMap-keyed pattern registryasap-query-engine/src/engines/simple_engine/promql.rs,.../sql.rs— enum match sitesasap-planner-rs/src/planner/window.rs,cleanup.rs— enum match sites in the plannerDependency
Cleaning this up depends on #486 — time ranges need to be explicit before the enum can be safely removed; otherwise the implicit-default logic the enum currently encodes has nowhere to go.