Update 4 crates to rust 2024 edition (#19357)

## Which issue does this PR close? This addresses part of #15804 but does not close it. ## Rationale for this change Now that we are on MSRV 1.88 we can use rust edition 2024, which brings let chains and other nice features. It also improves `unsafe` checking. In order to introduce these changes in slower way instead of one massive PR that is too difficult to manage we are updating a few crates at a time. ## What changes are included in this PR? Updates these crates to 2024. - datafusion-cli - datafusion-proto - datafusion-substrait - datafusion-wasmtest ## Are these changes tested? Existing unit tests. There are no functional code changes. ## Are there any user-facing changes? None. ## Note It is recommended to review with the ignore whitespace setting: https://github.com/apache/datafusion/pull/19357/files?w=1

Author: timsaucer

datafusion-cli/Cargo.toml

@@ -20,7 +20,7 @@ name = "datafusion-cli"
 description = "Command Line Client for DataFusion query engine."
 readme = "README.md"
 version = { workspace = true }
-edition = { workspace = true }
+edition = "2024"
 homepage = { workspace = true }
 repository = { workspace = true }
 license = { workspace = true }

datafusion-cli/examples/cli-session-context.rs

@@ -23,7 +23,7 @@ use std::sync::Arc;
 use datafusion::{
  dataframe::DataFrame,
  error::DataFusionError,
- execution::{context::SessionState, TaskContext},
+ execution::{TaskContext, context::SessionState},
  logical_expr::{LogicalPlan, LogicalPlanBuilder},
  prelude::SessionContext,
 };

datafusion-cli/src/catalog.rs

@@ -18,13 +18,13 @@
 use std::any::Any;
 use std::sync::{Arc, Weak};
 
-use crate::object_storage::{get_object_store, AwsOptions, GcpOptions};
+use crate::object_storage::{AwsOptions, GcpOptions, get_object_store};
 
 use datafusion::catalog::{CatalogProvider, CatalogProviderList, SchemaProvider};
 
 use datafusion::common::plan_datafusion_err;
-use datafusion::datasource::listing::ListingTableUrl;
 use datafusion::datasource::TableProvider;
+use datafusion::datasource::listing::ListingTableUrl;
 use datafusion::error::Result;
 use datafusion::execution::context::SessionState;
 use datafusion::execution::session_state::SessionStateBuilder;
@@ -152,10 +152,10 @@ impl SchemaProvider for DynamicObjectStoreSchemaProvider {
 
  async fn table(&self, name: &str) -> Result<Option<Arc<dyn TableProvider>>> {
  let inner_table = self.inner.table(name).await;
- if inner_table.is_ok() {
- if let Some(inner_table) = inner_table? {
-  return Ok(Some(inner_table));
- }
+ if inner_table.is_ok()
+ && let Some(inner_table) = inner_table?
+ {
+ return Ok(Some(inner_table));
  }
 
  // if the inner schema provider didn't have a table by
@@ -219,12 +219,12 @@ impl SchemaProvider for DynamicObjectStoreSchemaProvider {
 }
 
 pub fn substitute_tilde(cur: String) -> String {
- if let Some(usr_dir_path) = home_dir() {
- if let Some(usr_dir) = usr_dir_path.to_str() {
-  if cur.starts_with('~') && !usr_dir.is_empty() {
-  return cur.replacen('~', usr_dir, 1);
-  }
- }
+ if let Some(usr_dir_path) = home_dir()
+ && let Some(usr_dir) = usr_dir_path.to_str()
+ && cur.starts_with('~')
+ && !usr_dir.is_empty()
+ {
+ return cur.replacen('~', usr_dir, 1);
  }
  cur
 }
@@ -359,10 +359,12 @@ mod tests {
  } else {
  "/home/user"
  };
- env::set_var(
- if cfg!(windows) { "USERPROFILE" } else { "HOME" },
- test_home_path,
- );
+ unsafe {
+ env::set_var(
+ if cfg!(windows) { "USERPROFILE" } else { "HOME" },
+ test_home_path,
+ );
+ }
  let input = "~/Code/datafusion/benchmarks/data/tpch_sf1/part/part-0.parquet";
  let expected = PathBuf::from(test_home_path)
  .join("Code")
@@ -376,12 +378,16 @@ mod tests {
  .to_string();
  let actual = substitute_tilde(input.to_string());
  assert_eq!(actual, expected);
- match original_home {
- Some(home_path) => env::set_var(
- if cfg!(windows) { "USERPROFILE" } else { "HOME" },
- home_path.to_str().unwrap(),
- ),
- None => env::remove_var(if cfg!(windows) { "USERPROFILE" } else { "HOME" }),
+ unsafe {
+ match original_home {
+ Some(home_path) => env::set_var(
+ if cfg!(windows) { "USERPROFILE" } else { "HOME" },
+ home_path.to_str().unwrap(),
+ ),
+ None => {
+ env::remove_var(if cfg!(windows) { "USERPROFILE" } else { "HOME" })
+ }
+ }
  }
  }
 }

datafusion-cli/src/cli_context.rs

@@ -20,7 +20,7 @@ use std::sync::Arc;
 use datafusion::{
  dataframe::DataFrame,
  error::DataFusionError,
- execution::{context::SessionState, TaskContext},
+ execution::{TaskContext, context::SessionState},
  logical_expr::LogicalPlan,
  prelude::SessionContext,
 };

datafusion-cli/src/command.rs

@@ -19,7 +19,7 @@
 
 use crate::cli_context::CliSessionContext;
 use crate::exec::{exec_and_print, exec_from_lines};
-use crate::functions::{display_all_functions, Function};
+use crate::functions::{Function, display_all_functions};
 use crate::print_format::PrintFormat;
 use crate::print_options::PrintOptions;
 use clap::ValueEnum;

datafusion-cli/src/exec.rs

@@ -35,19 +35,19 @@ use datafusion::execution::memory_pool::MemoryConsumer;
 use datafusion::logical_expr::{DdlStatement, LogicalPlan};
 use datafusion::physical_plan::execution_plan::EmissionType;
 use datafusion::physical_plan::spill::get_record_batch_memory_size;
-use datafusion::physical_plan::{execute_stream, ExecutionPlanProperties};
+use datafusion::physical_plan::{ExecutionPlanProperties, execute_stream};
 use datafusion::sql::parser::{DFParser, Statement};
 use datafusion::sql::sqlparser;
 use datafusion::sql::sqlparser::dialect::dialect_from_str;
 use futures::StreamExt;
 use log::warn;
 use object_store::Error::Generic;
-use rustyline::error::ReadlineError;
 use rustyline::Editor;
+use rustyline::error::ReadlineError;
 use std::collections::HashMap;
 use std::fs::File;
-use std::io::prelude::*;
 use std::io::BufReader;
+use std::io::prelude::*;
 use tokio::signal;
 
 /// run and execute SQL statements and commands, against a context with the given print options
@@ -168,7 +168,10 @@ pub async fn exec_from_repl(
  }
  }
  } else {
- eprintln!("'\\{}' is not a valid command, you can use '\\?' to see all commands", &line[1..]);
+ eprintln!(
+ "'\\{}' is not a valid command, you can use '\\?' to see all commands",
+ &line[1..]
+ );
  }
  }
  Ok(line) => {
@@ -334,7 +337,9 @@ impl StatementExecutor {
  if matches!(err.as_ref(), Generic { store, source: _ } if "S3".eq_ignore_ascii_case(store))
  && self.statement_for_retry.is_some() =>
  {
- warn!("S3 region is incorrect, auto-detecting the correct region (this may be slow). Consider updating your region configuration.");
+ warn!(
+ "S3 region is incorrect, auto-detecting the correct region (this may be slow). Consider updating your region configuration."
+ );
  let plan =
  create_plan(ctx, self.statement_for_retry.take().unwrap(), true)
  .await?;
@@ -699,8 +704,7 @@ mod tests {
  #[tokio::test]
  async fn create_object_store_table_gcs() -> Result<()> {
  let service_account_path = "fake_service_account_path";
- let service_account_key =
- "{\"private_key\": \"fake_private_key.pem\",\"client_email\":\"fake_client_email\", \"private_key_id\":\"id\"}";
+ let service_account_key = "{\"private_key\": \"fake_private_key.pem\",\"client_email\":\"fake_client_email\", \"private_key_id\":\"id\"}";
  let application_credentials_path = "fake_application_credentials_path";
  let location = "gcs://bucket/path/file.parquet";
 
@@ -713,7 +717,9 @@ mod tests {
  assert!(err.to_string().contains("os error 2"));
 
  // for service_account_key
- let sql = format!("CREATE EXTERNAL TABLE test STORED AS PARQUET OPTIONS('gcp.service_account_key' '{service_account_key}') LOCATION '{location}'");
+ let sql = format!(
+ "CREATE EXTERNAL TABLE test STORED AS PARQUET OPTIONS('gcp.service_account_key' '{service_account_key}') LOCATION '{location}'"
+ );
  let err = create_external_table_test(location, &sql)
  .await
  .unwrap_err()
@@ -748,8 +754,9 @@ mod tests {
  let location = "path/to/file.cvs";
 
  // Test with format options
- let sql =
- format!("CREATE EXTERNAL TABLE test STORED AS CSV LOCATION '{location}' OPTIONS('format.has_header' 'true')");
+ let sql = format!(
+ "CREATE EXTERNAL TABLE test STORED AS CSV LOCATION '{location}' OPTIONS('format.has_header' 'true')"
+ );
  create_external_table_test(location, &sql).await.unwrap();
 
  Ok(())

datafusion-cli/src/functions.rs

@@ -27,9 +27,9 @@ use arrow::datatypes::{DataType, Field, Schema, SchemaRef, TimeUnit};
 use arrow::record_batch::RecordBatch;
 use arrow::util::pretty::pretty_format_batches;
 use datafusion::catalog::{Session, TableFunctionImpl};
-use datafusion::common::{plan_err, Column};
-use datafusion::datasource::memory::MemorySourceConfig;
+use datafusion::common::{Column, plan_err};
 use datafusion::datasource::TableProvider;
+use datafusion::datasource::memory::MemorySourceConfig;
 use datafusion::error::Result;
 use datafusion::execution::cache::cache_manager::CacheManager;
 use datafusion::logical_expr::Expr;

datafusion-cli/src/helper.rs

@@ -67,7 +67,7 @@ impl CliHelper {
  return Ok(ValidationResult::Invalid(Some(format!(
  " 🤔 Invalid dialect: {}",
  self.dialect
- ))))
+ ))));
  }
  };
  let lines = split_from_semicolon(sql);
@@ -121,10 +121,10 @@ impl Hinter for CliHelper {
 fn is_open_quote_for_location(line: &str, pos: usize) -> bool {
  let mut sql = line[..pos].to_string();
  sql.push('\'');
- if let Ok(stmts) = DFParser::parse_sql(&sql) {
- if let Some(Statement::CreateExternalTable(_)) = stmts.back() {
-  return true;
- }
+ if let Ok(stmts) = DFParser::parse_sql(&sql)
+ && let Some(Statement::CreateExternalTable(_)) = stmts.back()
+ {
+ return true;
  }
  false
 }

datafusion-cli/src/highlighter.rs

@@ -23,7 +23,7 @@ use std::{
 };
 
 use datafusion::sql::sqlparser::{
- dialect::{dialect_from_str, Dialect, GenericDialect},
+ dialect::{Dialect, GenericDialect, dialect_from_str},
  keywords::Keyword,
  tokenizer::{Token, Tokenizer},
 };
@@ -94,8 +94,8 @@ impl Color {
 
 #[cfg(test)]
 mod tests {
- use super::config::Dialect;
  use super::SyntaxHighlighter;
+ use super::config::Dialect;
  use rustyline::highlight::Highlighter;
 
  #[test]

datafusion-cli/src/main.rs

@@ -38,11 +38,10 @@ use datafusion_cli::object_storage::instrumented::{
  InstrumentedObjectStoreMode, InstrumentedObjectStoreRegistry,
 };
 use datafusion_cli::{
- exec,
+ DATAFUSION_CLI_VERSION, exec,
  pool_type::PoolType,
  print_format::PrintFormat,
  print_options::{MaxRows, PrintOptions},
- DATAFUSION_CLI_VERSION,
 };
 
 use clap::Parser;
@@ -504,8 +503,7 @@ mod tests {
  ctx.register_udtf("parquet_metadata", Arc::new(ParquetMetadataFunc {}));
 
  // input with single quote
- let sql =
- "SELECT * FROM parquet_metadata('../datafusion/core/tests/data/fixed_size_list_array.parquet')";
+ let sql = "SELECT * FROM parquet_metadata('../datafusion/core/tests/data/fixed_size_list_array.parquet')";
  let df = ctx.sql(sql).await?;
  let rbs = df.collect().await?;
 
@@ -518,8 +516,7 @@ mod tests {
  "#);
 
  // input with double quote
- let sql =
- "SELECT * FROM parquet_metadata(\"../datafusion/core/tests/data/fixed_size_list_array.parquet\")";
+ let sql = "SELECT * FROM parquet_metadata(\"../datafusion/core/tests/data/fixed_size_list_array.parquet\")";
  let df = ctx.sql(sql).await?;
  let rbs = df.collect().await?;
  assert_snapshot!(batches_to_string(&rbs), @r#"
@@ -539,8 +536,7 @@ mod tests {
  ctx.register_udtf("parquet_metadata", Arc::new(ParquetMetadataFunc {}));
 
  // input with string columns
- let sql =
- "SELECT * FROM parquet_metadata('../parquet-testing/data/data_index_bloom_encoding_stats.parquet')";
+ let sql = "SELECT * FROM parquet_metadata('../parquet-testing/data/data_index_bloom_encoding_stats.parquet')";
  let df = ctx.sql(sql).await?;
  let rbs = df.collect().await?;