Skip to content

[Refactor] Remove the unused promql/sql/metricsql frontend features of asap-planner #1065

Closed

Description

@milindsrivastava1997

Example

asap-planner has three frontend features: promql, sql and metricsql, all on by default. They exist so that a runtime planning PromQL alone can build with default-features = false, features = ["promql"] and avoid linking DataFusion and the MetricsQL parser. If a workload's language is disabled, planning fails with UserInputError::FrontendDisabled.

Nothing actually builds that way:

  • No crate in the workspace disables them. integration-tests asks for promql + sql, and metricsql stays on through defaults.
  • The only consumer is the CI step rust.yml:143-144 (--no-default-features --features promql). Its test half has never exercised the PromQL-only build: the self dev-dependency asap-planner = { path = ".", features = ["test-support"] } turns default features back on (ci: the PromQL-only asap-planner test step still builds with sql and metricsql #1056).
  • The sql-only and metricsql-off combinations are never built.

Concrete maintenance problem

The features cost:

  • About 18 #[cfg(feature = ...)] sites in crates/planner/src/lib.rs
  • cfg-gated tests in crates/planner/tests/{e2e_plan,summary_sharing,sql_table_scans}.rs
  • The FrontendDisabled error variant
  • A CI step

They protect a build configuration that has no user.

Behavior that must remain unchanged

All three frontends are always available. Planning results are unchanged.

Before and after structure

Before: asap-frontend-{promql,sql,metricsql} are optional dependencies of asap-planner, behind the promql/sql/metricsql features.

After:

  • They become regular dependencies.
  • The promql/sql/metricsql features, their cfgs, UserInputError::FrontendDisabled and the not(sql) test (e2e_plan.rs:337) are removed.
  • The CI step "Check and test asap-planner with the PromQL frontend alone" is removed.
  • The test-support feature stays.

The fix for #1056 (unmerged on the ci-speed branch, commit 73eb80e) becomes unnecessary.

Why the proposed structure is minimally complex

The change only deletes code. If a PromQL-only runtime ever needs this, the features can be added back then.

Existing unit and end-to-end protection

  • cargo test --workspace
  • Corpus snapshots in crates/devtools/tests/corpus_snapshots/; these must stay unchanged.

Compatibility or migration impact

  • UserInputError::FrontendDisabled is removed from the public API.
  • External users that pass default-features = false or name these features would fail to resolve. ASAPQuery-backend uses asap_planner; confirm before merging that it does not set either.

Activity

  1. milindsrivastava1997 commented on Oct 29, 2026

    @milindsrivastava1997
    CollaboratorAuthor

    Findings from checking the issue against the code:

    • ASAPQuery-backend does use these features. Its Cargo.toml sets asap-planner = { ..., default-features = false, features = ["promql"] }, added in ASAPQuery-backend#843. Its dependency tree has no DataFusion and no MetricsQL parser today. The backend should support every language, so it needs to drop default-features = false, features = ["promql"]. That edit works against its current pinned rev, so it can land before this change. Otherwise its next rev bump will fail to resolve.
    • After that, the backend links DataFusion and the MetricsQL parser, so expect longer builds and a larger binary.
    • crates/planner/src/lib.rs has 25 #[cfg(feature = ...)] sites, not about 18.
    • crates/planner/tests/sql_per_group_topk.rs is also gated, with a crate-level #![cfg(all(feature = "sql", feature = "promql"))].
    • The promql feature already does nothing: asap-executor and asap-plan-selection depend on asap-frontend-promql unconditionally.
    • Comments that will go stale: the [features] and facade comments in crates/planner/Cargo.toml, and the module doc at crates/planner/src/lib.rs:22.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions