theirix commented on PR #19311: URL: https://github.com/apache/datafusion/pull/19311#issuecomment-3689505547
> > > Don't we have this already at the logical and physical expression layers? > > > > > > @adriangb please correct me if I'm wrong, but since we are accessing SQL ast, we cannot have it at a higher level, especially physical planning. The proposed function won't be used in higher-level layers, but only for planning. > > I think the question is if we already have an optimizer rule to simplify expressions (including evaluating literals), what benefit does this PR bring to do it at the SQL planning level? Does it only work if the optimizer is fed with an expression `datafusion_expr::Expr`? [These lines with logic for AST expression](https://github.com/apache/datafusion/pull/19311/changes#diff-df4cb38888c9b754de88df54f0c9b87a58c39a673c4d257828de8096358bec86R449) rely on having a numeric as a physical value - check if it is in `0..1` range, float or int, etc. We don't pass it as an expression for the optimizer to process later. For other cases, of course, optimizer rules can properly work with expressions, and this function won't be needed. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
