findepi commented on code in PR #11516:
URL: https://github.com/apache/datafusion/pull/11516#discussion_r1686655186
##########
datafusion/core/src/datasource/listing_table_factory.rs:
##########
@@ -49,16 +49,18 @@ impl ListingTableFactory {
impl TableProviderFactory for ListingTableFactory {
async fn create(
&self,
- state: &SessionState,
+ state: &dyn CatalogSession,
cmd: &CreateExternalTable,
) -> Result<Arc<dyn TableProvider>> {
- let file_format = state
+ // TODO remove downcast_ref from here. Should file format factory be
an extension to session state?
Review Comment:
FWIW, `IcebergTableProvider` from https://github.com/apache/iceberg-rust
https://github.com/apache/iceberg-rust/blob/bbd042da5835b3e2e8ce7c05d45377c367464efd/crates/integrations/datafusion/src/table.rs#L76-L88
doesn't use the session at all.
i would be tempted to think that the pluggable FileFormat is equivalent to
implementing Hive table format, where file format is underspecified and
pluggable. Hive table format is super important industry-wide and maybe it
could deserve it's own module (crate). Then, the pluggable file format
functionality could be its implementation detail and not necessarily part of DF
core.
@jayzhan211 @alamb please correct me if i am thinking about this in a wrong
way
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
