andygrove commented on code in PR #4227:
URL: https://github.com/apache/arrow-datafusion/pull/4227#discussion_r1024278856
##########
datafusion/core/src/datasource/listing_table_factory.rs:
##########
@@ -52,27 +58,61 @@ impl TableProviderFactory for ListingTableFactory {
state: &SessionState,
cmd: &CreateExternalTable,
) -> datafusion_common::Result<Arc<dyn TableProvider>> {
- let file_extension = self.file_type.get_ext();
+ let file_compression_type =
+ match
FileCompressionType::from_str(cmd.file_compression_type.as_str()) {
+ Ok(t) => t,
+ Err(_) => Err(DataFusionError::Execution(
+ "Only known FileCompressionTypes can be
ListingTables!".to_string(),
+ ))?,
+ };
+
+ let file_type = match FileType::from_str(cmd.file_type.as_str()) {
+ Ok(t) => t,
+ Err(_) => Err(DataFusionError::Execution(
+ "Only known FileTypes can be ListingTables!".to_string(),
+ ))?,
+ };
Review Comment:
Let's include some context in the error message so the user knows what they
used that isn't supported. Also changes to use `map_err` to remove the need to
match here.
```suggestion
let file_compression_type = FileCompressionType::from_str(
cmd.file_compression_type.as_str(),
)
.map_err(|_| {
DataFusionError::Execution(format!(
"Unknown FileCompressionType {}",
cmd.file_compression_type.as_str()
))
})?;
let file_type =
FileType::from_str(cmd.file_type.as_str()).map_err(|_| {
DataFusionError::Execution(format!("Unknown FileType {}",
cmd.file_type))
})?;
```
--
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...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
