Skip to content

Tracking issue for refactoring/de-duplicating logic across PromQL/SQL/Elastic #489

Open

Description

@milindsrivastava1997

Tracking issue for de-duplication opportunities found across the PromQL, SQL, and Elastic DSL code paths in asap-planner-rs and asap-query-engine. Items are grouped by crate, ranked by impact.

asap-planner-rs

  • [HIGH] Controller boilerplate — promql/controller.rs, sql/controller.rs, elastic_dsl/controller.rs share an identical new / from_yaml / from_file / generate / generate_to_dir shape. generate_to_dir (create dir, serialize streaming + inference YAML, write both files) is byte-for-byte identical in all three. Extract into a shared free fn or trait default method.

  • [HIGH] Generator orchestration boilerplate — promql/generator.rs, sql/generator.rs, elastic_dsl/generator.rs each re-implement, near-verbatim, in their generate_*_plan functions:

    • cleanup-policy resolution (config.aggregate_cleanup.as_ref().and_then(|c| c.policy).unwrap_or(CleanupPolicy::ReadBased))
    • duplicate-query validation (HashSet + nested loop over qg.queries)
    • sequential ID assignment (dedup_map.keys().enumerate() loop)
    • cleanup_map YAML stanza construction in each build_*_inference_yaml
  • [MEDIUM] cleanup_param wrapper shell — planner/promql.rs, planner/sql.rs, planner/elastic_dsl.rs each wrap their cleanup-param computation in the identical if cleanup_policy == NoCleanup { None } else { Some(get_*_cleanup_param(...).map_err(...)?) }. Only the inner call differs.

asap-query-engine

  • [HIGH] Duplicated "build QueryExecutionContext" tail — engines/simple_engine/sql.rs:658-679, engines/simple_engine/promql.rs:317-339, and inline in engines/simple_engine/elastic.rs:71-92 all execute the same 6-statement sequence: clone streaming config, resolve grouping_labels/aggregated_labels with identical fallback semantics, construct QueryExecutionContext. SQL and PromQL already have private helpers for this; Elastic doesn't even have it functionized yet.
  • [HIGH] Duplicated "resolve aggregation: query_config first, else build requirements + capability match" skeleton — sql.rs:585-602, sql.rs:735-758, and promql.rs:1029-1059 share the same try/fallback control shape. Note: Elastic (elastic.rs:45) deliberately lacks the fallback — a real semantic gap worth its own look, not blind unification.
  • [MEDIUM] now_secs_f64()-shaped boilerplate — drivers/query/adapters/prometheus_http.rs:75-78, clickhouse_http.rs:39-42, elastic_http.rs:48-51 all copy-paste the same SystemTime::now().duration_since(UNIX_EPOCH)...as_secs_f64() snippet to default a missing time parameter.
  • [MEDIUM] Runtime-info merge logic — drivers/query/adapters/prometheus_http.rs:255-301 (handle_runtime_info) and elastic_http.rs:245-286 (handle_runtime_info_with_headers) duplicate the "fetch earliest-timestamp map, merge into fallback runtime data" logic. ClickHouse's version is just a stub, so this is 2-of-3, not 3-of-3.

Activity

  1. milindsrivastava1997 commented on Jul 13, 2026

    @milindsrivastava1997
    ContributorAuthor

    QueryRequirements isn't a shared vocabulary yet

    The config-hit path and the capability-matching fallback path each define "what a query needs" independently, so they can silently disagree. SQL's config-hit path (build_spatiotemporal_context) derives labels/statistic by hand instead of via QueryRequirements (#519). PromQL derives QueryRequirements unconditionally but only uses it for metadata — never to check that the config-path's resolved agg_info actually agrees with what capability-matching would pick.

    This is the same root cause as capability_matching.rs's rules being out of sync with what the planner emits (.design_docs/planner-engine-inconsistencies.md), and overlaps #519, #501, #267.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions