2010YOUY01 commented on code in PR #25564:
URL: https://github.com/apache/datafusion/pull/25564#discussion_r4068406255
##########
datafusion/physical-optimizer/src/limited_distinct_aggregation.rs:
##########
@@ -47,16 +47,15 @@ impl LimitedDistinctAggregation {
fn transform_agg(
aggr: &AggregateExec,
limit: usize,
- ) -> Option<Arc<dyn ExecutionPlan>> {
+ ) -> Option<Transformed<Arc<dyn ExecutionPlan>>> {
// rules for transforming this Aggregate are held in this method
if !aggr.is_unordered_unfiltered_group_by_distinct() {
return None;
}
- // We found what we want: clone, copy the limit down, and return
modified node
- let new_aggr =
aggr.with_new_limit_options(Some(LimitOptions::new(limit)));
-
- Some(Arc::new(new_aggr))
+ let new_aggr =
aggr.clone().try_optimize_distinct_soft_limit(limit).ok()?;
Review Comment:
- `Option` instead of `Result`: I agree, updated in
[2cf27d8](https://github.com/apache/datafusion/pull/25564/commits/2cf27d8e1022d765d46548ac4260ce1b16eb9ef1)
- Avoiding deep clone: It seems impossible due to the optimizer interface
restriction?
--
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]
