You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Epic] Extract catalog functionality from the core to make it more modular #10782
Is your feature request related to a problem or challenge?
As @goldmedal started trying to move the DynamicFileProvider so others could use it in #10745 I think it is clear that there is not a good way to add additional catalog support in the core without everything being intertwined.
Thus I think we should try and extract the different catalog providers out of datafusion core so it it easier
Describe the solution you'd like
I suggest the following final layout:
traits like CatalogProvider, SchemaProvider, etc in a new crate datafusion-catalog (since these traits rely on table provider, etc I think this can't be in datafusion-common or datafusion-expr)
The built in Memory* providers are in datafusion-catalog
The bult in InformationSchema providers are in datafusion-catalog
The newly proposed DynamicFileCatalog in datafusion-catalog
(eventually) the LIstingTableProvider (which is by far the most complicated) moved to its own crate datafusion-catalog-listing
I agree with this direction. But now this seems hard to achieve because SchemaProvider depends on TableProvider and TableProvider depends on SessionState.
Thanks @alamb for starting this discussion.
If I understood correctly there is a use case for using catalog abstractions/implementation but without datafusion core?
Like @lewiszlw correctly mentioned, we got some coupling between providers and the core. I'm just trying to understand the usecase when providers needed without the core
Is it due to the complexity of ListingTable so it has it's own crate? If they have common things then it is better to organize them into one crate. If ListingTable is so different than others, it is nice to have an independent crate
If I understood correctly there is a use case for using catalog abstractions/implementation but without datafusion core?
In my mind the real use usecase is to more easily use datafusion without having to bring in all the dependencies of LIstingTable (like parquet, avro, json, etc)
So the real usecase is getting ListingTable out of the core. But since the catalog API is in the core now there is no way to get ListingTable out of the core without also first moving the catalog API
Is it due to the complexity of ListingTable so it has it's own crate? If they have common things then it is better to organize them into one crate. If ListingTable is so different than others, it is nice to have an independent crate
I think both the complexity of ListingTable but also because if its dependency tree (e.g. parquet-rs and avro and json and object_store and ...)
For use cases like WASM it is quite messy to have the API split up like it currently is
I agree with this direction. But now this seems hard to achieve because SchemaProvider depends on TableProvider and TableProvider depends on SessionState.
I agree @lewiszlw -- well put. I made a first PR to start detangling things here: #10794 (it just splits SessionState into its own module)
Longer term we would have to figure out where SessionState would live (it still depends on several things in the core crate like datasource::provider and datasource::function 🤔
Maybe we could look into splitting out datafusion-datasource / datafusion-datasource-parquet / datafusion-datsource-avro, etc -- I don't have time to drive this at the moment but would be interested in helping anyone who did
If I understood correctly there is a use case for using catalog abstractions/implementation but without datafusion core?
In my mind the real use usecase is to more easily use datafusion without having to bring in all the dependencies of LIstingTable (like parquet, avro, json, etc)
So the real usecase is getting ListingTable out of the core. But since the catalog API is in the core now there is no way to get ListingTable out of the core without also first moving the catalog API
Hm... they probably thrive to have their own readers/writes perhaps other than arrow-rs implementation, that makes sense for me. And yes, if DF stands for extensibility we should make this happen. Not sure how difficult that can be though. We probably need to start with replacing core abstractions with traits instead of implementations to decouple it.
Hm... they probably thrive to have their own readers/writes perhaps other than arrow-rs implementation, that makes sense for me. And yes, if DF stands for extensibility we should make this happen. Not sure how difficult that can be though. We probably need to start with replacing core abstractions with traits instead of implementations to decouple it.
Yes something like this -- I think most of the traits already exist (e.g. CatalogProvider) but figuring out how to decouple SessionState (which is referred to by CatalogProvider is the trickiest bit I think)
There is a dead lock that block us from extracting out catalog 😆
To move CatalogProvider out we need to move TableProvider to datafusion-datasource
To move TableProvider out we need to avoid dependency on SessionState for scan function, but there is CatalogProviderList in SessionState
SessionState contains tons of concept that are not all necessary for scan
pubstructSessionState{/// A unique UUID that identifies the sessionsession_id:String,/// Responsible for analyzing and rewrite a logical plan before optimizationanalyzer:Analyzer,/// Provides support for customising the SQL planner, e.g. to add support for custom operators like `->>` or `?`expr_planners:Vec<Arc<dynExprPlanner>>,/// Responsible for optimizing a logical planoptimizer:Optimizer,/// Responsible for optimizing a physical execution planphysical_optimizers:PhysicalOptimizer,/// Responsible for planning `LogicalPlan`s, and `ExecutionPlan`query_planner:Arc<dynQueryPlanner + Send + Sync>,/// Collection of catalogs containing schemas and ultimately TableProviderscatalog_list:Arc<dynCatalogProviderList>,/// Table Functionstable_functions:HashMap<String,Arc<TableFunction>>,/// Scalar functions that are registered with the contextscalar_functions:HashMap<String,Arc<ScalarUDF>>,/// Aggregate functions registered in the contextaggregate_functions:HashMap<String,Arc<AggregateUDF>>,/// Window functions registered in the contextwindow_functions:HashMap<String,Arc<WindowUDF>>,/// Deserializer registry for extensions.serializer_registry:Arc<dynSerializerRegistry>,/// Holds registered external FileFormat implementationsfile_formats:HashMap<String,Arc<dynFileFormatFactory>>,/// Session configurationconfig:SessionConfig,/// Table optionstable_options:TableOptions,/// Execution propertiesexecution_props:ExecutionProps,/// TableProviderFactories for different file formats.////// Maps strings like "JSON" to an instance of [`TableProviderFactory`]////// This is used to create [`TableProvider`] instances for the/// `CREATE EXTERNAL TABLE ... STORED AS <FORMAT>` for custom file/// formats other than those built into DataFusion////// [`TableProvider`]: crate::datasource::provider::TableProvidertable_factories:HashMap<String,Arc<dynTableProviderFactory>>,/// Runtime environmentruntime_env:Arc<RuntimeEnv>,/// [FunctionFactory] to support pluggable user defined function handler.////// It will be invoked on `CREATE FUNCTION` statements./// thus, changing dialect o PostgreSql is requiredfunction_factory:Option<Arc<dynFunctionFactory>>,}
To move TableProvider out we need to avoid dependency on SessionState for scan function, but there is CatalogProviderList in SessionState
🤔 we could potentially move SessionState into the catalog crate
In fact the more I think about it, it seems to make some sort of sense to have a crate with the catalog traits / interfaces such as SessionState, Catalog, TableProvider that can be shared broadly.
Then we could make separate crates that actually had the implementations like
If we're planning to replace SessionState, I'd like the minimum dependencies. This helps avoid potential circular dependency issues that might arise from other components, even if they're not necessary.
5. (eventually) the LIstingTableProvider (which is by far the most complicated) moved to its own crate datafusion-catalog-listing
This will require moving file formats (datafusion/core/src/datasource/file_format) too.
I think this might be a good fit for a yet another top level module. What do you think?
@jonathanc-n
The main idea behind extracting a module is to identify dependencies by looking at which crates are imported.
For example, if we want to move CsvExec out, we’ll also need to move FileScanConfig and FileCompressionType. To extract FileScanConfig, we’ll need to move PartitionedFile as well. This process will reveal that we must move the entire datasource::listing module first, and whenever we need SessionState, replace it with trait Session
Oh i see, should we be putting priority on this soon? or should this be visited back another time?
In my opinion, this would qualify as "Things that make DataFusion easier to work with" (details here) and thus I will also try and find time to help this project along (both reviews as well as project planning)
I have been annoyed recently at the amount of time it takes to compile datafusion
For example, if we want to move CsvExec out, we’ll also need to move FileScanConfig and FileCompressionType. To extract FileScanConfig, we’ll need to move PartitionedFile as well. This process will reveal that we must move the entire datasource::listing module first, and whenever we need SessionState, replace it with trait Session
I went through the thread and I couldn't figure out if we had identified/agreed on a set of issues that compose this epic @alamb
Here is what I suggest:
Try and complete moving the items listed in this issues description.
Do NOT try and move ListingTableProider as part of this project (that is big enough that I think we should make its own epic for the reasons @jonathanc-n mentions)
I took a quick look and I think there are two things we could move to datafusion-catalog to complete this ticket:
Hi, I am working on moving InformationSchema into the datafusion-catalog.
This would require moving core/src/datasource/streaming.rs (StreaminTable) to some place out of the core,
I think moving it to datafusion-catalog should work fine since that's where the orignal TableProvider resides. So, that's what I am going ahead with, in the meantime let me know if you think otherwise.
Thanks
Hi, I am working on moving InformationSchema into the datafusion-catalog. This would require moving core/src/datasource/streaming.rs (StreaminTable) to some place out of the core, I think moving it to datafusion-catalog should work fine since that's where the orignal TableProvider resides. So, that's what I am going ahead with, in the meantime let me know if you think otherwise. Thanks
I think it makes sense to move StreamingTable as it is a relatively thin wrapper around Arc<dyn PartitionStream> 👍
Activity
alamb commentedon Jun 3, 2024
If this seems like a reasonable idea to people I will file tickets to break down the work
cc @andygrove @jayzhan211 @comphead @mustafasrepo for your thoughts
lewiszlw commentedon Jun 4, 2024
I agree with this direction. But now this seems hard to achieve because
SchemaProviderdepends onTableProviderandTableProviderdepends onSessionState.comphead commentedon Jun 4, 2024
Thanks @alamb for starting this discussion.
If I understood correctly there is a use case for using catalog abstractions/implementation but without datafusion core?
Like @lewiszlw correctly mentioned, we got some coupling between providers and the core. I'm just trying to understand the usecase when providers needed without the core
jayzhan211 commentedon Jun 5, 2024
Is it due to the complexity of
ListingTableso it has it's own crate? If they have common things then it is better to organize them into one crate. If ListingTable is so different than others, it is nice to have an independent cratealamb commentedon Jun 5, 2024
In my mind the real use usecase is to more easily use datafusion without having to bring in all the dependencies of LIstingTable (like parquet, avro, json, etc)
So the real usecase is getting ListingTable out of the core. But since the catalog API is in the core now there is no way to get ListingTable out of the core without also first moving the catalog API
I think both the complexity of ListingTable but also because if its dependency tree (e.g. parquet-rs and avro and json and object_store and ...)
For use cases like
WASMit is quite messy to have the API split up like it currently isalamb commentedon Jun 5, 2024
I agree @lewiszlw -- well put. I made a first PR to start detangling things here: #10794 (it just splits
SessionStateinto its own module)Longer term we would have to figure out where
SessionStatewould live (it still depends on several things in the core crate likedatasource::provideranddatasource::function🤔Maybe we could look into splitting out
datafusion-datasource/datafusion-datasource-parquet/datafusion-datsource-avro, etc -- I don't have time to drive this at the moment but would be interested in helping anyone who didcomphead commentedon Jun 5, 2024
Hm... they probably thrive to have their own readers/writes perhaps other than arrow-rs implementation, that makes sense for me. And yes, if DF stands for extensibility we should make this happen. Not sure how difficult that can be though. We probably need to start with replacing core abstractions with traits instead of implementations to decouple it.
alamb commentedon Jun 6, 2024
Yes something like this -- I think most of the traits already exist (e.g.
CatalogProvider) but figuring out how to decouple SessionState (which is referred to byCatalogProvideris the trickiest bit I think)jayzhan211 commentedon Jul 11, 2024
I'm interesting on how datafusion can support vector index like Duckdb's VSS extension
Given the comment, it seems the issue here might be helpful 🤔 ?
I can work on this 👀
jayzhan211 commentedon Jul 12, 2024
There is a dead lock that block us from extracting out catalog 😆
CatalogProviderout we need to moveTableProvidertodatafusion-datasourceTableProviderout we need to avoid dependency onSessionStateforscanfunction, but there isCatalogProviderListinSessionStateSessionState contains tons of concept that are not all necessary for
scanalamb commentedon Jul 12, 2024
🤔 we could potentially move SessionState into the catalog crate
In fact the more I think about it, it seems to make some sort of sense to have a crate with the catalog traits / interfaces such as
SessionState,Catalog,TableProviderthat can be shared broadly.Then we could make separate crates that actually had the implementations like
datafusion-catalog: contains traitsdatafusion-catalog-basic(not a good name:MemoryCatalog, etc,InformationCatalog(potentiallyDynamicFileCatalog` / Introduce theDynamicFileCatalogindatafusion-catalog#11035 from @goldmedal )datafusion-catalog-listing:ListingTableAnd we could put the individual datasources like
datafusion-datasource-parquetdatafusion-datasource-csvdatafusion-datasource-json🤔
I think #11320 and #11403 from @Omega359 makes this easier
jayzhan211 commentedon Jul 13, 2024
datafusion-catalogdatafusion-catalog-common: contains traitsdatafusion-catalog-basicdatafusion-catalog: actual implementation🤔
Let's try moving SessionState, Catalog, TableProvider together into
datafusion-catalog-commonor common-runtimealamb commentedon Jul 15, 2024
Update: #11403 has been merged and I think building SessionState is much nicer now
9 remaining items
jayzhan211 commentedon Jul 17, 2024
If we're planning to replace
SessionState, I'd like the minimum dependencies. This helps avoid potential circular dependency issues that might arise from other components, even if they're not necessary.findepi commentedon Aug 20, 2024
This will require moving file formats (
datafusion/core/src/datasource/file_format) too.I think this might be a good fit for a yet another top level module. What do you think?
jayzhan211 commentedon Aug 20, 2024
We could try to move them out to a crate
datasourcefirst and maybe individual data source crate if requiredalamb commentedon Aug 20, 2024
I think this would be ideal
The parquet one in particular is quite large. The others are not as large
jayzhan211 commentedon Sep 2, 2024
@findepi Is there any blocker of moving catalog / datasource out of core?
findepi commentedon Sep 2, 2024
@jayzhan211 not aware of any blockers, but not working on this right now, if this is what you're asking about.
jayzhan211 commentedon Nov 9, 2024
@jonathanc-n
The main idea behind extracting a module is to identify dependencies by looking at which crates are imported.
For example, if we want to move CsvExec out, we’ll also need to move FileScanConfig and FileCompressionType. To extract FileScanConfig, we’ll need to move PartitionedFile as well. This process will reveal that we must move the entire datasource::listing module first, and whenever we need
SessionState, replace it with traitSessionjonathanc-n commentedon Nov 9, 2024
Oh i see, should we be putting priority on this soon? or should this be visited back another time?
jayzhan211 commentedon Nov 9, 2024
I will help to review with this.
edmondop commentedon Nov 11, 2024
I went through the thread and I couldn't figure out if we had identified/agreed on a set of issues that compose this epic @alamb
alamb commentedon Nov 13, 2024
In my opinion, this would qualify as "Things that make DataFusion easier to work with" (details here) and thus I will also try and find time to help this project along (both reviews as well as project planning)
I have been annoyed recently at the amount of time it takes to compile datafusion
Here is what I suggest:
ListingTableProideras part of this project (that is big enough that I think we should make its own epic for the reasons @jonathanc-n mentions)I took a quick look and I think there are two things we could move to
datafusion-catalogto complete this ticket:Memory*providers: https://github.com/apache/datafusion/blob/4c1ec807fb70ef8abe96299760dd2faaa556a49c/datafusion/core/src/catalog_common/memory.rsInformationSchemaproviders: https://github.com/apache/datafusion/blob/4c1ec807fb70ef8abe96299760dd2faaa556a49c/datafusion/core/src/catalog_common/information_schema.rsPerhaps we can file tickets for those items and I can link them to this epic?
logan-keede commentedon Jan 29, 2025
Hi, I am working on moving
InformationSchemainto thedatafusion-catalog.This would require moving
core/src/datasource/streaming.rs(StreaminTable) to some place out of the core,I think moving it to
datafusion-catalogshould work fine since that's where the orignalTableProviderresides. So, that's what I am going ahead with, in the meantime let me know if you think otherwise.Thanks
alamb commentedon Jan 29, 2025
I think it makes sense to move
StreamingTableas it is a relatively thin wrapper aroundArc<dyn PartitionStream>👍