You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/04/29 12:59:07 UTC
[I] Stop copying LogicalPlan and Exprs in `ReplaceDistinctWithAggregate` [datafusion]
alamb opened a new issue, #10293:
URL: https://github.com/apache/datafusion/issues/10293
### Is your feature request related to a problem or challenge?
Part of https://github.com/apache/datafusion/issues/9637
As part of making the planner faster, we are updating the optimizer passes to avoid copying `LogicalPlan` and `Expr` (see https://github.com/apache/datafusion/issues/9637)
### Describe the solution you'd like
I would like to reduce the amount of copying in this pass (even though it doesn't appear in current profiling)
### Describe alternatives you've considered
Apply the model from @Lordworms in https://github.com/apache/datafusion/pull/10166 to this pass
2. Update OptimizerRule::supports_rewrite` to return true
1. Update `OptimizerRule` to use `rewrite`
3. Update the pass itself to not copy the LogicalPlan (ideally using the [`TreeNode`](https://github.com/apache/datafusion/blob/b41ef20c5dad7bdd674e3cc5f35a9c99efae676c/datafusion/common/src/tree_node.rs#L33-L94) API) - it is implemented for `LogicalPlan` ([API](https://github.com/apache/datafusion/blob/b41ef20c5dad7bdd674e3cc5f35a9c99efae676c/datafusion/expr/src/logical_plan/tree_node.rs#L18-L38)) and `Expr` ([API](https://github.com/apache/datafusion/blob/b41ef20c5dad7bdd674e3cc5f35a9c99efae676c/datafusion/expr/src/tree_node.rs#L18))
Other examples: https://github.com/apache/datafusion/pull/10218
### Additional context
--
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-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Stop copying LogicalPlan and Exprs in `ReplaceDistinctWithAggregate` [datafusion]
Posted by "ClSlaid (via GitHub)" <gi...@apache.org>.
ClSlaid commented on issue #10293:
URL: https://github.com/apache/datafusion/issues/10293#issuecomment-2105950929
hi, @alamb, I'd like to know if those two expressions differ:
```rust
let aggr_expr = vec![Expr::AggregateFunction(AggregateFunction::new(
AggregateFunctionFunc::FirstValue,
select_expr,
false,
None,
sort_expr.clone(),
None,
))];
```
```rust
let aggr_expr = select_expr.into_iter().map(|e| {
Expr::AggregateFunction(AggregateFunction::new(
AggregateFunctionFunc::FirstValue,
vec![e],
false,
None,
sort_expr.clone(),
None,
))
});
```
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Stop copying LogicalPlan and Exprs in `ReplaceDistinctWithAggregate` [datafusion]
Posted by "ClSlaid (via GitHub)" <gi...@apache.org>.
ClSlaid commented on issue #10293:
URL: https://github.com/apache/datafusion/issues/10293#issuecomment-2105629456
May I take a look? Merci.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Stop copying LogicalPlan and Exprs in `ReplaceDistinctWithAggregate` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10293:
URL: https://github.com/apache/datafusion/issues/10293#issuecomment-2105681808
of course thank you @ClSlaid -- just be aware these PRs have tended to be tricky. I recommend doing it incrementally if possible. Let me know if you need some help 🙏
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Stop copying LogicalPlan and Exprs in `ReplaceDistinctWithAggregate` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #10293: Stop copying LogicalPlan and Exprs in `ReplaceDistinctWithAggregate`
URL: https://github.com/apache/datafusion/issues/10293
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Stop copying LogicalPlan and Exprs in `ReplaceDistinctWithAggregate` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10293:
URL: https://github.com/apache/datafusion/issues/10293#issuecomment-2105992692
> hi, @alamb, I'd like to know if those two expressions are semantically identical:
Yes, I believe they are the same
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org