I wonder if perhaps this is a sign that we don't really need these locks 🤔 will have a play around

I wonder if perhaps this is a sign that we don't really need these locks 🤔 will have a play around

IMO that lock cannot be avoided. If we don't put it over SessionState then we need to either put it in the SessionState or change all the methods to require &mut self, which is likely to be put inside another lock to achieve the "interior mutability" 🥲

BTW I think @tustvold has some larger ideas of how to simplify the configuration system -- see #4617 and related tickets

I think it is fine to merge this PR , even though there is likely going to be some churn in this API anyways

Benchmark runs are scheduled for baseline = 4542031 and contender = da0de9d. da0de9d is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Thanks for that info!

I remember when I refactor the original ExecutionContext to SessionContext, I also thought the signature of the &mut self can be relaxed, the reason I didn't make this change in those PRs is just to keep them small.