This is an automated email from the ASF dual-hosted git repository.

andygrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-ballista.git


The following commit(s) were added to refs/heads/main by this push:
     new 65dc2dfb4 feat(benchmarks): add --skip to omit queries from a tpch 
suite run (#2129)
65dc2dfb4 is described below

commit 65dc2dfb4bb540bfc5ffb40e69304ce384e48caf
Author: Andy Grove <[email protected]>
AuthorDate: Tue Jul 21 15:48:51 2026 -0600

    feat(benchmarks): add --skip to omit queries from a tpch suite run (#2129)
    
    Running the whole suite minus one query previously meant driving 21
    single-query jobs, which changes the measurement: each job starts a fresh
    process against a cold cache, so the numbers are not comparable to a
    continuous suite run.
    
    `--skip N` (repeatable) drops queries from the 1..=22 suite while keeping 
it a
    single run. An explicit `--query` still wins, so asking for a query directly
    always runs it. Skip values outside 1..=22, and a skip set that would 
exclude
    every query, are rejected rather than silently ignored.
    
    Both the DataFusion and Ballista paths now share one `select_queries` helper
    instead of duplicating the selection logic.
---
 benchmarks/src/bin/tpch.rs | 121 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 113 insertions(+), 8 deletions(-)

diff --git a/benchmarks/src/bin/tpch.rs b/benchmarks/src/bin/tpch.rs
index 53e1fa0f0..2f20804d7 100644
--- a/benchmarks/src/bin/tpch.rs
+++ b/benchmarks/src/bin/tpch.rs
@@ -80,6 +80,12 @@ struct BallistaBenchmarkOpt {
     #[structopt(short, long)]
     query: Option<usize>,
 
+    /// Query numbers to skip when running the whole suite. Can be specified
+    /// multiple times, e.g. `--skip 18 --skip 21`. Ignored when `--query` is
+    /// given.
+    #[structopt(long = "skip", number_of_values = 1)]
+    skip: Vec<usize>,
+
     /// Activate debug mode to see query results
     #[structopt(short, long)]
     debug: bool,
@@ -144,6 +150,12 @@ struct DataFusionBenchmarkOpt {
     #[structopt(short, long)]
     query: Option<usize>,
 
+    /// Query numbers to skip when running the whole suite. Can be specified
+    /// multiple times, e.g. `--skip 18 --skip 21`. Ignored when `--query` is
+    /// given.
+    #[structopt(long = "skip", number_of_values = 1)]
+    skip: Vec<usize>,
+
     /// Activate debug mode to see query results
     #[structopt(short, long)]
     debug: bool,
@@ -312,6 +324,48 @@ async fn main() -> Result<()> {
     }
 }
 
+/// Resolves which TPC-H queries a benchmark run should execute.
+///
+/// A `--query` selects exactly that one and `--skip` is ignored, so asking 
for a
+/// query explicitly always runs it. Otherwise the full 1..=22 suite runs minus
+/// any `--skip` entries. Skip values outside 1..=22 are reported rather than
+/// silently ignored, since a typo would otherwise look like it took effect.
+fn select_queries(query: Option<usize>, skip: &[usize]) -> Result<Vec<usize>> {
+    if let Some(q) = query {
+        return Ok(vec![q]);
+    }
+
+    if let Some(bad) = skip.iter().find(|q| !(1..=22).contains(*q)) {
+        return Err(DataFusionError::Execution(format!(
+            "--skip {bad} is not a TPC-H query number (expected 1-22)"
+        )));
+    }
+
+    let selected: Vec<usize> = (1..=22).filter(|q| 
!skip.contains(q)).collect();
+    if selected.is_empty() {
+        return Err(DataFusionError::Execution(
+            "--skip excluded every query; nothing to run".to_string(),
+        ));
+    }
+
+    if !skip.is_empty() {
+        let mut skipped: Vec<usize> = skip.to_vec();
+        skipped.sort_unstable();
+        skipped.dedup();
+        println!(
+            "Skipping {} of 22 queries: {}",
+            skipped.len(),
+            skipped
+                .iter()
+                .map(|q| q.to_string())
+                .collect::<Vec<_>>()
+                .join(", ")
+        );
+    }
+
+    Ok(selected)
+}
+
 #[allow(clippy::await_holding_lock)]
 async fn benchmark_datafusion(opt: DataFusionBenchmarkOpt) -> 
Result<Vec<RecordBatch>> {
     println!("Running benchmarks with the following options: {opt:?}");
@@ -357,10 +411,7 @@ async fn benchmark_datafusion(opt: DataFusionBenchmarkOpt) 
-> Result<Vec<RecordB
     }
 
     // Determine which queries to run
-    let query_numbers: Vec<usize> = opt
-        .query
-        .map(|q| vec![q])
-        .unwrap_or_else(|| (1..=22).collect());
+    let query_numbers = select_queries(opt.query, &opt.skip)?;
 
     let mut benchmark_run = BenchmarkRun::new();
     let mut result: Vec<RecordBatch> = Vec::with_capacity(1);
@@ -496,10 +547,7 @@ async fn benchmark_ballista(opt: BallistaBenchmarkOpt) -> 
Result<()> {
     );
 
     // Determine which queries to run
-    let query_numbers: Vec<usize> = opt
-        .query
-        .map(|q| vec![q])
-        .unwrap_or_else(|| (1..=22).collect());
+    let query_numbers = select_queries(opt.query, &opt.skip)?;
 
     let oracle_ctx = if opt.verify {
         let cfg = SessionConfig::new()
@@ -1627,6 +1675,62 @@ mod tests {
     use std::env;
     use std::sync::Arc;
 
+    #[test]
+    fn select_queries_runs_whole_suite_by_default() {
+        assert_eq!(
+            select_queries(None, &[]).unwrap(),
+            (1..=22).collect::<Vec<_>>()
+        );
+    }
+
+    // The motivating case: run the full suite but leave out the query that
+    // exhausts the memory pool, without having to drive 21 single-query jobs.
+    #[test]
+    fn select_queries_omits_skipped() {
+        let selected = select_queries(None, &[18]).unwrap();
+        assert_eq!(selected.len(), 21);
+        assert!(!selected.contains(&18));
+        assert!(selected.contains(&17) && selected.contains(&19));
+    }
+
+    #[test]
+    fn select_queries_omits_several_skipped() {
+        let selected = select_queries(None, &[18, 21, 5]).unwrap();
+        assert_eq!(selected.len(), 19);
+        for q in [5, 18, 21] {
+            assert!(!selected.contains(&q), "query {q} should have been 
skipped");
+        }
+    }
+
+    // An explicit --query is a direct instruction, so it wins over --skip 
rather
+    // than silently producing an empty run.
+    #[test]
+    fn select_queries_prefers_explicit_query_over_skip() {
+        assert_eq!(select_queries(Some(18), &[18]).unwrap(), vec![18]);
+    }
+
+    // A repeated skip must not change the outcome.
+    #[test]
+    fn select_queries_tolerates_duplicate_skips() {
+        assert_eq!(select_queries(None, &[18, 18]).unwrap().len(), 21);
+    }
+
+    // A typo like `--skip 42` would otherwise look like it took effect.
+    #[test]
+    fn select_queries_rejects_out_of_range_skip() {
+        for bad in [0usize, 23, 100] {
+            assert!(
+                select_queries(None, &[bad]).is_err(),
+                "--skip {bad} should be rejected"
+            );
+        }
+    }
+
+    #[test]
+    fn select_queries_rejects_skipping_everything() {
+        assert!(select_queries(None, &(1..=22).collect::<Vec<_>>()).is_err());
+    }
+
     fn run_with(queries: Vec<QueryRun>) -> BenchmarkRun {
         BenchmarkRun {
             benchmark_version: "test".to_string(),
@@ -2054,6 +2158,7 @@ mod tests {
             // run the query to compute actual results of the query
             let opt = DataFusionBenchmarkOpt {
                 query: Some(n),
+                skip: vec![],
                 debug: false,
                 iterations: 1,
                 partitions: 2,


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to