You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "waruto210 (via GitHub)" <gi...@apache.org> on 2024/09/07 17:08:25 UTC

[I] Consider using the FileFormat::get_ext when creating ListingOptions [datafusion]

waruto210 opened a new issue, #12378:
URL: https://github.com/apache/datafusion/issues/12378

   ### Is your feature request related to a problem or challenge?
   
   When I use the following code to attempt creating external tables using SQL and Rust API respectively, a "Corrupt footer" error occurs. Eventually, I discovered that this was because there were files of other formats in the `partitioned` directory, but DataFusion was reading them as data files. I think this behavior is confusing, because the SQL specifies the Parquet format, and the Rust code also creates a new `ParquetFormat`. 
   ```sql
   CREATE EXTERNAL TABLE hits
   STORED AS PARQUET
   LOCATION 'partitioned/';
   ```
   ```rust
     let mut opts = TableParquetOptions::default();
     opts.set("pushdown_filters", "true").unwrap();
     let format = ParquetFormat::new().with_options(opts);
     let options = ListingOptions::new(Arc::new(format))
         .with_table_partition_cols(vec![("A".to_owned(), DataType::Int32)]);
     ctx.register_listing_table(
         "hits",
         "partitioned/",
         options,
         None,
         None,
     )
     .await
   ```
   
   ### Describe the solution you'd like
   
   Currently, when creating ListingOptions using format, `format.get_ext` is not called. I believe `format.get_ext` should be used as the default file extension filter, which would also better align with the definition of `FileFormat`trait. I you agree with me, I could submit a pr to make that change.
   
   ```rust
   impl ListingOptions {
     /// Creates an options instance with the given format
     /// Default values:
     /// - no file extension filter
     /// - no input partition to discover
     /// - one target partition
     /// - stat collection
     pub fn new(format: Arc<dyn FileFormat>) -> Self {
         Self {
             file_extension: String::new(),
             format,
             table_partition_cols: vec![],
             collect_stat: true,
             target_partitions: 1,
             file_sort_order: vec![],
         }
     }
   ```
   
   ### Describe alternatives you've considered
   
   _No response_
   
   ### Additional context
   
   _No response_


-- 
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-unsubscribe@datafusion.apache.org.apache.org

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


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

Re: [I] Consider using the FileFormat::get_ext when creating ListingOptions [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #12378: Consider using the FileFormat::get_ext when creating ListingOptions
URL: https://github.com/apache/datafusion/issues/12378


-- 
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-unsubscribe@datafusion.apache.org

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


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

Re: [I] Consider using the FileFormat::get_ext when creating ListingOptions [datafusion]

Posted by "waruto210 (via GitHub)" <gi...@apache.org>.
waruto210 commented on issue #12378:
URL: https://github.com/apache/datafusion/issues/12378#issuecomment-2336552953

   @alamb PTAL


-- 
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-unsubscribe@datafusion.apache.org

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


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

Re: [I] Consider using the FileFormat::get_ext when creating ListingOptions [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #12378:
URL: https://github.com/apache/datafusion/issues/12378#issuecomment-2338101059

   > I you agree with me, I could submit a pr to make that change.
   
   This proposal makes sense to me. Thank you @waruto210 


-- 
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-unsubscribe@datafusion.apache.org

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


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