You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/08/30 14:56:17 UTC

[GitHub] [arrow] jorgecarleitao commented on a change in pull request #8079: ARROW-9815: [Rust][DataFusion] Remove the use of Arc/Mutex to protect plan time structures

jorgecarleitao commented on a change in pull request #8079:
URL: https://github.com/apache/arrow/pull/8079#discussion_r479779945



##########
File path: rust/datafusion/src/execution/context.rs
##########
@@ -255,26 +253,17 @@ impl ExecutionContext {
 
     /// Register a table using a custom TableProvider so that it can be referenced from SQL
     /// statements executed against this context.
-    pub fn register_table(
-        &mut self,
-        name: &str,
-        provider: Box<dyn TableProvider + Send + Sync>,
-    ) {
-        let mut ctx_state = self.state.lock().expect("failed to lock mutex");
-        ctx_state.datasources.insert(name.to_string(), provider);
+    pub fn register_table(&mut self, name: &str, provider: Box<dyn TableProvider>) {

Review comment:
       I think that we aimed for `TableProvider` to be `Send + Sync` on ARROW-9425 , so that others could use `ExecutionContext` on multi-threaded environments such as Python.
   
   See e.g. the usage [here](https://github.com/jorgecarleitao/datafusion-python/blob/master/src/lib.rs#L57). I think that that code only compiles if `ExecutionContext` is sharable between threads.
   
   




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

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