Skip to content

Commit 2186e2f

Browse files
committed
feat: optimize classifier and enhance child mapping
- Inlined classifier temps for improved performance. - Replaced replace_map loop with an iterator collect for better efficiency. - Added new method map_single_child to streamline child processing. - Integrated map_single_child in both insert_filter_below_unary and insert_below functions.
1 parent aa38b49 commit 2186e2f

1 file changed

Lines changed: 28 additions & 28 deletions

File tree

‎datafusion/optimizer/src/push_down_filter.rs‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,16 +1004,18 @@ impl OptimizerRule for PushDownFilter {
10041004
// As for plan Filter: Column(a+b) > 0 -- Agg: groupby:[Column(a)+Column(b)]
10051005
// After push, we need to replace `a+b` with Column(a)+Column(b)
10061006
// So we need create a replace_map, add {`a+b` --> Expr(Column(a)+Column(b))}
1007-
let mut replace_map = HashMap::new();
1008-
for expr in &agg.group_expr {
1009-
replace_map.insert(expr.schema_name().to_string(), expr.clone());
1010-
}
1007+
let replace_map = agg
1008+
.group_expr
1009+
.iter()
1010+
.map(|expr| (expr.schema_name().to_string(), expr.clone()))
1011+
.collect::<HashMap<_, _>>();
10111012

10121013
push_down_filter_through_unary(
10131014
filter.predicate,
10141015
|expr| {
1015-
let cols = expr.column_refs();
1016-
cols.iter().all(|c| group_expr_columns.contains(c))
1016+
expr.column_refs()
1017+
.iter()
1018+
.all(|c| group_expr_columns.contains(c))
10171019
},
10181020
LogicalPlan::Aggregate(agg),
10191021
|push_predicates| {
@@ -1090,8 +1092,9 @@ impl OptimizerRule for PushDownFilter {
10901092
push_down_filter_through_unary(
10911093
filter.predicate,
10921094
|expr| {
1093-
let cols = expr.column_refs();
1094-
cols.iter().all(|c| potential_partition_keys.contains(c))
1095+
expr.column_refs()
1096+
.iter()
1097+
.all(|c| potential_partition_keys.contains(c))
10951098
},
10961099
LogicalPlan::Window(window),
10971100
Ok,
@@ -1375,20 +1378,7 @@ fn insert_filter_below_unary(
13751378
plan: LogicalPlan,
13761379
predicate: Expr,
13771380
) -> Result<Transformed<LogicalPlan>> {
1378-
let mut predicate = Some(predicate);
1379-
let transformed_plan = plan.map_children(|child| {
1380-
if let Some(predicate) = predicate.take() {
1381-
make_filter(predicate, Arc::new(child)).map(Transformed::yes)
1382-
} else {
1383-
// already took the predicate
1384-
internal_err!("node had more than one input")
1385-
}
1386-
})?;
1387-
1388-
// make sure we did the actual replacement
1389-
assert_or_internal_err!(predicate.is_none(), "node had no inputs");
1390-
1391-
Ok(transformed_plan)
1381+
map_single_child(plan, |child| make_filter(predicate, Arc::new(child)))
13921382
}
13931383

13941384
/// Creates a new LogicalPlan::Filter node.
@@ -1413,18 +1403,28 @@ fn insert_below(
14131403
plan: LogicalPlan,
14141404
new_child: LogicalPlan,
14151405
) -> Result<Transformed<LogicalPlan>> {
1416-
let mut new_child = Some(new_child);
1417-
let transformed_plan = plan.map_children(|_child| {
1418-
if let Some(new_child) = new_child.take() {
1419-
Ok(Transformed::yes(new_child))
1406+
map_single_child(plan, |_child| Ok(new_child))
1407+
}
1408+
1409+
fn map_single_child<F>(
1410+
plan: LogicalPlan,
1411+
replace_child: F,
1412+
) -> Result<Transformed<LogicalPlan>>
1413+
where
1414+
F: FnOnce(LogicalPlan) -> Result<LogicalPlan>,
1415+
{
1416+
let mut replace_child = Some(replace_child);
1417+
let transformed_plan = plan.map_children(|child| {
1418+
if let Some(replace_child) = replace_child.take() {
1419+
replace_child(child).map(Transformed::yes)
14201420
} else {
1421-
// already took the new child
1421+
// already replaced the child
14221422
internal_err!("node had more than one input")
14231423
}
14241424
})?;
14251425

14261426
// make sure we did the actual replacement
1427-
assert_or_internal_err!(new_child.is_none(), "node had no inputs");
1427+
assert_or_internal_err!(replace_child.is_none(), "node had no inputs");
14281428

14291429
Ok(transformed_plan)
14301430
}

0 commit comments

Comments
 (0)