alamb commented on code in PR #16970:
URL: https://github.com/apache/datafusion/pull/16970#discussion_r2252103615
##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -137,13 +137,15 @@ impl OptimizerContext {
Self {
query_execution_start_time: Utc::now(),
alias_generator: Arc::new(AliasGenerator::new()),
- options,
+ options: Arc::new(options),
}
}
/// Specify whether to enable the filter_null_keys rule
pub fn filter_null_keys(mut self, filter_null_keys: bool) -> Self {
- self.options.optimizer.filter_null_join_keys = filter_null_keys;
+ Arc::make_mut(&mut self.options)
Review Comment:
Yeah -- I think this is unavoidable if someone wants to modify the config
options
I think it is better than what is on main as today every OptimizerConfig
*requires* a clone of the `ConfigOptions` -- using `make_mut` means the code
will now only clone the config when it actually needs to be modified
##########
datafusion/expr/src/execution_props.rs:
##########
@@ -68,9 +72,10 @@ impl ExecutionProps {
/// Marks the execution of query started timestamp.
/// This also instantiates a new alias generator.
- pub fn start_execution(&mut self) -> &Self {
+ pub fn mark_start_execution(&mut self, config_options: Arc<ConfigOptions>)
-> &Self {
Review Comment:
i think this is technically a breaking API. It would be nicer for downstream
users if we kept the old function and deprecated it so the compiler told them
what was happening.
I pushed a commit to do that
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
