You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Refactor] Remove the unused promql/sql/metricsql frontend features of asap-planner #1065
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.
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.
Activity
milindsrivastava1997 commentedon Oct 29, 2026
Findings from checking the issue against the code:
Cargo.tomlsetsasap-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 dropdefault-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.crates/planner/src/lib.rshas 25#[cfg(feature = ...)]sites, not about 18.crates/planner/tests/sql_per_group_topk.rsis also gated, with a crate-level#![cfg(all(feature = "sql", feature = "promql"))].promqlfeature already does nothing:asap-executorandasap-plan-selectiondepend onasap-frontend-promqlunconditionally.[features]and facade comments incrates/planner/Cargo.toml, and the module doc atcrates/planner/src/lib.rs:22.revert: leave the PromQL-only step and planner features to #1065
ci: cache dependencies per job with rust-cache (#1071)
refactor: remove asap-planner's promql/sql/metricsql features (#1065)
fix(deps): depend on all of asap-planner's frontends (#850)
refactor: remove asap-planner's promql/sql/metricsql features (#1065)
refactor: remove asap-planner's promql/sql/metricsql features (#1065) (…