tinfoil-knight commented on code in PR #9744:
URL: https://github.com/apache/arrow-datafusion/pull/9744#discussion_r1536234244
##########
datafusion/sql/src/statement.rs:
##########
@@ -866,12 +866,16 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
FileType::from_str(&file_type).map_err(|_| {
DataFusionError::Configuration(format!("Unknown FileType {}",
file_type))
})?
+ } else if let Some(format) = options.remove("format") {
+ // try to infer file format from the "format" key in options
+ FileType::from_str(&format)
+ .map_err(|e| DataFusionError::Configuration(format!("{}", e)))?
} else {
let e = || {
DataFusionError::Configuration(
- "Format not explicitly set and unable to get file extension!
Use STORED AS to define file format."
- .to_string(),
- )
+ "Format not explicitly set and unable to get file
extension! Use STORED AS to define file format."
+ .to_string(),
+ )
Review Comment:
One thing to note is that, currently, order of priority for resolving the
`file_type` is:
1. `STORED AS`
2. Legacy: `COPY {} TO {} OPTIONS (format <>)`
3. File extension
Let me know if we care about the order of resolution & if this requires any
change.
##########
datafusion/sql/src/statement.rs:
##########
@@ -867,26 +867,32 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
DataFusionError::Configuration(format!("Unknown FileType {}",
file_type))
})?
} else {
- let e = || {
- DataFusionError::Configuration(
- "Format not explicitly set and unable to get file extension!
Use STORED AS to define file format."
- .to_string(),
- )
- };
- // try to infer file format from file extension
- let extension: &str = &Path::new(&statement.target)
- .extension()
- .ok_or_else(e)?
- .to_str()
- .ok_or_else(e)?
- .to_lowercase();
-
- FileType::from_str(extension).map_err(|e| {
- DataFusionError::Configuration(format!(
- "{}. Use STORED AS to define file format.",
- e
- ))
- })?
+ if let Some(format) = options.remove("format") {
+ // try to infer file format from the "format" key in options
+ FileType::from_str(&format)
+ .map_err(|e| DataFusionError::Configuration(format!("{}",
e)))?
+ } else {
+ let e = || {
+ DataFusionError::Configuration(
+ "Format not explicitly set and unable to get file
extension! Use STORED AS to define file format."
+ .to_string(),
+ )
+ };
+ // try to infer file format from file extension
+ let extension: &str = &Path::new(&statement.target)
+ .extension()
+ .ok_or_else(e)?
+ .to_str()
+ .ok_or_else(e)?
+ .to_lowercase();
+
+ FileType::from_str(extension).map_err(|e| {
+ DataFusionError::Configuration(format!(
+ "{}. Use STORED AS to define file format.",
+ e
+ ))
+ })?
+ }
Review Comment:
One thing to note is that, currently, order of priority for resolving the
`file_type` is:
1. `STORED AS`
2. Legacy: `COPY {} TO {} OPTIONS (format <>)`
3. File extension
Let me know if we care about the order of resolution & if this requires any
change.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
