Skip to content

Commit 2d5f016

Browse files
authored
feat: feature-gate sqllogictests datafusion-substrait behind optional 'substrait' feature (apache#21268)
## Which issue does this PR close? - Closes apache#21269 ## Rationale for this change `datafusion-substrait` is a heavyweight dependency that most developers and users don't need. Making it optional saves compile time for the common case. ## What changes are included in this PR? - `datafusion-substrait` is now an optional dependency in `datafusion-sqllogictest/Cargo.toml` - New `substrait` feature flag gates the dependency - `#[cfg(feature = "substrait")]` gates the module, imports, and `run_test_file_substrait_round_trip` function - No-op fallback function returns an error when the feature is disabled - CI workflow updated to pass `--features substrait` for the round-trip test ## Are these changes tested? - Compiles without `substrait` feature (default) - Compiles with `--features substrait` - CI substrait round-trip test updated to enable the feature ## Are there any user-facing changes? No — this only affects the sqllogictest binary, not the datafusion library itself. The substrait round-trip test requires `--features substrait` to run.
1 parent 1e68674 commit 2d5f016

File tree

5 files changed

+28
-8
lines changed

5 files changed

+28
-8
lines changed

.github/workflows/rust.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ jobs:
305305
--lib \
306306
--tests \
307307
--bins \
308-
--features serde,avro,json,backtrace,integration-tests,parquet_encryption
308+
--features serde,avro,json,backtrace,integration-tests,parquet_encryption,substrait
309309
- name: Verify Working Directory Clean
310310
run: git diff --exit-code
311311
# Check no temporary directories created during test.
@@ -473,7 +473,7 @@ jobs:
473473
export RUST_MIN_STACK=20971520
474474
export TPCH_DATA=`realpath datafusion/sqllogictest/test_files/tpch/data`
475475
cargo test plan_q --package datafusion-benchmarks --profile ci --features=ci -- --test-threads=1
476-
INCLUDE_TPCH=true cargo test --features backtrace,parquet_encryption --profile ci --package datafusion-sqllogictest --test sqllogictests
476+
INCLUDE_TPCH=true cargo test --features backtrace,parquet_encryption,substrait --profile ci --package datafusion-sqllogictest --test sqllogictests
477477
- name: Verify Working Directory Clean
478478
run: git diff --exit-code
479479

@@ -537,7 +537,7 @@ jobs:
537537
# command cannot be run for all the .slt files. Run it for just one that works (limit.slt)
538538
# until most of the tickets in https://github.com/apache/datafusion/issues/16248 are addressed
539539
# and this command can be run without filters.
540-
run: cargo test --test sqllogictests -- --substrait-round-trip limit.slt
540+
run: cargo test -p datafusion-sqllogictest --test sqllogictests --features substrait -- --substrait-round-trip limit.slt
541541

542542
# Temporarily commenting out the Windows flow, the reason is enormously slow running build
543543
# Waiting for new Windows 2025 github runner
@@ -570,7 +570,7 @@ jobs:
570570
uses: ./.github/actions/setup-macos-aarch64-builder
571571
- name: Run tests (excluding doctests)
572572
shell: bash
573-
run: cargo test --profile ci --exclude datafusion-cli --workspace --lib --tests --bins --features avro,json,backtrace,integration-tests
573+
run: cargo test --profile ci --exclude datafusion-cli --workspace --lib --tests --bins --features avro,json,backtrace,integration-tests,substrait
574574

575575
vendor:
576576
name: Verify Vendored Code

datafusion/sqllogictest/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ chrono = { workspace = true, optional = true }
4848
clap = { version = "4.5.60", features = ["derive", "env"] }
4949
datafusion = { workspace = true, default-features = true, features = ["avro"] }
5050
datafusion-spark = { workspace = true, features = ["core"] }
51-
datafusion-substrait = { workspace = true, default-features = true }
51+
datafusion-substrait = { workspace = true, default-features = true, optional = true }
5252
futures = { workspace = true }
5353
half = { workspace = true, default-features = true }
5454
indicatif = "0.18"
@@ -79,6 +79,7 @@ postgres = [
7979
parquet_encryption = [
8080
"datafusion/parquet_encryption",
8181
]
82+
substrait = ["datafusion-substrait"]
8283

8384
[dev-dependencies]
8485
env_logger = { workspace = true }

datafusion/sqllogictest/bin/sqllogictests.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ use clap::{ColorChoice, Parser};
1919
use datafusion::common::instant::Instant;
2020
use datafusion::common::utils::get_available_parallelism;
2121
use datafusion::common::{DataFusionError, Result, exec_datafusion_err, exec_err};
22+
#[cfg(feature = "substrait")]
23+
use datafusion_sqllogictest::DataFusionSubstraitRoundTrip;
2224
use datafusion_sqllogictest::TestFile;
2325
use datafusion_sqllogictest::{
24-
CurrentlyExecutingSqlTracker, DataFusion, DataFusionSubstraitRoundTrip, Filter,
25-
TestContext, df_value_validator, read_dir_recursive, setup_scratch_dir,
26-
should_skip_file, should_skip_record, value_normalizer,
26+
CurrentlyExecutingSqlTracker, DataFusion, Filter, TestContext, df_value_validator,
27+
read_dir_recursive, setup_scratch_dir, should_skip_file, should_skip_record,
28+
value_normalizer,
2729
};
2830
use futures::stream::StreamExt;
2931
use indicatif::{
@@ -426,6 +428,7 @@ fn is_env_truthy(name: &str) -> bool {
426428
})
427429
}
428430

431+
#[cfg(feature = "substrait")]
429432
async fn run_test_file_substrait_round_trip(
430433
test_file: TestFile,
431434
validator: Validator,
@@ -468,6 +471,19 @@ async fn run_test_file_substrait_round_trip(
468471
res
469472
}
470473

474+
#[cfg(not(feature = "substrait"))]
475+
async fn run_test_file_substrait_round_trip(
476+
_test_file: TestFile,
477+
_validator: Validator,
478+
_mp: MultiProgress,
479+
_mp_style: ProgressStyle,
480+
_filters: &[Filter],
481+
_currently_executing_sql_tracker: CurrentlyExecutingSqlTracker,
482+
_colored_output: bool,
483+
) -> Result<()> {
484+
exec_err!("Cannot run substrait round-trip: the 'substrait' feature is not enabled")
485+
}
486+
471487
async fn run_test_file(
472488
test_file: TestFile,
473489
validator: Validator,

datafusion/sqllogictest/src/engines/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919
mod conversion;
2020
mod currently_executed_sql;
2121
mod datafusion_engine;
22+
#[cfg(feature = "substrait")]
2223
mod datafusion_substrait_roundtrip_engine;
2324
mod output;
2425

2526
pub use datafusion_engine::DFSqlLogicTestError;
2627
pub use datafusion_engine::DataFusion;
2728
pub use datafusion_engine::convert_batches;
2829
pub use datafusion_engine::convert_schema_to_types;
30+
#[cfg(feature = "substrait")]
2931
pub use datafusion_substrait_roundtrip_engine::DataFusionSubstraitRoundTrip;
3032
pub use output::DFColumnType;
3133
pub use output::DFOutput;

datafusion/sqllogictest/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ pub use engines::DFColumnType;
3434
pub use engines::DFOutput;
3535
pub use engines::DFSqlLogicTestError;
3636
pub use engines::DataFusion;
37+
#[cfg(feature = "substrait")]
3738
pub use engines::DataFusionSubstraitRoundTrip;
3839
pub use engines::convert_batches;
3940
pub use engines::convert_schema_to_types;

0 commit comments

Comments
 (0)