bench(clickhouse): fix review issues and pin LTS version

- Pass subcommand arg to clickhouse-bench in run-sql-bench.sh for consistency
- Use BenchmarkArg + create_benchmark() in main.rs like other engines
- Replace `which` with `clickhouse local --version` for binary verification
- Pin ClickHouse to LTS release v25.8.18.1 from GitHub Releases

Signed-off-by: fastio <pengjian.uestc@gmail.com>

Author: fastio

.github/scripts/run-sql-bench.sh

@@ -132,7 +132,7 @@ fi
 # ClickHouse-bench only runs for local benchmarks (clickhouse-local reads local files).
 if ! $is_remote && [[ "$has_clickhouse" == "true" ]] && [[ -f "target/release_debug/clickhouse-bench" ]]; then
     # shellcheck disable=SC2086
-    target/release_debug/clickhouse-bench \
+    target/release_debug/clickhouse-bench "$subcommand" \
         -d gh-json \
         $opts \
         -o ch-results.json

.github/workflows/sql-benchmarks.yml

@@ -129,10 +129,13 @@ jobs:
 
       - name: Install ClickHouse
         if: contains(matrix.targets, 'clickhouse:')
+        env:
+          CLICKHOUSE_VERSION: "25.8.18.1"
         run: |
-          curl https://clickhouse.com/ | sh
-          sudo ./clickhouse install
-          echo "CLICKHOUSE_BINARY=$(which clickhouse)" >> $GITHUB_ENV
+          wget -qO- "https://github.com/ClickHouse/ClickHouse/releases/download/v${CLICKHOUSE_VERSION}-lts/clickhouse-common-static-${CLICKHOUSE_VERSION}-amd64.tgz" | tar xz
+          cp clickhouse-common-static-${CLICKHOUSE_VERSION}/usr/bin/clickhouse .
+          chmod +x clickhouse
+          echo "CLICKHOUSE_BINARY=$PWD/clickhouse" >> $GITHUB_ENV
 
       - name: Build binaries
         shell: bash

benchmarks/clickhouse-bench/src/lib.rs

@@ -70,7 +70,7 @@ impl ClickHouseClient {
     /// Check that the ClickHouse binary is available.
     ///
     /// For absolute paths, checks that the file exists on disk.
-    /// For bare names (e.g., `"clickhouse"`), tries to resolve via `$PATH` using `which`.
+    /// For bare names (e.g., `"clickhouse"`), tries to invoke it to verify it's resolvable.
     fn verify_binary(binary: &PathBuf) -> Result<()> {
         if binary.is_absolute() {
             anyhow::ensure!(
@@ -80,28 +80,31 @@ impl ClickHouseClient {
                  and ensure it is on $PATH.",
                 path = binary.display()
             );
-        } else {
-            // Try to find the binary on $PATH via `which`.
-            let output = Command::new("which")
-                .arg(binary.as_os_str())
-                .output()
-                .context("Failed to run `which` to locate clickhouse binary")?;
-
-            anyhow::ensure!(
-                output.status.success(),
-                "ClickHouse binary '{name}' not found on $PATH. \
-                 Install ClickHouse (https://clickhouse.com/docs/en/install) or set \
-                 CLICKHOUSE_BINARY env var to an absolute path before building.",
-                name = binary.display()
-            );
-
-            let resolved = String::from_utf8_lossy(&output.stdout);
-            tracing::debug!(
-                resolved = resolved.trim(),
-                "Resolved clickhouse binary from PATH"
-            );
         }
 
+        // Verify the binary is actually usable by running `clickhouse local --version`.
+        let output = Command::new(binary.as_os_str())
+            .args(["local", "--version"])
+            .output()
+            .with_context(|| {
+                format!(
+                    "ClickHouse binary '{name}' not found on $PATH. \
+                     Install ClickHouse (https://clickhouse.com/docs/en/install) or set \
+                     CLICKHOUSE_BINARY env var to an absolute path before building.",
+                    name = binary.display()
+                )
+            })?;
+
+        anyhow::ensure!(
+            output.status.success(),
+            "ClickHouse binary at '{name}' failed to run: {stderr}",
+            name = binary.display(),
+            stderr = String::from_utf8_lossy(&output.stderr)
+        );
+
+        let version = String::from_utf8_lossy(&output.stdout);
+        tracing::debug!(version = version.trim(), "Verified clickhouse binary");
+
         Ok(())
     }
 

benchmarks/clickhouse-bench/src/main.rs

@@ -6,13 +6,12 @@ use std::path::PathBuf;
 use clap::Parser;
 use clickhouse_bench::ClickHouseClient;
 use tokio::runtime::Runtime;
-use vortex_bench::Benchmark;
+use vortex_bench::BenchmarkArg;
 use vortex_bench::Engine;
 use vortex_bench::Format;
 use vortex_bench::Opt;
 use vortex_bench::Opts;
-use vortex_bench::clickbench::ClickBenchBenchmark;
-use vortex_bench::clickbench::Flavor;
+use vortex_bench::create_benchmark;
 use vortex_bench::create_output_writer;
 use vortex_bench::display::DisplayFormat;
 use vortex_bench::runner::SqlBenchmarkRunner;
@@ -21,11 +20,14 @@ use vortex_bench::setup_logging_and_tracing;
 
 /// ClickHouse (clickhouse-local) benchmark runner.
 ///
-/// Runs ClickBench queries against Parquet data using clickhouse-local as a performance baseline.
+/// Runs queries against Parquet data using clickhouse-local as a performance baseline.
 /// This allows comparing ClickHouse's native Parquet reading performance against other engines
 /// (DuckDB, DataFusion) on the same hardware and dataset.
 #[derive(Parser)]
 struct Args {
+    #[arg(value_enum)]
+    benchmark: BenchmarkArg,
+
     #[arg(short, long, default_value_t = 5)]
     iterations: usize,
 
@@ -63,10 +65,7 @@ fn main() -> anyhow::Result<()> {
 
     setup_logging_and_tracing(args.verbose, args.tracing)?;
 
-    let flavor = opts.get_as::<Flavor>("flavor").unwrap_or_default();
-    let remote_data_dir = opts.get_as::<String>("remote-data-dir");
-    let benchmark =
-        ClickBenchBenchmark::new(flavor, None, remote_data_dir)?.with_engine(Engine::ClickHouse);
+    let benchmark = create_benchmark(args.benchmark, &opts)?;
 
     let filtered_queries = filter_queries(
         benchmark.queries()?,
@@ -83,7 +82,7 @@ fn main() -> anyhow::Result<()> {
     let formats = vec![Format::Parquet];
 
     let mut runner = SqlBenchmarkRunner::new(
-        &benchmark,
+        benchmark.as_ref(),
         Engine::ClickHouse,
         formats,
         args.track_memory,
@@ -93,7 +92,7 @@ fn main() -> anyhow::Result<()> {
     runner.run_all(
         &filtered_queries,
         args.iterations,
-        |format| ClickHouseClient::new(&benchmark, format),
+        |format| ClickHouseClient::new(benchmark.as_ref(), format),
         |ctx, _query_idx, _format, query| ctx.execute_query(query),
     )?;