refactor: remove asap-planner's promql/sql/metricsql features (#1065)

.github/workflows/rust.yml

@@ -108,12 +108,6 @@ jobs:
     - name: Run the DAG viewer's Node tests
       run: node --test tools/dag-viewer/stages.test.mjs tools/dag-viewer/enumeration.test.mjs tools/dag-viewer/memo.test.mjs
 
-    # A PromQL-only runtime builds asap-planner without DataFusion or MetricsQL.
-    - name: Check and test asap-planner with the PromQL frontend alone
-      run: |
-        cargo check -p asap-planner --no-default-features --features promql --locked
-        cargo test -p asap-planner --no-default-features --features promql --locked
-
     - name: Verify MetricsQL frontend as a stable external dependency
       run: bash tools/verify_metricsql_external_consumer.sh
 

crates/integration-tests/Cargo.toml

@@ -17,7 +17,7 @@ asap-plan-selection = { path = "../plan-selection" }
 [dev-dependencies]
 asap-devtools = { path = "../devtools" }
 # The tests plan PromQL and SQL workloads through the facade.
-asap-planner = { path = "../planner", features = ["promql", "sql", "test-support"] }
+asap-planner = { path = "../planner", features = ["test-support"] }
 asap_sketchlib = { workspace = true }
 serde_json = "1"
 tokio = { version = "1", features = ["rt", "macros", "rt-multi-thread"] }

crates/planner/Cargo.toml

@@ -5,9 +5,8 @@ version = "0.1.0"
 edition = "2021"
 
 # The library facade (issue #429): one entry point that takes prepared input and
-# returns the selected DAG. By default it is the only crate that depends on
-# every frontend (see `[features]` below to drop some)
-# — before it, the sole facade re-exporting more than one was `asap-devtools`,
+# returns the selected DAG. It is the only crate that depends on every
+# frontend — before it, the sole facade re-exporting more than one was `asap-devtools`,
 # a developer-tools crate. Everything below the frontend boundary lives in the
 # stage crates (`asap-logical-asap-optimization`, `asap-physical-asap-optimization`,
 # `asap-plan-selection`), so a third party writing an optimization pass (issue
@@ -17,9 +16,9 @@ edition = "2021"
 asap-types = { path = "../types" }
 asap-cost = { path = "../cost" }
 asap-plan-selection = { path = "../plan-selection" }
-asap-frontend-sql = { path = "../frontend-sql", optional = true }
-asap-frontend-promql = { path = "../frontend-promql", optional = true }
-asap-frontend-metricsql = { path = "../frontend-metricsql", optional = true }
+asap-frontend-sql = { path = "../frontend-sql" }
+asap-frontend-promql = { path = "../frontend-promql" }
+asap-frontend-metricsql = { path = "../frontend-metricsql" }
 asap-logical-asap-optimization = { path = "../logical-asap-optimization" }
 # The Planner's own executor: `plan_installable` offers only the summary
 # families it runs when the caller leaves them unrestricted.
@@ -28,16 +27,7 @@ serde = { version = "1", features = ["derive"] }
 serde_json = "1"
 thiserror = "2"
 
-# One feature per query-language frontend. A runtime that plans PromQL alone
-# builds with `default-features = false, features = ["promql"]` and links
-# neither DataFusion (the SQL frontend) nor the MetricsQL parser. A workload
-# whose language's feature is off fails validation with
-# `UserInputError::FrontendDisabled`.
 [features]
-default = ["promql", "sql", "metricsql"]
-promql = ["dep:asap-frontend-promql"]
-sql = ["dep:asap-frontend-sql"]
-metricsql = ["dep:asap-frontend-metricsql"]
 # `test_support::query_time_reference`, for a runtime's correctness tests.
 test-support = []
 

crates/planner/src/lib.rs

@@ -16,23 +16,15 @@
 //! new frontend, a deserialized plan, a test that does not want to build SQL
 //! and a catalog — skips this crate and calls
 //! [`asap_plan_selection::optimize`] directly.
-//!
-//! Each frontend is a cargo feature — `promql`, `sql` and `metricsql`, all on
-//! by default. A workload whose language's feature is off fails validation
-//! with [`UserInputError::FrontendDisabled`].
 
 use asap_types::workload::parsed_workload::{ParsedWorkload, ParsedWorkloadError};
-#[cfg(feature = "sql")]
 use asap_types::workload::SqlDialect;
 use asap_types::workload::{QueryLanguage, WorkloadError};
 
