Repository navigation
promql.rs: decouple from QueryPatternType #508
Description
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])); 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. - Stage 1: added
Statistic::is_approximate(), mirroring the existingPromQLFunction/AggregationOperatormethods. - Stage 2:
promql_utilities::get_statistics_to_computedropped itspattern_typeparam. - Stage 3:
asap_types::build_query_requirements_promqldropped itspattern_typeparam. - Stage 4:
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. - Stage 5:
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. - Stage 6: deleted
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).
Activity