alamb commented on code in PR #10331:
URL: https://github.com/apache/datafusion/pull/10331#discussion_r1586538159
##########
datafusion-examples/examples/expr_api.rs:
##########
@@ -258,7 +258,7 @@ pub fn physical_expr(schema: &Schema, expr: Expr) ->
Result<Arc<dyn PhysicalExpr
ExprSimplifier::new(SimplifyContext::new(&props).with_schema(df_schema.clone()));
// apply type coercion here to ensure types match
- let expr = simplifier.coerce(expr, df_schema.clone())?;
+ let expr = simplifier.coerce(expr, &df_schema)?;
Review Comment:
This shows the effect of this API change on users (add a &)
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -208,14 +206,8 @@ impl<S: SimplifyInfo> ExprSimplifier<S> {
///
/// See the [type coercion module](datafusion_expr::type_coercion)
/// documentation for more details on type coercion
- ///
- // Would be nice if this API could use the SimplifyInfo
- // rather than creating an DFSchemaRef coerces rather than doing
- // it manually.
- // https://github.com/apache/datafusion/issues/3793
- pub fn coerce(&self, expr: Expr, schema: DFSchemaRef) -> Result<Expr> {
+ pub fn coerce(&self, expr: Expr, schema: &DFSchema) -> Result<Expr> {
Review Comment:
This is the actual change (I don't think making it take `SimplifyInfo` is
practical given how much coerce does)
--
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
