Make SessionState members private #4764
Conversation
| F: FnMut(&dyn ExecutionPlan, &dyn PhysicalOptimizerRule), | ||
| { | ||
| let optimizers = &session_state.physical_optimizers; | ||
| let optimizers = session_state.physical_optimizers(); |
There was a problem hiding this comment.
This is quite a peculiar API, I'm not sure why the physical planner would be calling the optimizers and not SessionState itself
There was a problem hiding this comment.
it may predate when the list of optimizers was on the session state 🤔
There was a problem hiding this comment.
Ok - I will add it to my list
| F: FnMut(&dyn ExecutionPlan, &dyn PhysicalOptimizerRule), | ||
| { | ||
| let optimizers = &session_state.physical_optimizers; | ||
| let optimizers = session_state.physical_optimizers(); |
There was a problem hiding this comment.
it may predate when the list of optimizers was on the session state 🤔
|
I plan to merge this tomorrow morning unless I hear anything further |
|
Benchmark runs are scheduled for baseline = c9350d1 and contender = 41c72cf. 41c72cf is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
5 participants
Which issue does this PR close?
Closes #.
Rationale for this change
Exposing members publicly is a very strong and broad API commitment, which makes it very difficult to make non-breaking changes, as it is very hard to reason about how the type is being used.
What changes are included in this PR?
Makes the members of
SessionStateprivateAre these changes tested?
Are there any user-facing changes?