Every branch on QueryPatternType in promql.rs (calculate_start_timestamp_promql, extract_quantile_param_promql, extract_topk_param, build_query_kwargs_promql, the label match in build_promql_execution_context_tail) is fully derivable from which tokens are present on PromQLMatchResult (a "function"/"function_args" token vs an "aggregation" token). The enum adds no information beyond what the match result already encodes, but is threaded through 5+ helper method signatures.
Resolution
Scope grew during implementation: rather than inlining token checks at each call site, asap_types::query_requirements::QueryRequirements (metric, statistics, data_range_ms, grouping_labels, spatial_filter_normalized, topk_count_events) became the single canonical derivation — computed once via build_query_requirements_promql immediately after pattern matching, and consumed identically by both asap-query-engine's main path and capability-matching fallback, and by both of asap-planner-rs's independent code paths (SingleQueryProcessor and the AQE extractor). This mirrors how SQL already normalizes every query shape into one struct (SQLQueryData) with zero downstream shape-branching.
Landed as 6 dependency-ordered stages:
- Stage 0 (correctness fix, independent of the rest): a real bug was found — non-collapsable spatial-of-temporal combinations (e.g.
sum(min_over_time(x[5m])); only sum+sum_over_time, sum+count_over_time, min+min_over_time, max+max_over_time are actually collapsable) were structurally accepted by both crates' pattern matchers but silently dropped the outer aggregation, returning wrong (ungrouped) data instead of erroring. Fixed by narrowing the pattern definitions to only the 4 actually-collapsable pairs, so such combinations no longer match any pattern at all.
- Stage 1: added
Statistic::is_approximate(), mirroring the existing PromQLFunction/AggregationOperator methods.
- Stage 2:
promql_utilities::get_statistics_to_compute dropped its pattern_type param.
- Stage 3:
asap_types::build_query_requirements_promql dropped its pattern_type param.
- Stage 4:
asap-query-engine unified onto QueryRequirements; controller_patterns flattened from HashMap<QueryPatternType, _> to Vec<PromQLPattern>; end-timestamp alignment made unconditional (matching SQL's align_end_timestamp_sql exactly, eliminating another shape-dependent branch); the standalone test_offline_precomputes debug binary (no real query/tokens) got its own local, unexported pattern-kind enum.
- Stage 5:
asap-planner-rs unified the same way. Along the way: found and unified a pre-existing divergence between planner and query-engine on modifier-less topk grouping labels; adopted SQL's window-sizing invariant (t_repeat_ms >= data_ingestion_interval_ms, and for genuinely multi-interval queries data_range_ms >= t_repeat_ms) in both set_window_parameters (PromQL) and compute_sql_window (SQL) for consistency — relaxed for single-scrape-interval queries (spatial-only, or a coincidentally narrow temporal query) so an ordinary "refresh less often than the scrape interval" configuration isn't wrongly rejected; get_cleanup_param simplified to take data_range_ms directly; build_patterns() flattened. should_be_performant's OnlyTemporal-only punting gate and quantile/topk kwarg extraction deliberately kept as raw-token checks rather than forced onto QueryRequirements, since collapsing them would have silently changed behavior.
- Stage 6: deleted
QueryPatternType entirely from promql_utilities::query_logics::enums (definition + Display impl) — zero remaining references anywhere in the Rust workspace.
All 6 stages landed independently compiling, tests passing, and clippy-clean (cargo clippy --workspace --all-targets --all-features --locked -- -D warnings, matching CI).
Every branch on
QueryPatternTypein promql.rs (calculate_start_timestamp_promql,extract_quantile_param_promql,extract_topk_param,build_query_kwargs_promql, the label match inbuild_promql_execution_context_tail) is fully derivable from which tokens are present onPromQLMatchResult(a"function"/"function_args"token vs an"aggregation"token). The enum adds no information beyond what the match result already encodes, but is threaded through 5+ helper method signatures.Resolution
Scope grew during implementation: rather than inlining token checks at each call site,
asap_types::query_requirements::QueryRequirements(metric, statistics, data_range_ms, grouping_labels, spatial_filter_normalized, topk_count_events) became the single canonical derivation — computed once viabuild_query_requirements_promqlimmediately after pattern matching, and consumed identically by bothasap-query-engine's main path and capability-matching fallback, and by both ofasap-planner-rs's independent code paths (SingleQueryProcessorand the AQE extractor). This mirrors how SQL already normalizes every query shape into one struct (SQLQueryData) with zero downstream shape-branching.Landed as 6 dependency-ordered stages:
sum(min_over_time(x[5m])); onlysum+sum_over_time,sum+count_over_time,min+min_over_time,max+max_over_timeare actually collapsable) were structurally accepted by both crates' pattern matchers but silently dropped the outer aggregation, returning wrong (ungrouped) data instead of erroring. Fixed by narrowing the pattern definitions to only the 4 actually-collapsable pairs, so such combinations no longer match any pattern at all.Statistic::is_approximate(), mirroring the existingPromQLFunction/AggregationOperatormethods.promql_utilities::get_statistics_to_computedropped itspattern_typeparam.asap_types::build_query_requirements_promqldropped itspattern_typeparam.asap-query-engineunified ontoQueryRequirements;controller_patternsflattened fromHashMap<QueryPatternType, _>toVec<PromQLPattern>; end-timestamp alignment made unconditional (matching SQL'salign_end_timestamp_sqlexactly, eliminating another shape-dependent branch); the standalonetest_offline_precomputesdebug binary (no real query/tokens) got its own local, unexported pattern-kind enum.asap-planner-rsunified the same way. Along the way: found and unified a pre-existing divergence between planner and query-engine on modifier-lesstopkgrouping labels; adopted SQL's window-sizing invariant (t_repeat_ms >= data_ingestion_interval_ms, and for genuinely multi-interval queriesdata_range_ms >= t_repeat_ms) in bothset_window_parameters(PromQL) andcompute_sql_window(SQL) for consistency — relaxed for single-scrape-interval queries (spatial-only, or a coincidentally narrow temporal query) so an ordinary "refresh less often than the scrape interval" configuration isn't wrongly rejected;get_cleanup_paramsimplified to takedata_range_msdirectly;build_patterns()flattened.should_be_performant'sOnlyTemporal-only punting gate and quantile/topk kwarg extraction deliberately kept as raw-token checks rather than forced ontoQueryRequirements, since collapsing them would have silently changed behavior.QueryPatternTypeentirely frompromql_utilities::query_logics::enums(definition +Displayimpl) — zero remaining references anywhere in the Rust workspace.All 6 stages landed independently compiling, tests passing, and clippy-clean (
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings, matching CI).