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



##########
File path: rust/datafusion/src/execution/context.rs
##########
@@ -477,9 +466,9 @@ impl ExecutionConfig {
 /// Execution context for registering data sources and executing queries
 pub struct ExecutionContextState {
     /// Data sources that are registered with the context
-    pub datasources: Box<HashMap<String, Box<dyn TableProvider + Send + 
Sync>>>,
+    pub datasources: HashMap<String, Box<dyn TableProvider + Send + Sync>>,
     /// Scalar functions that are registered with the context
-    pub scalar_functions: Box<HashMap<String, Box<ScalarFunction>>>,
+    pub scalar_functions: HashMap<String, Arc<ScalarFunction>>,

Review comment:
       just for the sake of my understanding (prob a very basic CS question): 
why shouldn't we use simply `ScalarFunction` here? Is so that we can safely 
share this object with others without having to share the whole `HashMap`?




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


Reply via email to