andygrove commented on a change in pull request #8079:
URL: https://github.com/apache/arrow/pull/8079#discussion_r479790490
##########
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 agree that `TableProvider` no longer needs `Send` and `Sync` now that
it is just used during planning and no longer performs execution.
I don't think changing this will affect whether ExecutionContext can be sent
between threads but I don't have time today to actually confirm that.
If we do want to be sure that `ExecutionContext` can be shared between
threads, we should add a unit test for that.
Hopefully, I'll be able to find some time this evening to look at this some
more.
----------------------------------------------------------------
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:
[email protected]
