Skip to content

Commit d54d101

Browse files
committed
Prevent take_optimizable from discarding arbitrary plan node
`take_optimizable` started from inspecting top level plan node (it should be final aggregation) and then descended trying to find matching partial aggregation. When doing so, it would ignore any single-source nodes it passes by. As a result, `AggregateStatistics` could change the plan in an undesired manner.
1 parent b42d9b8 commit d54d101

File tree

1 file changed

+7
-15
lines changed

1 file changed

+7
-15
lines changed

datafusion/physical-optimizer/src/aggregate_statistics.rs

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -115,22 +115,14 @@ fn take_optimizable(node: &dyn ExecutionPlan) -> Option<Arc<dyn ExecutionPlan>>
115115
if !final_agg_exec.mode().is_first_stage()
116116
&& final_agg_exec.group_expr().is_empty()
117117
{
118-
let mut child = Arc::clone(final_agg_exec.input());
119-
loop {
120-
if let Some(partial_agg_exec) =
121-
child.as_any().downcast_ref::<AggregateExec>()
118+
let child = final_agg_exec.input();
119+
if let Some(partial_agg_exec) = child.as_any().downcast_ref::<AggregateExec>()
120+
{
121+
if partial_agg_exec.mode().is_first_stage()
122+
&& partial_agg_exec.group_expr().is_empty()
123+
&& partial_agg_exec.filter_expr().iter().all(|e| e.is_none())
122124
{
123-
if partial_agg_exec.mode().is_first_stage()
124-
&& partial_agg_exec.group_expr().is_empty()
125-
&& partial_agg_exec.filter_expr().iter().all(|e| e.is_none())
126-
{
127-
return Some(child);
128-
}
129-
}
130-
if let [childrens_child] = child.children().as_slice() {
131-
child = Arc::clone(childrens_child);
132-
} else {
133-
break;
125+
return Some(Arc::clone(child));
134126
}
135127
}
136128
}

0 commit comments

Comments
 (0)