-#[cfg(feature = "metricsql")]
 use asap_frontend_metricsql::{lower_metricsql_query_workload, MetricsqlError};
-#[cfg(feature = "promql")]
 use asap_frontend_promql::{
     lower_promql_query_workload, lower_promql_query_workload_with_histograms, PromqlError,
 };
-#[cfg(feature = "sql")]
 use asap_frontend_sql::{lower_sql_dialect, SqlError};
 
 // The optimization stage's vocabulary is this facade's vocabulary too: a caller
@@ -83,9 +75,7 @@ pub use asap_cost::{
     MeasuredOperationCost, MeasuredOperationCosts, NodePriceSource, OperatorCost, OperatorKind,
     ResourceSpeed, ResourceUnitCost, SummaryOperation,
 };
-#[cfg(feature = "promql")]
 pub use asap_frontend_promql::{HistogramCatalog, HistogramKind};
-#[cfg(feature = "sql")]
 pub use asap_frontend_sql::SqlCatalog;
 pub use asap_logical_asap_optimization::{DeploymentCapabilities, SupportedSummary};
 pub use asap_plan_selection::{RejectionCause, SearchReport};
@@ -103,22 +93,16 @@ pub use asap_types::workload::PlanningWorkload;
 
 /// The lowering dependencies of one frontend. Which variant applies is fixed by
 /// `PlanningWorkload::query_workload.language`; [`UserInput::validate`] checks
-/// that they agree. Each variant exists only when its frontend's feature is on.
+/// that they agree.
 #[non_exhaustive]
 pub enum FrontendInput<'a> {
