mingmwang commented on a change in pull request #2029:
URL: https://github.com/apache/arrow-datafusion/pull/2029#discussion_r830775266
##########
File path: datafusion/src/execution/context.rs
##########
@@ -208,6 +186,11 @@ impl SessionContext {
self.state.lock().runtime_env.clone()
}
+ /// Return a copied version of config for this Session
+ pub fn copied_config(&self) -> SessionConfig {
Review comment:
> I think a more standard interface would be
>
> ```rust
> pub fn config(&self) -> &SessionConfig {
> ..
> }
> ```
>
> And then the caller can choose to do `state.config().clone()` if they want
a copy
>
> However I see the reason for doing it this way is that `SessionConfig` is
wrapped in a Mutex.
>
> I wonder if we can remove the Mutex in some future PR (and maybe use
>
> ```rust
> #[derive(Clone)]
> pub struct SessionContext {
> ...
> /// Shared session state for the session
> pub state: Arc<SessionState>,
> }
> ```
>
> And then handle cloning / copying internal to SessionContext when it is
mutated. 🤔
Hi, @alamb @yjshen
The reason that I name the method 'copied_config' is because I want to
emphasize the fact that the configuration inside the SessionContext can be
changed at anytime and this method returns a copied version. And I can not
return the reference since it is wrapped by a Mutex.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
