Clippy and lints

AdamGS · AdamGS · commit bbdd8ff0409d · 2025-12-12T16:24:25.000Z
Signed-off-by: Adam Gutglick &lt;adam@spiraldb.com&gt;
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/benchmarks/ddb-bench/src/main.rs b/benchmarks/ddb-bench/src/main.rs
@@ -17,7 +17,7 @@ use vortex_bench::Opts;
 use vortex_bench::conversions::convert_parquet_to_vortex;
 use vortex_bench::create_benchmark;
 use vortex_bench::display::DisplayFormat;
-use vortex_bench::runner::BenchmarkRunner;
+use vortex_bench::runner::SqlBenchmarkRunner;
 use vortex_bench::runner::filter_queries;
 use vortex_bench::setup_logging_and_tracing;
 
@@ -106,7 +106,7 @@ async fn main() -> anyhow::Result<()> {
         }
     }
 
-    let mut runner = BenchmarkRunner::new(
+    let mut runner = SqlBenchmarkRunner::new(
         &*benchmark,
         Engine::DuckDB,
         args.formats.clone(),
diff --git a/benchmarks/df-bench/src/main.rs b/benchmarks/df-bench/src/main.rs
@@ -24,7 +24,7 @@ use vortex_bench::Opts;
 use vortex_bench::conversions::convert_parquet_to_vortex;
 use vortex_bench::create_benchmark;
 use vortex_bench::display::DisplayFormat;
-use vortex_bench::runner::BenchmarkRunner;
+use vortex_bench::runner::SqlBenchmarkRunner;
 use vortex_bench::runner::filter_queries;
 use vortex_bench::setup_logging_and_tracing;
 
@@ -129,7 +129,7 @@ async fn main() -> anyhow::Result<()> {
         }
     }
 
-    let mut runner = BenchmarkRunner::new(
+    let mut runner = SqlBenchmarkRunner::new(
         &*benchmark,
         Engine::DataFusion,
         args.formats.clone(),
diff --git a/benchmarks/lance-bench/Cargo.toml b/benchmarks/lance-bench/Cargo.toml
@@ -19,14 +19,11 @@ lance = { version = "0.39.0" }
 lance-encoding = { version = "0.39.0" }
 
 anyhow = { workspace = true }
-arrow-array = { workspace = true }
 arrow-cast = { workspace = true }
 arrow-schema = { workspace = true }
 clap = { workspace = true, features = ["derive"] }
-datafusion = { workspace = true }
-datafusion-physical-plan = { workspace = true }
 futures = { workspace = true }
-parquet.workspace = true
+parquet = { workspace = true }
 tempfile = { workspace = true }
 tokio = { workspace = true, features = ["full"] }
 tracing.workspace = true
diff --git a/benchmarks/lance-bench/src/compress.rs b/benchmarks/lance-bench/src/compress.rs
@@ -14,12 +14,13 @@ use std::fs;
 use std::path::Path;
 use std::sync::Arc;
 
-use arrow_array::RecordBatch;
-use arrow_array::RecordBatchIterator;
+use anyhow::anyhow;
 use arrow_schema::Schema;
 use futures::StreamExt;
 use lance::dataset::Dataset;
 use lance::dataset::WriteParams;
+use lance::deps::arrow_array::RecordBatch;
+use lance::deps::arrow_array::RecordBatchIterator;
 use lance_encoding::version::LanceFileVersion;
 use tempfile::TempDir;
 
@@ -32,7 +33,9 @@ pub async fn lance_compress_write_only(
     schema: Arc<Schema>,
     dataset_path: &Path,
 ) -> anyhow::Result<()> {
-    let path = dataset_path.to_str().unwrap();
+    let path = dataset_path
+        .to_str()
+        .ok_or_else(|| anyhow!("Failed to convert path to str"))?;
     let reader = RecordBatchIterator::new(batches.into_iter().map(Ok), schema);
     // Lance v2.1 fails on CMSProvider dataset.
     let write_params = WriteParams::with_storage_version(LanceFileVersion::V2_0);
@@ -110,7 +113,9 @@ pub async fn lance_compress_write(
     // Create a fixed subdirectory for decompression testing.
     let dataset_dir = temp_dir.path().join("dataset");
     fs::create_dir_all(&dataset_dir)?;
-    let path = dataset_dir.to_str().unwrap();
+    let path = dataset_dir
+        .to_str()
+        .ok_or_else(|| anyhow!("Failed to convert path to str"))?;
 
     let reader = RecordBatchIterator::new(converted_batches.into_iter().map(Ok), converted_schema);
     // Lance v2.1 fails on CMSProvider dataset.
diff --git a/benchmarks/lance-bench/src/convert.rs b/benchmarks/lance-bench/src/convert.rs
@@ -118,7 +118,7 @@ pub async fn convert_parquet_to_lance<'p>(
             create_dir_all(&lance_dir).await?;
 
             // Collect all Parquet files in the directory
-            let parquet_files: Vec<_> = fs::read_dir(&parquet_dir)?
+            let parquet_files: Vec<_> = fs::read_dir(parquet_dir)?
                 .filter_map(|entry| entry.ok())
                 .filter(|entry| {
                     let path = entry.path();
diff --git a/benchmarks/lance-bench/src/main.rs b/benchmarks/lance-bench/src/main.rs
@@ -6,11 +6,12 @@ use std::sync::Arc;
 
 use clap::Parser;
 use clap::value_parser;
-use datafusion::arrow::array::RecordBatch;
-use datafusion::prelude::SessionContext;
-use datafusion_physical_plan::ExecutionPlan;
+
 use lance::datafusion::LanceTableProvider;
 use lance::dataset::Dataset;
+use lance::deps::arrow_array::RecordBatch;
+use lance::deps::datafusion::physical_plan::ExecutionPlan;
+use lance::deps::datafusion::prelude::SessionContext;
 use lance_bench::convert::convert_parquet_to_lance;
 use tracing::info;
 use vortex_bench::Benchmark;
@@ -21,7 +22,7 @@ use vortex_bench::Opt;
 use vortex_bench::Opts;
 use vortex_bench::create_benchmark;
 use vortex_bench::display::DisplayFormat;
-use vortex_bench::runner::BenchmarkRunner;
+use vortex_bench::runner::SqlBenchmarkRunner;
 use vortex_bench::runner::filter_queries;
 use vortex_bench::setup_logging_and_tracing;
 
@@ -86,7 +87,7 @@ async fn main() -> anyhow::Result<()> {
     // Convert Parquet to Lance format
     generate_lance_data(&*benchmark).await?;
 
-    let mut runner = BenchmarkRunner::new(
+    let mut runner = SqlBenchmarkRunner::new(
         &*benchmark,
         Engine::DataFusion,
         vec![Format::Lance],
@@ -186,14 +187,13 @@ async fn generate_lance_data<B: Benchmark + ?Sized>(benchmark: &B) -> anyhow::Re
         // TPC-H/TPC-DS use {table}_ prefix, others may use the table name directly
         let file_prefix = benchmark
             .pattern(table.name, Format::Parquet)
-            .map(|p| {
+            .and_then(|p| {
                 // Extract prefix from pattern like "customer_*.parquet" -> "customer_"
                 let pattern_str = p.as_str();
                 pattern_str
                     .strip_suffix(&format!("*.{}", Format::Parquet.ext()))
                     .map(|s| s.to_string())
-            })
-            .flatten();
+            });
 
         convert_parquet_to_lance(
             &parquet_dir,
diff --git a/vortex-bench/src/clickbench/clickbench_benchmark.rs b/vortex-bench/src/clickbench/clickbench_benchmark.rs
@@ -74,8 +74,7 @@ impl Benchmark for ClickBenchBenchmark {
             None => Path::new(env!("CARGO_MANIFEST_DIR")).join("clickbench_queries.sql"),
         };
 
-        Ok(fs::read_to_string(queries_filepath)
-            .unwrap()
+        Ok(fs::read_to_string(queries_filepath)?
             .split(';')
             .map(|s| s.trim())
             .filter(|s| !s.is_empty())
diff --git a/vortex-bench/src/clickbench/clickbench_data.rs b/vortex-bench/src/clickbench/clickbench_data.rs
@@ -216,7 +216,7 @@ impl Flavor {
                     let output_path = basepath.join(Format::Parquet.name()).join(format!("hits_{idx}.parquet"));
                     let client = client.clone();
 
-                    idempotent_async(output_path.clone(),  move|output_path| async move {
+                    idempotent_async(output_path,  move|output_path| async move {
                         info!("Downloading file {idx}");
                         let url = format!("https://pub-3ba949c0f0354ac18db1f0f14f0a2c52.r2.dev/clickbench/parquet_many/hits_{idx}.parquet");
                         let  response = retry_get(&client, url).await?;
diff --git a/vortex-bench/src/lib.rs b/vortex-bench/src/lib.rs
@@ -300,12 +300,17 @@ impl Opts {
         self.inner.get(key).map(|s| s.as_str())
     }
 
+    #[expect(clippy::panic)]
     pub fn get_as<T>(&self, key: &str) -> Option<T>
     where
         T: FromStr,
         <T as FromStr>::Err: std::fmt::Debug,
     {
-        self.inner.get(key).map(|v| v.parse().unwrap())
+        self.inner.get(key).map(|v| {
+            v.parse().unwrap_or_else(|_| {
+                panic!("opts value {key} was parsed into an inappropriate type")
+            })
+        })
     }
 }
 
diff --git a/vortex-bench/src/runner.rs b/vortex-bench/src/runner.rs
@@ -12,6 +12,7 @@ use std::time::Duration;
 use std::time::Instant;
 
 use indicatif::ProgressBar;
+use vortex::error::vortex_panic;
 
 use crate::Benchmark;
 use crate::BenchmarkDataset;
@@ -32,7 +33,7 @@ pub struct BenchmarkResults {
     pub memory_measurements: Vec<MemoryMeasurement>,
 }
 
-/// A generic benchmark runner that handles common scaffolding:
+/// A benchmark runner that handles common scaffolding for SQL-oriented benchmarks:
 /// - Progress bar management
 /// - Memory tracking
 /// - Measurement collection
@@ -43,7 +44,7 @@ pub struct BenchmarkResults {
 /// - Context creation
 /// - Table registration
 /// - Query execution
-pub struct BenchmarkRunner {
+pub struct SqlBenchmarkRunner {
     engine: Engine,
     benchmark_dataset: BenchmarkDataset,
     storage: String,
@@ -58,7 +59,7 @@ pub struct BenchmarkRunner {
     memory_measurements: Vec<MemoryMeasurement>,
 }
 
-impl BenchmarkRunner {
+impl SqlBenchmarkRunner {
     /// Create a new benchmark runner.
     pub fn new<B: Benchmark + ?Sized>(
         benchmark: &B,
@@ -213,28 +214,28 @@ impl BenchmarkRunner {
         });
 
         // Validate row count if expected counts are provided
-        if let Some(expected_counts) = &self.expected_row_counts {
-            if query_idx < expected_counts.len() {
-                assert_eq!(
-                    row_count,
-                    expected_counts[query_idx],
-                    "Row count mismatch for query {query_idx} - {engine}:{format}",
-                    engine = self.engine,
-                );
-            }
+        if let Some(expected_counts) = &self.expected_row_counts
+            && query_idx < expected_counts.len()
+        {
+            assert_eq!(
+                row_count,
+                expected_counts[query_idx],
+                "Row count mismatch for query {query_idx} - {engine}:{format}",
+                engine = self.engine,
+            );
         }
 
         // Record memory measurement if tracking is enabled
-        if let Some(tracker) = self.memory_tracker.as_ref() {
-            if let Some(memory_result) = tracker.end_query() {
-                self.memory_measurements.push(MemoryMeasurement::new(
-                    query_idx,
-                    target,
-                    self.benchmark_dataset.clone(),
-                    self.storage.clone(),
-                    memory_result,
-                ));
-            }
+        if let Some(tracker) = self.memory_tracker.as_ref()
+            && let Some(memory_result) = tracker.end_query()
+        {
+            self.memory_measurements.push(MemoryMeasurement::new(
+                query_idx,
+                target,
+                self.benchmark_dataset.clone(),
+                self.storage.clone(),
+                memory_result,
+            ));
         }
 
         self.progress_bar.inc(1);
@@ -325,7 +326,7 @@ impl BenchmarkRunner {
                 self.run_query(query_idx, format, iterations, || {
                     let row_count =
                         execute(&ctx, query_idx, query.as_str()).unwrap_or_else(|err| {
-                            panic!("query {query_idx} failed: {err}");
+                            vortex_panic!("query {query_idx} failed: {err}");
                         });
                     (row_count, ())
                 });
@@ -375,7 +376,7 @@ impl BenchmarkRunner {
                     let (row_count, iter_result) = execute(&ctx, query_idx, query.as_str())
                         .await
                         .unwrap_or_else(|err| {
-                            panic!("query {query_idx} failed: {err}");
+                            vortex_panic!("query {query_idx} failed: {err}");
                         });
                     runs.push(start.elapsed());
 
diff --git a/vortex-bench/src/statpopgen/download_vcf.rs b/vortex-bench/src/statpopgen/download_vcf.rs
@@ -4,23 +4,17 @@
 use anyhow::Context;
 use anyhow::Result;
 use anyhow::bail;
-
 use indicatif::ProgressBar;
 use indicatif::ProgressStyle;
 use noodles_vcf::Record;
 use parquet::arrow::AsyncArrowWriter;
-
 use reqwest::Client;
 use tokio::fs::File;
-
 use tokio::io::BufReader;
-
 use tracing::info;
-
 use vortex::io::IoBuf;
 
 use super::StatPopGenBenchmark;
-
 use crate::idempotent_async;
 use crate::statpopgen::builder::GnomADBuilder;
 use crate::statpopgen::schema::schema_from_vcf_header;
diff --git a/vortex-bench/src/statpopgen/statpopgen_benchmark.rs b/vortex-bench/src/statpopgen/statpopgen_benchmark.rs
@@ -176,11 +176,12 @@ impl Benchmark for StatPopGenBenchmark {
         vec![TableSpec::new("statpopgen", None)]
     }
 
+    #[expect(clippy::expect_used, clippy::unwrap_in_result)]
     fn pattern(&self, _table_name: &str, format: Format) -> Option<glob::Pattern> {
         Some(
             format!("{}.{}", Self::FILE_NAME, format.ext())
                 .parse()
-                .unwrap(),
+                .expect("valid glob pattern"),
         )
     }
 }
diff --git a/vortex-bench/src/tpcds/duckdb.rs b/vortex-bench/src/tpcds/duckdb.rs
@@ -11,7 +11,7 @@ use crate::Format;
 
 pub fn generate_tpcds(base_dir: PathBuf, scale_factor: String) -> Result<PathBuf> {
     // Create output directory based on format
-    let output_dir = base_dir.join(Format::Parquet.name().to_string());
+    let output_dir = base_dir.join(Format::Parquet.name());
     std::fs::create_dir_all(&output_dir)?;
 
     // Check if already generated
diff --git a/vortex-bench/src/tpcds/tpcds_benchmark.rs b/vortex-bench/src/tpcds/tpcds_benchmark.rs
@@ -124,7 +124,12 @@ impl Benchmark for TpcDsBenchmark {
         ]
     }
 
+    #[expect(clippy::expect_used, clippy::unwrap_in_result)]
     fn pattern(&self, table_name: &str, format: Format) -> Option<Pattern> {
-        Some(format!("{}.{}", table_name, format.ext()).parse().unwrap())
+        Some(
+            format!("{}.{}", table_name, format.ext())
+                .parse()
+                .expect("valid glob pattern"),
+        )
     }
 }
diff --git a/vortex-bench/src/tpch/benchmark.rs b/vortex-bench/src/tpch/benchmark.rs
@@ -139,11 +139,12 @@ impl Benchmark for TpcHBenchmark {
         ]
     }
 
+    #[expect(clippy::expect_used, clippy::unwrap_in_result)]
     fn pattern(&self, table_name: &str, format: Format) -> Option<Pattern> {
         Some(
             format!("{}_*.{}", table_name, format.ext())
                 .parse()
-                .unwrap(),
+                .expect("valid glob pattern"),
         )
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -300,12 +300,17 @@ impl Opts {`
`300`	`300`	`self.inner.get(key).map(\|s\| s.as_str())`
`301`	`301`	`}`
`302`	`302`
	`303`	`+ #[expect(clippy::panic)]`
`303`	`304`	`pub fn get_as<T>(&self, key: &str) -> Option<T>`
`304`	`305`	`where`
`305`	`306`	`T: FromStr,`
`306`	`307`	`<T as FromStr>::Err: std::fmt::Debug,`
`307`	`308`	`{`
`308`		`- self.inner.get(key).map(\|v\| v.parse().unwrap())`
	`309`	`+ self.inner.get(key).map(\|v\| {`
	`310`	`+ v.parse().unwrap_or_else(\|_\| {`
	`311`	`+ panic!("opts value {key} was parsed into an inappropriate type")`
	`312`	`+ })`
	`313`	`+ })`
`309`	`314`	`}`
`310`	`315`	`}`
`311`	`316`