-    #[cfg(feature = "sql")]
-    Sql { catalog: &'a SqlCatalog },
-    #[cfg(feature = "promql")]
+    Sql {
+        catalog: &'a SqlCatalog,
+    },
     Promql {
         histograms: Option<HistogramCatalog>,
     },
-    #[cfg(feature = "metricsql")]
     Metricsql,
-    /// Keeps `'a` in use when the `sql` feature is off; it cannot be
-    /// constructed.
-    #[cfg(not(feature = "sql"))]
-    #[doc(hidden)]
-    NoSqlFeature(std::convert::Infallible, std::marker::PhantomData<&'a ()>),
 }
 
 #[non_exhaustive]
@@ -155,26 +139,14 @@ impl<'a> UserInput<'a> {
         self.workload.validate().map_err(UserInputError::Workload)?;
 
         let language = &self.workload.query_workload.language;
-        // The `FrontendInput` variant that lowers `language`, and its cargo
-        // feature with whether this build enables it.
-        let (frontend, feature) = match language {
-            QueryLanguage::SQL(_) | QueryLanguage::DataFusion => {
-                ("Sql", Some(("sql", cfg!(feature = "sql"))))
-            }
-            QueryLanguage::PromQL => ("Promql", Some(("promql", cfg!(feature = "promql")))),
-            QueryLanguage::MetricsQL => (
-                "Metricsql",
-                Some(("metricsql", cfg!(feature = "metricsql"))),
-            ),
+        // The `FrontendInput` variant that lowers `language`.
+        let frontend = match language {
+            QueryLanguage::SQL(_) | QueryLanguage::DataFusion => "Sql",
+            QueryLanguage::PromQL => "Promql",
+            QueryLanguage::MetricsQL => "Metricsql",
             // No frontend lowers it.
-            QueryLanguage::ElasticDSL => ("none", None),
+            QueryLanguage::ElasticDSL => "none",
         };
-        if let Some((feature, false)) = feature {
-            return Err(UserInputError::FrontendDisabled {
-                language: format!("{language:?}"),
-                feature,
-            });
-        }
         if self.frontend_specific.name() != frontend {
             return Err(UserInputError::FrontendMismatch {
                 language: format!("{language:?}"),
@@ -189,14 +161,9 @@ impl<'a> UserInput<'a> {
 impl FrontendInput<'_> {
     fn name(&self) -> &'static str {
         match self {
-            #[cfg(feature = "sql")]
             Self::Sql { .. } => "Sql",
-            #[cfg(feature = "promql")]
             Self::Promql { .. } => "Promql",
-            #[cfg(feature = "metricsql")]
             Self::Metricsql => "Metricsql",
-            #[cfg(not(feature = "sql"))]
-            Self::NoSqlFeature(never, _) => match *never {},
         }
     }
 }
@@ -213,29 +180,15 @@ pub enum UserInputError {
         language: String,
         frontend: &'static str,
     },
-    /// The workload's language needs a frontend this build leaves out: the
-    /// caller must enable `asap-planner`'s cargo feature `feature`. Bad
-    /// input for this build, not an internal bug.
-    #[error(
-        "workload language {language} needs asap-planner's `{feature}` cargo feature, \
-         which this build does not enable"
-    )]
-    FrontendDisabled {
-        language: String,
-        feature: &'static str,
-    },
 }
 
 #[derive(Debug, thiserror::Error)]
 #[non_exhaustive]
 pub enum LoweringError {
-    #[cfg(feature = "sql")]
     #[error(transparent)]
     Sql(SqlError),
-    #[cfg(feature = "promql")]
     #[error(transparent)]
     Promql(PromqlError),
-    #[cfg(feature = "metricsql")]
     #[error(transparent)]
     Metricsql(MetricsqlError),
 }
@@ -270,8 +223,7 @@ pub enum PlanError {
 /// optimization pass over the result: by default [`Stage1ToStage3Pass`], the
 /// planning stages in order.
 ///
-/// Async because the SQL frontend plans through DataFusion; it stays async
-/// when the `sql` feature is off, so callers see one signature. One failed query
+/// Async because the SQL frontend plans through DataFusion. One failed query
 /// fails the whole call: the output is positionally aligned with
 /// `QueryWorkload::entries()`, and a partial result would silently break that.
 pub async fn e2e_plan(input: UserInput<'_>) -> Result<PlanOutput, PlanError> {
@@ -294,7 +246,6 @@ pub async fn e2e_plan(input: UserInput<'_>) -> Result<PlanOutput, PlanError> {
 /// every repeating query, and the output must cover every entry.
 async fn lower(input: &UserInput<'_>) -> Result<Vec<asap_types::ir::QueryRoot>, PlanError> {
     match &input.frontend_specific {
-        #[cfg(feature = "sql")]
         FrontendInput::Sql { catalog } => {
             let dialect = match &input.workload.query_workload.language {
                 QueryLanguage::SQL(dialect) => dialect.clone(),
@@ -318,7 +269,6 @@ async fn lower(input: &UserInput<'_>) -> Result<Vec<asap_types::ir::QueryRoot>,
             }
             Ok(lowered)
         }
-        #[cfg(feature = "promql")]
         FrontendInput::Promql { histograms, .. } => {
             let lowered = match histograms {
                 Some(histograms) => {
@@ -332,14 +282,11 @@ async fn lower(input: &UserInput<'_>) -> Result<Vec<asap_types::ir::QueryRoot>,
             })?;
             Ok(lowered)
         }
-        #[cfg(feature = "metricsql")]
         FrontendInput::Metricsql => {
             lower_metricsql_query_workload(input.workload).map_err(|source| PlanError::Lowering {
                 entry_index: None,
                 source: LoweringError::Metricsql(source),
             })
         }
-        #[cfg(not(feature = "sql"))]
-        FrontendInput::NoSqlFeature(never, _) => match *never {},
     }
 }

crates/planner/tests/e2e_plan.rs

@@ -1,20 +1,16 @@
 //! End-to-end coverage for the library facade (#429) and for plugging a
-//! different optimization pass into it (#430). The SQL tests need the `sql`
-//! feature; the PromQL-only build checks that a SQL workload names it instead.
+//! different optimization pass into it (#430).
 
 use std::rc::Rc;
 
-#[cfg(feature = "sql")]
 use asap_frontend_sql::{lower_sql_dialect, SqlCatalog};
 use asap_plan_selection::pass::PlanningModels;
-#[cfg(feature = "sql")]
 use asap_plan_selection::pass::{
     run_stage1_to_stage3, OptimizationInput, OptimizationPass, OptimizeError, PlanOutput,
     Stage1ToStage3Pass,
 };
 use asap_planner::{e2e_plan, FrontendInput, PlanError, UserInput, UserInputError};
 use asap_types::ir::properties::{AccuracyMetric, AccuracyTarget};
-#[cfg(feature = "sql")]
 use asap_types::ir::schema::{DataType, Field, Schema};
 use asap_types::workload::{
     BatchEntry, DataArrival, DataWorkload, DurationMs, PlanningWorkload, Predictability, Query,
@@ -51,7 +47,6 @@ fn batch(sql: &str) -> BatchEntry {
     }
 }
 
-#[cfg(feature = "sql")]
 fn lineitem_catalog() -> SqlCatalog {
     SqlCatalog::new().with_table(
         "lineitem",
@@ -90,7 +85,6 @@ fn sql_workload(
 /// The facade turns a prepared workload into one selected DAG per query, in
 /// `QueryWorkload::entries()` order, without the caller touching any stage's
 /// candidates.
-#[cfg(feature = "sql")]
 #[tokio::test]
 async fn plans_every_query_in_entry_order() {
     let workload = sql_workload(
@@ -114,7 +108,6 @@ async fn plans_every_query_in_entry_order() {
 
 /// The facade selects what the planning stages select over the frontends'
 /// roots ([`run_stage1_to_stage3`]), and reports that selection.
-#[cfg(feature = "sql")]
 #[tokio::test]
 async fn facade_plans_match_run_stage1_to_stage3() {
     let workload = sql_workload(
@@ -190,7 +183,6 @@ async fn facade_plans_match_run_stage1_to_stage3() {
 /// A repeating SQL query reaches the optimizer. `lower_sql_batch` walks
 /// `query_batch` alone, so driving the frontend through it would drop the
 /// repeating entries.
-#[cfg(feature = "sql")]
 #[tokio::test]
 async fn lowers_repeating_sql_entries_too() {
     let workload = sql_workload(
@@ -223,7 +215,6 @@ async fn lowers_repeating_sql_entries_too() {
 
 /// A caller-supplied pass replaces the shipped algorithm entirely: the trait is
 /// the whole optimization stage, not a rule inside it.
-#[cfg(feature = "sql")]
 #[tokio::test]
 async fn runs_a_caller_supplied_pass_instead_of_the_shipped_one() {
     struct CountingPass;
@@ -265,7 +256,6 @@ async fn runs_a_caller_supplied_pass_instead_of_the_shipped_one() {
 
 /// The harness catches a pass that mislabels which query a plan belongs to,
 /// rather than letting the mislabelled plan reach a deployment.
-#[cfg(feature = "sql")]
 #[tokio::test]
 async fn harness_rejects_a_pass_that_mislabels_entry_indices() {
     struct Mangling;
@@ -311,7 +301,6 @@ async fn harness_rejects_a_pass_that_mislabels_entry_indices() {
 
 /// A frontend input that cannot lower the workload's language fails before any
 /// query is parsed.
-#[cfg(feature = "sql")]
 #[tokio::test]
 async fn rejects_a_frontend_that_does_not_match_the_workload_language() {
     let workload = sql_workload(
@@ -331,32 +320,6 @@ async fn rejects_a_frontend_that_does_not_match_the_workload_language() {
     ));
 }
 
-/// Without the `sql` feature a SQL workload fails validation, naming the
-/// feature to enable, before any frontend runs.
-#[cfg(not(feature = "sql"))]
-#[tokio::test]
-async fn a_sql_workload_without_the_sql_feature_names_the_missing_feature() {
-    let workload = sql_workload(
-        vec![batch("SELECT COUNT(DISTINCT l_orderkey) FROM lineitem")],
-        None,
-    );
-    let input = UserInput::new(
-        &workload,
-        FrontendInput::Promql { histograms: None },
-        PlanningModels::builtin(),
-    );
-
-    let err = e2e_plan(input).await.unwrap_err();
-    assert!(
-        matches!(
-            err,
-            PlanError::Input(UserInputError::FrontendDisabled { feature: "sql", .. })
-        ),
-        "{err:?}"
-    );
-    assert!(err.to_string().contains("`sql` cargo feature"), "{err}");
-}
-
 /// A repeating PromQL query yields one plan carrying its selected DAG root.
 #[tokio::test]
 async fn each_plan_carries_its_selected_root() {

crates/planner/tests/sql_per_group_topk.rs

@@ -5,7 +5,6 @@
 //! rows PromQL `topk by (city)` answers over the same data, and the plan
 //! Stage 3 selects under an approximate target installs (each materialized
 //! view it keeps refreshes).
-#![cfg(all(feature = "sql", feature = "promql"))]
 
 use std::collections::{BTreeMap, BTreeSet};
 use std::sync::Arc;

crates/planner/tests/sql_table_scans.rs

@@ -6,7 +6,6 @@
 //! sketch or exactly. A plan that keeps panes at ingestion time runs as a
 //! runtime runs it: each pane built in an ingestion run over its window from
 //! the table bound at the plan's `Scan`, the queries answered from the panes.
-#![cfg(feature = "sql")]
 
 use std::collections::BTreeMap;
 use std::sync::Arc;