Add configurable normalization for configuration options and preserve case for S3 paths

datafusion-cli/src/object_storage.rs

@@ -472,12 +472,13 @@ mod tests {
 
     #[tokio::test]
     async fn s3_object_store_builder() -> Result<()> {
-        let access_key_id = "fake_access_key_id";
-        let secret_access_key = "fake_secret_access_key";
+        // "fake" is uppercase to ensure the values are not lowercased when parsed
+        let access_key_id = "FAKE_access_key_id";
+        let secret_access_key = "FAKE_secret_access_key";
         let region = "fake_us-east-2";
         let endpoint = "endpoint33";
-        let session_token = "fake_session_token";
-        let location = "s3://bucket/path/file.parquet";
+        let session_token = "FAKE_session_token";
+        let location = "s3://bucket/path/FAKE/file.parquet";
 
         let table_url = ListingTableUrl::parse(location)?;
         let scheme = table_url.scheme();

datafusion/common/src/config.rs

@@ -19,6 +19,7 @@
 
 use std::any::Any;
 use std::collections::{BTreeMap, HashMap};
+use std::error::Error;
 use std::fmt::{self, Display};
 use std::str::FromStr;
 
@@ -38,7 +39,7 @@ use crate::{DataFusionError, Result};
 ///    /// Amazing config
 ///    pub struct MyConfig {
 ///        /// Field 1 doc
-///        field1: String, default = "".to_string()
+///        field1: String, transform = str::to_lowercase, default = "".to_string()
 ///
 ///        /// Field 2 doc
 ///        field2: usize, default = 232
@@ -67,9 +68,12 @@ use crate::{DataFusionError, Result};
 ///     fn set(&mut self, key: &str, value: &str) -> Result<()> {
 ///         let (key, rem) = key.split_once('.').unwrap_or((key, ""));
 ///         match key {
-///             "field1" => self.field1.set(rem, value),
-///             "field2" => self.field2.set(rem, value),
-///             "field3" => self.field3.set(rem, value),
+///             "field1" => {
+///                 let value = str::to_lowercase(value);
+///                 self.field1.set(rem, value.as_ref())
+///             },
+///             "field2" => self.field2.set(rem, value.as_ref()),
+///             "field3" => self.field3.set(rem, value.as_ref()),
 ///             _ => _internal_err!(
 ///                 "Config value \"{}\" not found on MyConfig",
 ///                 key
@@ -102,15 +106,14 @@ use crate::{DataFusionError, Result};
 /// ```
 ///
 /// NB: Misplaced commas may result in nonsensical errors
-///
 #[macro_export]
 macro_rules! config_namespace {
     (
      $(#[doc = $struct_d:tt])*
      $vis:vis struct $struct_name:ident {
         $(
         $(#[doc = $d:tt])*
-        $field_vis:vis $field_name:ident : $field_type:ty, default = $default:expr
+        $field_vis:vis $field_name:ident : $field_type:ty, $(transform = $transform:expr,)? default = $default:expr
         )*$(,)*
     }
     ) => {
@@ -127,9 +130,13 @@ macro_rules! config_namespace {
         impl ConfigField for $struct_name {
             fn set(&mut self, key: &str, value: &str) -> Result<()> {
                 let (key, rem) = key.split_once('.').unwrap_or((key, ""));
+
                 match key {
                     $(
-                       stringify!($field_name) => self.$field_name.set(rem, value),
+                       stringify!($field_name) => {
+                           $(let value = $transform(value);)?
+                           self.$field_name.set(rem, value.as_ref())
+                       },
                     )*
                     _ => return _config_err!(
                         "Config value \"{}\" not found on {}", key, stringify!($struct_name)
@@ -212,11 +219,12 @@ config_namespace! {
         pub enable_ident_normalization: bool, default = true
 
         /// When set to true, SQL parser will normalize options value (convert value to lowercase)
-        pub enable_options_value_normalization: bool, default = true
+        pub enable_options_value_normalization: bool, default = false
 
         /// Configure the SQL dialect used by DataFusion's parser; supported values include: Generic,
         /// MySQL, PostgreSQL, Hive, SQLite, Snowflake, Redshift, MsSQL, ClickHouse, BigQuery, and Ansi.
-        pub dialect: String, default = "generic".to_string()
+        pub dialect: String, default = "generic".to_string() // no need to lowercase because
+        // [`sqlparser::dialect_from_str`] is case-insensitive
 
         /// If true, permit lengths for `VARCHAR` such as `VARCHAR(20)`, but
         /// ignore the length. If false, error if a `VARCHAR` with a length is
@@ -431,7 +439,7 @@ config_namespace! {
         ///
         /// Note that this default setting is not the same as
         /// the default parquet writer setting.
-        pub compression: Option<String>, default = Some("zstd(3)".into())
+        pub compression: Option<String>, transform = str::to_lowercase, default = Some("zstd(3)".into())
 
         /// (writing) Sets if dictionary encoding is enabled. If NULL, uses
         /// default parquet writer setting
@@ -444,7 +452,7 @@ config_namespace! {
         /// Valid values are: "none", "chunk", and "page"
         /// These values are not case sensitive. If NULL, uses
         /// default parquet writer setting
-        pub statistics_enabled: Option<String>, default = Some("page".into())
+        pub statistics_enabled: Option<String>, transform = str::to_lowercase, default = Some("page".into())
 
         /// (writing) Sets max statistics size for any column. If NULL, uses
         /// default parquet writer setting
@@ -470,7 +478,7 @@ config_namespace! {
         /// delta_byte_array, rle_dictionary, and byte_stream_split.
         /// These values are not case sensitive. If NULL, uses
         /// default parquet writer setting
-        pub encoding: Option<String>, default = None
+        pub encoding: Option<String>, transform = str::to_lowercase, default = None
 
         /// (writing) Use any available bloom filters when reading parquet files
         pub bloom_filter_on_read: bool, default = true
@@ -971,29 +979,46 @@ impl<F: ConfigField + Default> ConfigField for Option<F> {
     }
 }
 
+fn parse<T>(input: &str) -> Result<T>
+where
+    T: FromStr,
+    T::Err: Display,
+    <T as FromStr>::Err: Sync + Send + Error + 'static,
+{
+    input.parse().map_err(|e| {
+        DataFusionError::Context(
+            format!(
+                "Error parsing '{}' as {}",
+                input,
+                std::any::type_name::<T>()
+            ),
+            Box::new(DataFusionError::External(Box::new(e))),
+        )
+    })
+}
+
 #[macro_export]
 macro_rules! config_field {
     ($t:ty) => {
+        config_field!($t, value => parse(value)?);
+    };
+
+    ($t:ty, $arg:ident => $transform:expr) => {
         impl ConfigField for $t {
             fn visit<V: Visit>(&self, v: &mut V, key: &str, description: &'static str) {
                 v.some(key, self, description)
             }
 
-            fn set(&mut self, _: &str, value: &str) -> Result<()> {
-                *self = value.parse().map_err(|e| {
-                    DataFusionError::Context(
-                        format!(concat!("Error parsing {} as ", stringify!($t),), value),
-                        Box::new(DataFusionError::External(Box::new(e))),
-                    )
-                })?;
+            fn set(&mut self, _: &str, $arg: &str) -> Result<()> {
+                *self = $transform;
                 Ok(())
             }
         }
     };
 }
 
 config_field!(String);
-config_field!(bool);
+config_field!(bool, value => parse(value.to_lowercase().as_str())?);
 config_field!(usize);
 config_field!(f64);
 config_field!(u64);

datafusion/core/src/datasource/stream.rs

@@ -62,7 +62,7 @@ impl TableProviderFactory for StreamTableFactory {
         let header = if let Ok(opt) = cmd
             .options
             .get("format.has_header")
-            .map(|has_header| bool::from_str(has_header))
+            .map(|has_header| bool::from_str(has_header.to_lowercase().as_str()))
             .transpose()
         {
             opt.unwrap_or(false)

datafusion/core/tests/config_from_env.rs

@@ -22,10 +22,19 @@ use std::env;
 fn from_env() {
     // Note: these must be a single test to avoid interference from concurrent execution
     let env_key = "DATAFUSION_OPTIMIZER_FILTER_NULL_JOIN_KEYS";
-    env::set_var(env_key, "true");
-    let config = ConfigOptions::from_env().unwrap();
+    // valid testing in different cases
+    for bool_option in ["true", "TRUE", "True"] {
+        env::set_var(env_key, bool_option);
+        let config = ConfigOptions::from_env().unwrap();
+        env::remove_var(env_key);
+        assert!(config.optimizer.filter_null_join_keys);
+    }
+
+    // invalid testing
+    env::set_var(env_key, "ttruee");
+    let err = ConfigOptions::from_env().unwrap_err().strip_backtrace();
+    assert_eq!(err, "Error parsing 'ttruee' as bool\ncaused by\nExternal error: provided string was not `true` or `false`");
     env::remove_var(env_key);
-    assert!(config.optimizer.filter_null_join_keys);
 
     let env_key = "DATAFUSION_EXECUTION_BATCH_SIZE";
 
@@ -37,7 +46,7 @@ fn from_env() {
     // for invalid testing
     env::set_var(env_key, "abc");
     let err = ConfigOptions::from_env().unwrap_err().strip_backtrace();
-    assert_eq!(err, "Error parsing abc as usize\ncaused by\nExternal error: invalid digit found in string");
+    assert_eq!(err, "Error parsing 'abc' as usize\ncaused by\nExternal error: invalid digit found in string");
 
     env::remove_var(env_key);
     let config = ConfigOptions::from_env().unwrap();

datafusion/sql/src/planner.rs

@@ -56,7 +56,7 @@ impl Default for ParserOptions {
             parse_float_as_decimal: false,
             enable_ident_normalization: true,
             support_varchar_with_length: true,
-            enable_options_value_normalization: true,
+            enable_options_value_normalization: false,
         }
     }
 }

datafusion/sqllogictest/test_files/create_external_table.slt

@@ -226,6 +226,20 @@ OPTIONS (
         has_header false,
         compression gzip);
 
+# Verify that some options are case insensitive
+statement ok
+CREATE EXTERNAL TABLE IF NOT EXISTS region (
+        r_regionkey BIGINT,
+        r_name VARCHAR,
+        r_comment VARCHAR,
+        r_rev VARCHAR,
+) STORED AS CSV LOCATION 'test_files/tpch/data/region.tbl'
+OPTIONS (
+        format.delimiter '|',
+        has_header FALSE,
+        compression GZIP);
+
+
 # Create an external parquet table and infer schema to order by
 
 # query should succeed

datafusion/sqllogictest/test_files/information_schema.slt

@@ -258,7 +258,7 @@ datafusion.optimizer.skip_failed_rules false
 datafusion.optimizer.top_down_join_key_reordering true
 datafusion.sql_parser.dialect generic
 datafusion.sql_parser.enable_ident_normalization true
-datafusion.sql_parser.enable_options_value_normalization true
+datafusion.sql_parser.enable_options_value_normalization false
 datafusion.sql_parser.parse_float_as_decimal false
 datafusion.sql_parser.support_varchar_with_length true
 
@@ -351,7 +351,7 @@ datafusion.optimizer.skip_failed_rules false When set to true, the logical plan
 datafusion.optimizer.top_down_join_key_reordering true When set to true, the physical plan optimizer will run a top down process to reorder the join keys
 datafusion.sql_parser.dialect generic Configure the SQL dialect used by DataFusion's parser; supported values include: Generic, MySQL, PostgreSQL, Hive, SQLite, Snowflake, Redshift, MsSQL, ClickHouse, BigQuery, and Ansi.
 datafusion.sql_parser.enable_ident_normalization true When set to true, SQL parser will normalize ident (convert ident to lowercase when not quoted)
-datafusion.sql_parser.enable_options_value_normalization true When set to true, SQL parser will normalize options value (convert value to lowercase)
+datafusion.sql_parser.enable_options_value_normalization false When set to true, SQL parser will normalize options value (convert value to lowercase)
 datafusion.sql_parser.parse_float_as_decimal false When set to true, SQL parser will parse float as decimal type
 datafusion.sql_parser.support_varchar_with_length true If true, permit lengths for `VARCHAR` such as `VARCHAR(20)`, but ignore the length. If false, error if a `VARCHAR` with a length is specified. The Arrow type system does not have a notion of maximum string length and thus DataFusion can not enforce such limits.
 

datafusion/sqllogictest/test_files/set_variable.slt

@@ -93,10 +93,10 @@ datafusion.execution.coalesce_batches false
 statement ok
 set datafusion.catalog.information_schema = true
 
-statement error DataFusion error: Error parsing 1 as bool
+statement error DataFusion error: Error parsing '1' as bool
 SET datafusion.execution.coalesce_batches to 1
 
-statement error DataFusion error: Error parsing abc as bool
+statement error DataFusion error: Error parsing 'abc' as bool
 SET datafusion.execution.coalesce_batches to abc
 
 # set u64 variable
@@ -132,10 +132,10 @@ datafusion.execution.batch_size 2
 statement ok
 set datafusion.catalog.information_schema = true
 
-statement error DataFusion error: Error parsing -1 as usize
+statement error DataFusion error: Error parsing '-1' as usize
 SET datafusion.execution.batch_size to -1
 
-statement error DataFusion error: Error parsing abc as usize
+statement error DataFusion error: Error parsing 'abc' as usize
 SET datafusion.execution.batch_size to abc
 
 statement error External error: invalid digit found in string

docs/source/user-guide/configs.md

@@ -122,6 +122,6 @@ Environment variables are read during `SessionConfig` initialisation so they mus
 | datafusion.explain.show_schema                                          | false                     | When set to true, the explain statement will print schema information                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    |
 | datafusion.sql_parser.parse_float_as_decimal                            | false                     | When set to true, SQL parser will parse float as decimal type                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                            |
 | datafusion.sql_parser.enable_ident_normalization                        | true                      | When set to true, SQL parser will normalize ident (convert ident to lowercase when not quoted)                                                                                                                                                                                                                                                                                                                                                                                                                                                                           |
-| datafusion.sql_parser.enable_options_value_normalization                | true                      | When set to true, SQL parser will normalize options value (convert value to lowercase)                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   |
+| datafusion.sql_parser.enable_options_value_normalization                | false                     | When set to true, SQL parser will normalize options value (convert value to lowercase)                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   |
 | datafusion.sql_parser.dialect                                           | generic                   | Configure the SQL dialect used by DataFusion's parser; supported values include: Generic, MySQL, PostgreSQL, Hive, SQLite, Snowflake, Redshift, MsSQL, ClickHouse, BigQuery, and Ansi.                                                                                                                                                                                                                                                                                                                                                                                   |
 | datafusion.sql_parser.support_varchar_with_length                       | true                      | If true, permit lengths for `VARCHAR` such as `VARCHAR(20)`, but ignore the length. If false, error if a `VARCHAR` with a length is specified. The Arrow type system does not have a notion of maximum string length and thus DataFusion can not enforce such limits.                                                                                                                                                                                                                                                                                                    |