adriangb commented on code in PR #20065:
URL: https://github.com/apache/datafusion/pull/20065#discussion_r2749731114
##########
datafusion/optimizer/src/common_subexpr_eliminate.rs:
##########
@@ -702,6 +702,11 @@ impl CSEController for ExprCSEController<'_> {
#[expect(deprecated)]
let is_normal_minus_aggregates = matches!(
node,
+ // TODO: there's an argument for removing `Literal` from here,
+ // maybe using `Expr::placemement().should_push_to_leaves()`
instead
+ // so that we extract common literals and don't broadcast them to
num_batch_rows multiple times.
+ // However that currently breaks things like `percentile_cont()`
which expect literal arguments
+ // (and would instead be getting `col(__common_expr_n)`).
Expr::Literal(..)
Review Comment:
I think this is an interesting callout to consider. There's some version of
this that allows the literals to be optimized but "stops" at scalar functions.
To consider for another PR...
--
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]
