alamb commented on code in PR #13576:
URL: https://github.com/apache/datafusion/pull/13576#discussion_r1893256755


##########
datafusion/common/src/config.rs:
##########
@@ -211,12 +221,15 @@ config_namespace! {
         /// When set to true, SQL parser will normalize ident (convert ident 
to lowercase when not quoted)
         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
+        /// When set to true, SQL parser will normalize options value (convert 
value to lowercase).
+        /// Note that this option is ignored and will be removed in the 
future. All case-insensitive values
+        /// are normalized automatically.
+        pub enable_options_value_normalization: bool, warn = 
"`enable_options_value_normalization` is deprecated and ignored", default = 
false

Review Comment:
   the use of the warning is quite cool
   
   However, it isn't shown by default in datafusion-cli FWIW
   
   ```
   > set datafusion.sql_parser.enable_options_value_normalization = true;
   0 row(s) fetched.
   Elapsed 0.001 seconds.
   ```
   
   I had to run it with RUST_LOG=info
   
   ```
   RUST_LOG=info cargo run
       Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.13s
        Running `target/debug/datafusion-cli`
   DataFusion CLI v43.0.0
   > set datafusion.sql_parser.enable_options_value_normalization = true;
   [2024-12-19T23:23:40Z WARN  datafusion_common::config] 
`enable_options_value_normalization` is deprecated and ignored
   0 row(s) fetched.
   Elapsed 0.007 seconds
   ```
   



##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -161,70 +160,6 @@ fn parse_ident_normalization() {
     }
 }
 
-#[test]
-fn test_parse_options_value_normalization() {

Review Comment:
   Could you please put this test back and updated it so that it passes? That 
way we can reason about / more easily see the change in behavior introduced by 
this PR 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to