diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 62b8693b0..d45d197f5 100644 --- a/.github/workflows/rust.yml +++ b/.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 diff --git a/crates/integration-tests/Cargo.toml b/crates/integration-tests/Cargo.toml index a042ed033..6c4eef634 100644 --- a/crates/integration-tests/Cargo.toml +++ b/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"] } diff --git a/crates/planner/Cargo.toml b/crates/planner/Cargo.toml index 5c87dffc8..6ed4549da 100644 --- a/crates/planner/Cargo.toml +++ b/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 = [] diff --git a/crates/planner/src/lib.rs b/crates/planner/src/lib.rs index 40ce83aa5..25cf652c0 100644 --- a/crates/planner/src/lib.rs +++ b/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, }, - #[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 { @@ -294,7 +246,6 @@ pub async fn e2e_plan(input: UserInput<'_>) -> Result { /// every repeating query, and the output must cover every entry. async fn lower(input: &UserInput<'_>) -> Result, 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, } Ok(lowered) } - #[cfg(feature = "promql")] FrontendInput::Promql { histograms, .. } => { let lowered = match histograms { Some(histograms) => { @@ -332,14 +282,11 @@ async fn lower(input: &UserInput<'_>) -> Result, })?; 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 {}, } } diff --git a/crates/planner/tests/e2e_plan.rs b/crates/planner/tests/e2e_plan.rs index 90b64713e..f4e298668 100644 --- a/crates/planner/tests/e2e_plan.rs +++ b/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() { diff --git a/crates/planner/tests/sql_per_group_topk.rs b/crates/planner/tests/sql_per_group_topk.rs index 267c46980..dce6eff38 100644 --- a/crates/planner/tests/sql_per_group_topk.rs +++ b/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; diff --git a/crates/planner/tests/sql_table_scans.rs b/crates/planner/tests/sql_table_scans.rs index 9fb3bb678..e38220ffb 100644 --- a/crates/planner/tests/sql_table_scans.rs +++ b/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; diff --git a/crates/planner/tests/summary_sharing.rs b/crates/planner/tests/summary_sharing.rs index 960ba54fa..94e7ace47 100644 --- a/crates/planner/tests/summary_sharing.rs +++ b/crates/planner/tests/summary_sharing.rs @@ -9,17 +9,14 @@ use std::rc::Rc; use asap_accuracy::sizing::{default_size_params, DEFAULT_DELTA}; use asap_frontend_promql::{lower_promql_query_workload, promql_metric_names}; -#[cfg(feature = "sql")] use asap_frontend_sql::SqlCatalog; use asap_plan_selection::pass::{PlanOutput, PlanningModels, QueryPlan}; use asap_plan_selection::RejectionCause; use asap_planner::{e2e_plan, FrontendInput, UserInput}; use asap_types::ir::operator::agg_intent::default_quantile; use asap_types::ir::properties::{AccuracyMetric, AccuracyTarget}; -#[cfg(feature = "sql")] use asap_types::ir::schema::{DataType, Field, Schema}; use asap_types::ir::schema::{FieldDataType, SketchAlgorithm, SketchParams}; -#[cfg(feature = "sql")] use asap_types::workload::SqlDialect; use asap_types::workload::{ DataArrival, DataWorkload, DurationMs, PlanningWorkload, Predictability, Query, QueryLanguage, @@ -264,7 +261,6 @@ fn root_operators(root: &QueryRoot) -> Vec<&Rc> { } } -#[cfg(feature = "sql")] async fn plan_sql(queries: &[&str]) -> PlanOutput { let workload = PlanningWorkload { query_workload: QueryWorkload { @@ -508,7 +504,6 @@ async fn identical_ungrouped_queries_share_their_producers() { /// The SQL frontend reaches the same sharing for two copies of one filtered /// percentile. -#[cfg(feature = "sql")] #[tokio::test] async fn identical_sql_percentiles_share_one_producer() { let query = @@ -521,7 +516,6 @@ async fn identical_sql_percentiles_share_one_producer() { /// The quantile is a evaluation parameter: SQL p50 and p99 over one filtered /// column build one KLL, named after its input, while each query keeps its /// own output column. -#[cfg(feature = "sql")] #[tokio::test] #[ignore = "plan selection still selects an exact plan for at least one query: with the built-in cost model its query-time summary costs more than exact (#580)"] async fn sql_p50_and_p99_share_one_producer() {