Omega359 commented on code in PR #13527:
URL: https://github.com/apache/datafusion/pull/13527#discussion_r1899197257
##########
datafusion/physical-expr/src/scalar_function.rs:
##########
@@ -243,6 +292,7 @@ pub fn create_physical_expr(
Arc::new(fun.clone()),
input_phy_exprs.to_vec(),
return_type,
+ Arc::new(config_options.clone()),
Review Comment:
That was my main concern as well. I am unsure that this change should merged
in as is to be honest as any fix will be just as disruptive as this PR is api
wise.
Pushing the clone higher up the stack is possible but I did run into major
issues trying to push it all the way up including all kinds of disruptive
changes like changing OptimizerConfig from &ConfigOptions to the Arc version.
Doing the opposite - pushing &ConfigOptions all the way down ran into issues
with ScalarFunctionExpr and DynEq/DynHash.
Speaking for myself I prefer the latter as I feel it's less disruptive
overall ... I just couldn't get it to work before. Changing the signature of
DynHash and DynEq traits may allow it to work but I haven't tried that yet.
Maybe with some more thought in January I can get it to work but I expect my
January time to be pretty limited.
--
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
