From 5f00e02b87c8718589c13475a7dc9f5de64ecc0c Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Wed, 28 Dec 2022 12:03:58 +0000 Subject: [PATCH] Avoid boxing config keys --- datafusion/core/src/config.rs | 45 +++++++++++++----------- datafusion/core/src/execution/context.rs | 17 ++++----- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/datafusion/core/src/config.rs b/datafusion/core/src/config.rs index 4dabb53d24250..e51dd94ead397 100644 --- a/datafusion/core/src/config.rs +++ b/datafusion/core/src/config.rs @@ -21,6 +21,7 @@ use arrow::datatypes::DataType; use datafusion_common::ScalarValue; use itertools::Itertools; use log::warn; +use std::borrow::Cow; use std::collections::{BTreeMap, HashMap}; use std::env; use std::fmt::{Debug, Formatter}; @@ -134,9 +135,9 @@ pub const OPT_HASH_JOIN_SINGLE_PARTITION_THRESHOLD: &str = /// Definition of a configuration option pub struct ConfigDefinition { /// key used to identifier this configuration option - key: String, + key: Cow<'static, str>, /// Description to be used in generated documentation - description: String, + description: Cow<'static, str>, /// Data type of this option data_type: DataType, /// Default value @@ -162,8 +163,8 @@ macro_rules! get_conf_value { impl ConfigDefinition { /// Create a configuration option definition pub fn new( - name: impl Into, - description: impl Into, + name: impl Into>, + description: impl Into>, data_type: DataType, default_value: ScalarValue, ) -> Self { @@ -177,8 +178,8 @@ impl ConfigDefinition { /// Create a configuration option definition with a boolean value pub fn new_bool( - key: impl Into, - description: impl Into, + key: impl Into>, + description: impl Into>, default_value: bool, ) -> Self { Self::new( @@ -191,8 +192,8 @@ impl ConfigDefinition { /// Create a configuration option definition with a u64 value pub fn new_u64( - key: impl Into, - description: impl Into, + key: impl Into>, + description: impl Into>, default_value: u64, ) -> Self { Self::new( @@ -205,8 +206,8 @@ impl ConfigDefinition { /// Create a configuration option definition with a string value pub fn new_string( - key: impl Into, - description: impl Into, + key: impl Into>, + description: impl Into>, default_value: Option, ) -> Self { Self::new( @@ -426,7 +427,7 @@ impl BuiltInConfigs { .map(normalize_for_display) .collect(); - for config in config_definitions.iter().sorted_by_key(|c| c.key.as_str()) { + for config in config_definitions.iter().sorted_by_key(|c| c.key.as_ref()) { let _ = writeln!( &mut docs, "| {} | {} | {} | {} |", @@ -450,7 +451,7 @@ fn normalize_for_display(mut v: ConfigDefinition) -> ConfigDefinition { /// Configuration options struct. This can contain values for built-in and custom options #[derive(Clone)] pub struct ConfigOptions { - options: HashMap, + options: HashMap, ScalarValue>, } /// Print the configurations in an ordered way so that we can directly compare the equality of two ConfigOptions by their debug strings @@ -514,28 +515,32 @@ impl ConfigOptions { } /// set a configuration option - pub fn set(&mut self, key: &str, value: ScalarValue) { - self.options.insert(key.to_string(), value); + pub fn set(&mut self, key: impl Into>, value: ScalarValue) { + self.options.insert(key.into(), value); } /// set a boolean configuration option - pub fn set_bool(&mut self, key: &str, value: bool) { + pub fn set_bool(&mut self, key: impl Into>, value: bool) { self.set(key, ScalarValue::Boolean(Some(value))) } /// set a `u64` configuration option - pub fn set_u64(&mut self, key: &str, value: u64) { + pub fn set_u64(&mut self, key: impl Into>, value: u64) { self.set(key, ScalarValue::UInt64(Some(value))) } /// set a `usize` configuration option - pub fn set_usize(&mut self, key: &str, value: usize) { + pub fn set_usize(&mut self, key: impl Into>, value: usize) { let value: u64 = value.try_into().expect("convert u64 to usize"); self.set(key, ScalarValue::UInt64(Some(value))) } /// set a `String` configuration option - pub fn set_string(&mut self, key: &str, value: impl Into) { + pub fn set_string( + &mut self, + key: impl Into>, + value: impl Into, + ) { self.set(key, ScalarValue::Utf8(Some(value.into()))) } @@ -566,7 +571,7 @@ impl ConfigOptions { } /// Access the underlying hashmap - pub fn options(&self) -> &HashMap { + pub fn options(&self) -> &HashMap, ScalarValue> { &self.options } @@ -590,7 +595,7 @@ mod test { ); let configs = BuiltInConfigs::default(); for config in configs.config_definitions { - assert!(docs.contains(&config.key)); + assert!(docs.contains(config.key.as_ref())); } } diff --git a/datafusion/core/src/execution/context.rs b/datafusion/core/src/execution/context.rs index fcfcf1ca69035..219cd34cd20e9 100644 --- a/datafusion/core/src/execution/context.rs +++ b/datafusion/core/src/execution/context.rs @@ -35,6 +35,7 @@ use crate::{ pub use datafusion_physical_expr::execution_props::ExecutionProps; use datafusion_physical_expr::var_provider::is_system_variables; use parking_lot::RwLock; +use std::borrow::Cow; use std::str::FromStr; use std::sync::Arc; use std::{ @@ -391,7 +392,7 @@ impl SessionContext { value, )) })?; - config_options.set_bool(&variable, new_value); + config_options.set_bool(variable, new_value); } ScalarValue::UInt64(_) => { @@ -401,7 +402,7 @@ impl SessionContext { value, )) })?; - config_options.set_u64(&variable, new_value); + config_options.set_u64(variable, new_value); } ScalarValue::Utf8(_) => { @@ -411,7 +412,7 @@ impl SessionContext { value, )) })?; - config_options.set_string(&variable, new_value); + config_options.set_string(variable, new_value); } _ => { @@ -1192,29 +1193,29 @@ impl SessionConfig { } /// Set a configuration option - pub fn set(mut self, key: &str, value: ScalarValue) -> Self { + pub fn set(mut self, key: impl Into>, value: ScalarValue) -> Self { self.config_options.set(key, value); self } /// Set a boolean configuration option - pub fn set_bool(self, key: &str, value: bool) -> Self { + pub fn set_bool(self, key: impl Into>, value: bool) -> Self { self.set(key, ScalarValue::Boolean(Some(value))) } /// Set a generic `u64` configuration option - pub fn set_u64(self, key: &str, value: u64) -> Self { + pub fn set_u64(self, key: impl Into>, value: u64) -> Self { self.set(key, ScalarValue::UInt64(Some(value))) } /// Set a generic `usize` configuration option - pub fn set_usize(self, key: &str, value: usize) -> Self { + pub fn set_usize(self, key: impl Into>, value: usize) -> Self { let value: u64 = value.try_into().expect("convert usize to u64"); self.set(key, ScalarValue::UInt64(Some(value))) } /// Set a generic `str` configuration option - pub fn set_str(self, key: &str, value: &str) -> Self { + pub fn set_str(self, key: impl Into>, value: &str) -> Self { self.set(key, ScalarValue::Utf8(Some(value.to_string()))) }