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

[GitBox <gi...@apache.org>]

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



##########
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:
       The table provider is used during single-thread planning (not multi-threaded execution) and thus there is no reason for the table provider to be `Send` and `Sync` anymore

##########
File path: rust/datafusion/src/execution/context.rs
##########
@@ -94,7 +96,7 @@ use crate::sql::{
 /// ```
 pub struct ExecutionContext {
     /// Internal state for the context
-    pub state: Arc<Mutex<ExecutionContextState>>,

Review comment:
       The main aim of this PR is to remove this `Arc<Mutex<>>` -- the rest of the PR is a result of that change




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