ozankabak commented on code in PR #9723:
URL: https://github.com/apache/datafusion/pull/9723#discussion_r1633047485


##########
datafusion/sql/src/statement.rs:
##########
@@ -850,7 +850,16 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                     return plan_err!("Unsupported Value in COPY statement {}", 
value);
                 }
             };
-            options.insert(key.to_lowercase(), value_string.to_lowercase());
+            if !(&key.contains('.')) {
+                // If config does not belong to any namespace, assume it is
+                // a format option and apply the format prefix for backwards
+                // compatibility.
+
+                let renamed_key = format!("format.{}", key);
+                options.insert(renamed_key.to_lowercase(), 
value_string.to_lowercase());

Review Comment:
   Having keys lowercased makes sense for standardization purposes, but I'm not 
sure why we are also lowercasing option values. It may either be because (1) of 
the same standardization concerns, or (2) by mistake.
   
   The most flexible way would be to make value (not key) standardization logic 
customizable, with the default being lowercase. If we had that, users like 
@xinlifoobar would be able to avoid standardization modifications. Other users 
who want other kinds of standardizations would be able to override it etc.
   
   @tinfoil-knight, would you be interested in thinking about how we can do 
this?



-- 
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