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

[I] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

alamb opened a new issue, #10782:
URL: https://github.com/apache/datafusion/issues/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  https://github.com/apache/datafusion/pull/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:
   1.  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`)
   2. The built in `Memory*` providers are in  `datafusion-catalog`
   3. The bult in `InformationSchema` providers are in `datafusion-catalog`
   4. The newly proposed `DynamicFileCatalog` in `datafusion-catalog` 
   5. (eventually) the LIstingTableProvider (which is by far the most complicated) moved to its own crate `datafusion-catalog-listing`
   
   ### 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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   newcomer here, so maybe i say something stupid, please bear with me
   These circular dependencies seem to revolve around `SessionState` which is big but also common needed.
   What about introducing an interface to that state, separating internal implementation (core) and others can access (API)?
   


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   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 


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   
   > 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](https://github.com/apache/datafusion/issues/13386#issue-2654282525)) 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:
   1. Try and complete moving the items listed in this issues description.
   1. 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:
   1. The `Memory*` providers: https://github.com/apache/datafusion/blob/4c1ec807fb70ef8abe96299760dd2faaa556a49c/datafusion/core/src/catalog_common/memory.rs
   2. The `InformationSchema` providers: https://github.com/apache/datafusion/blob/4c1ec807fb70ef8abe96299760dd2faaa556a49c/datafusion/core/src/catalog_common/information_schema.rs
   
   Perhaps we can file tickets for those items and I can link them to this epic? 
   
   
   
   
   
   
   


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > 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)


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   This graph is amazing btw, first it shows how tightly coupled we are, the second is it gives really nice understandable picture


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > 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
   
   
   


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   `SessionState` itself has implement `FunctionRegistry`, `OptimizerConfig`, are they similar to your idea?


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   @jayzhan211 not aware of any blockers, but not working on this right now, if this is what you're asking about.


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > 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?


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   Is it due to the complexity of `ListingTable` so it has it's own crate?


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > 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>` 👍 


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   I agree with this direction. But now this seems hard to achieve because `SchemaProvider` depends on `TableProvider` and `TableProvider` depends on `SessionState`.


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > datafusion-catalog: contains traits
   datafusion-catalog-basic
   
   ~datafusion-catalog~ datafusion-catalog-common: contains traits
   ~datafusion-catalog-basic~ datafusion-catalog: actual implementation
   🤔 
   
   Let's try moving SessionState, Catalog, TableProvider into `datafusion-catalog-common`


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   Update: https://github.com/apache/datafusion/pull/11403 has been merged and I think building SessionState is much nicer now


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > These 3 has no circular dependencies which mean we can/should move them out of core before moving catalog-common out of core.
   
   > PhysicalOptimizer
   
   I agree -- I think we can pull the physical optimzier rules into their own crate.
   
   The challenge would be any back dependencies (e.g. if any depend directly on execution plans in the core like `ParquetExec`.
   
   > RuntimeEnv
   
   I think this is already out of core (in `datafusion-execution`: https://github.com/apache/datafusion/blob/main/datafusion/execution/src/runtime_env.rs
   
   > SessionConfig
   
   Likewise, this is already in `datafusion-execution` and not in core: https://github.com/apache/datafusion/blob/main/datafusion/execution/src/config.rs#L94-L93


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   I think we need `datafusion-datasource` crate too. `TableProvider` can be a high level crate for `catalog` and `catalog-listing`


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > > 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.
   
   


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   I don't know what i am doing, but here is what i had in mind: https://github.com/apache/datafusion/pull/11516


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   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


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > 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 
   * `datafusion-catalog`: contains traits
   * `datafusion-catalog-basic` (not a good name`: `MemoryCatalog`, etc, `InformationCatalog` (potentially `DynamicFileCatalog` / https://github.com/apache/datafusion/pull/11035 from  @goldmedal )
   * `datafusion-catalog-listing`: `ListingTable`
   
   And we could put the individual datasources like
   * `datafusion-datasource-parquet`
   * `datafusion-datasource-csv`
   * `datafusion-datasource-json`
   * ...
   
   🤔 
   
   
   I think https://github.com/apache/datafusion/issues/11320 and https://github.com/apache/datafusion/pull/11403 from @Omega359  makes this easier


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   @jonathanc-n 
   The idea to extract out module is that you check the dependency by what crate is imported. 
   
   For example, if we need to move `CsvExec` out, we need to move `FileScanConfig` and `FileCompressionType` out too.
   To move `FileScanConfig` out, we need `PartitionedFile` to be moved out too. Then you will find out we need to move the whole datasource::listing out first. and so on.


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   Current dependencies for TableProvider that actually need instead of the whole SessionState
   
   QueryPlanner + ExecutionProps + SessionConfig + RuntimeEnv + FileFormat
   
   DataFrameTableProvider:  create_physical_plan
   MemTable: ExplainOptions + ExplainOptions
   ViewTable: create_physical_plan
   ListingTable: ExecutionOptions + RuntimeEnv + FileFormat
   SimpleCsvTable: create_physical_plan + TaskContext
   IndexTableProvider: create_physical_plan


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > We could try to move them out to a crate datasource first and maybe individual data source crate if required
   
   I think this would be ideal
   
   The parquet one in particular is quite large. The others are not as large 


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   Draft the dependency graph, incomplete
   
   ```mermaid
     graph TD;
         CatalogProvider --> TableProvider
         TableProvider --> SessionState
         TableProvider --> ExecutionPlan
         SessionState --> PhysicalOptimizer
         SessionState --> QueryPlanner
         SessionState --> CatalogProviderList
         SessionState --> TableFunction
         SessionState --> FileFormatFactory
         SessionState --> SessionConfig
         SessionState --> TableProviderFactory
         SessionState --> RuntimeEnv
         SessionState --> FunctionFactory
         TableFunction --> TableProvider
         FileFormatFactory --> FileFormat
         FileFormat --> SessionState
         FileFormat --> ExecutionPlan
         PhysicalOptimizer --> PartitionedFile
   ```
   
   Circular found
   
   ```mermaid
     graph TD;
         CatalogProvider --> TableProvider
         TableProvider --> SessionState
         SessionState --> TableFunction
         TableFunction --> TableProvider
   ```
   


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #10782: [Epic] Extract catalog functionality from the core to make it more modular
URL: https://github.com/apache/datafusion/issues/10782


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   I can work on this 👀 


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   Yeah I agree, should we be putting priority on this soon? or should this be visited back another time?


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   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


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   Shall we file a ticket / epic to pull physical optimizer out?


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   I had a proposal in #11420 to minimize dependency with small struct, I think replacing it with another smaller trait are similar idea, both looks good to me


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   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 strictly necessary.


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   I think we need to find a way to move `catalog_list: Arc<dyn CatalogProviderList>` and `table_factories: HashMap<String, Arc<dyn TableProviderFactory>>` out of `SessionState`


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   @findepi Is there any blocker of moving catalog / datasource out of core?


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   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


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   The dependencies in core is quite complex, take a note for it
   
   Draft the dependency graph, incomplete
   
   ```mermaid
     graph TD;
         CatalogProvider --> TableProvider
         TableProvider --> SessionState
         TableProvider --> ExecutionPlan
         SessionState --> PhysicalOptimizer
         SessionState --> QueryPlanner
         QueryPlanner --> ExecutionPlan
         SessionState --> CatalogProviderList
         SessionState --> TableFunction
         SessionState --> FileFormatFactory
         SessionState --> SessionConfig
         SessionState --> TableProviderFactory
         SessionState --> RuntimeEnv
         SessionState --> FunctionFactory
         FunctionFactory --> TableProvider
         TableFunction --> TableProvider
         FileFormatFactory --> FileFormat
         FileFormat --> SessionState
         FileFormat --> ExecutionPlan
         PhysicalOptimizer --> PartitionedFile
        CatalogProviderList --> CatalogProvider
   ```
   
   Circular found. It means they should be in the same crate
   
   ```mermaid
     graph TD;
         CatalogProvider --> TableProvider
         TableProvider --> SessionState
         SessionState --> TableFunction
         TableFunction --> TableProvider
   ```
   
   ```mermaid
     graph TD;
         CatalogProvider --> TableProvider
         TableProvider --> SessionState
         SessionState --> CatalogProviderList
         CatalogProviderList --> CatalogProvider
   ```
   
   ```mermaid
     graph TD;
         CatalogProvider --> TableProvider
         TableProvider --> SessionState
         SessionState --> FileFormatFactory
         FileFormatFactory --> FileFormat
        FileFormat --> SessionState
   ```
   
   CatalogProvider + TableProvider + SessionsState + FileFormat


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > And we could put the individual datasources like
   datafusion-datasource-parquet
   datafusion-datasource-csv
   datafusion-datasource-json
   ...
   
   We could try to move them out to a crate `datasource` first and maybe individual data source crate if required


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > 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: https://github.com/apache/datafusion/pull/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
   
   


-- 
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] [Epic] Extract catalog functionality from the core to make it more modular [datafusion]

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

   > Oh i see, should we be putting priority on this soon? or should this be visited back another time?
   
   I will help to review with this.


-- 
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