[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5477: Minor: Move TableProviderFactories up out of RuntimeEnv and into Sess…

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #5477:
URL: https://github.com/apache/arrow-datafusion/pull/5477#discussion_r1125553764


##########
datafusion/core/src/execution/context.rs:
##########
@@ -278,6 +282,15 @@ impl SessionContext {
         self.session_id.clone()
     }
 
+    /// Return the [`TableFactoryProvider`] that is registered for the

Review Comment:
   ```suggestion
       /// Return the [`TableProviderFactory`] that is registered for the
   ```



##########
datafusion/core/src/execution/runtime_env.rs:
##########
@@ -129,24 +114,12 @@ pub struct RuntimeConfig {
     pub memory_pool: Option<Arc<dyn MemoryPool>>,
     /// ObjectStoreRegistry to get object store based on url
     pub object_store_registry: Arc<ObjectStoreRegistry>,
-    /// Custom table factories for things like deltalake that are not part of core datafusion
-    pub table_factories: HashMap<String, Arc<dyn TableProviderFactory>>,
 }
 
 impl RuntimeConfig {
     /// New with default values
     pub fn new() -> Self {
-        let mut table_factories: HashMap<String, Arc<dyn TableProviderFactory>> =
-            HashMap::new();
-        table_factories.insert("PARQUET".into(), Arc::new(ListingTableFactory::new()));

Review Comment:
   this construction now happens as part of creating SessionState



##########
datafusion/core/tests/sql/create_drop.rs:
##########
@@ -126,12 +129,14 @@ async fn create_custom_table() -> Result<()> {
 
 #[tokio::test]
 async fn create_external_table_with_ddl() -> Result<()> {
-    let mut cfg = RuntimeConfig::new();
-    cfg.table_factories
-        .insert("MOCKTABLE".to_string(), Arc::new(TestTableFactory {}));
+    let cfg = RuntimeConfig::new();
     let env = RuntimeEnv::new(cfg).unwrap();
     let ses = SessionConfig::new();
-    let ctx = SessionContext::with_config_rt(ses, Arc::new(env));
+    let mut state = SessionState::with_config_rt(ses, Arc::new(env));

Review Comment:
   Here is the new pattern of how to add a table provider factory



##########
datafusion/core/src/execution/runtime_env.rs:
##########
@@ -22,10 +22,7 @@ use crate::{
     error::Result,
     execution::disk_manager::{DiskManager, DiskManagerConfig},
 };
-use std::collections::HashMap;
 
-use crate::datasource::datasource::TableProviderFactory;

Review Comment:
   The whole point of the PR is to remove the `datasource` dependency from the `runtime_env` module (as I am trying to move the `runtime_env` module into a new crate, `datafusion-execution`, that does not depend on `datasource`)
   
   I plan to move object_store registry in a separate PR to keep the reviews smaller
   
   



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

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