Skip to content

Commit 9b48073

Browse files
committed
Refactor utility layers and improve code compactness
Flatten single-use helper layers and tighten small utility helpers. In push_down_filter, fold the join-input classifier and bind predicate.column_refs() once for join inference. Shorten test-only import surface in utils.rs and compact mapping helpers in null_restriction.rs without changing semantics.
1 parent 787d0a4 commit 9b48073

File tree

3 files changed

+24
-30
lines changed

3 files changed

+24
-30
lines changed

datafusion/optimizer/src/push_down_filter.rs

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -285,26 +285,22 @@ fn can_evaluate_as_join_condition(predicate: &Expr) -> Result<bool> {
285285
Ok(is_evaluate)
286286
}
287287

288-
fn classify_join_input(plan: &LogicalPlan) -> (bool, bool) {
289-
match plan {
290-
LogicalPlan::SubqueryAlias(subquery_alias) => {
291-
let (is_scalar_aggregate, _) =
292-
classify_join_input(subquery_alias.input.as_ref());
293-
(is_scalar_aggregate, true)
294-
}
295-
LogicalPlan::Projection(projection) => {
296-
classify_join_input(projection.input.as_ref())
288+
fn is_scalar_subquery_cross_join(join: &Join) -> bool {
289+
fn classify(plan: &LogicalPlan) -> (bool, bool) {
290+
match plan {
291+
LogicalPlan::SubqueryAlias(subquery_alias) => {
292+
let (is_scalar_aggregate, _) = classify(subquery_alias.input.as_ref());
293+
(is_scalar_aggregate, true)
294+
}
295+
LogicalPlan::Projection(projection) => classify(projection.input.as_ref()),
296+
LogicalPlan::Aggregate(aggregate) => (aggregate.group_expr.is_empty(), false),
297+
_ => (false, false),
297298
}
298-
LogicalPlan::Aggregate(aggregate) => (aggregate.group_expr.is_empty(), false),
299-
_ => (false, false),
300299
}
301-
}
302300

303-
fn is_scalar_subquery_cross_join(join: &Join) -> bool {
304-
let (left_scalar_aggregate, left_is_derived_relation) =
305-
classify_join_input(join.left.as_ref());
301+
let (left_scalar_aggregate, left_is_derived_relation) = classify(join.left.as_ref());
306302
let (right_scalar_aggregate, right_is_derived_relation) =
307-
classify_join_input(join.right.as_ref());
303+
classify(join.right.as_ref());
308304
join.on.is_empty()
309305
&& join.filter.is_none()
310306
&& ((left_scalar_aggregate && right_is_derived_relation)
@@ -754,10 +750,11 @@ fn infer_join_predicates_impl<
754750
inferred_predicates: &mut InferredPredicates,
755751
) -> Result<()> {
756752
for predicate in input_predicates {
753+
let column_refs = predicate.column_refs();
757754
let mut join_cols_to_replace = HashMap::new();
758755
let mut saw_non_replaceable_ref = false;
759756

760-
for &col in &predicate.column_refs() {
757+
for &col in &column_refs {
761758
let replacement = join_col_keys.iter().find_map(|(l, r)| {
762759
if ENABLE_LEFT_TO_RIGHT && col == *l {
763760
Some((col, *r))

datafusion/optimizer/src/utils.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,7 @@ use log::{debug, trace};
3939
pub use datafusion_expr::expr_rewriter::NamePreserver;
4040

4141
#[cfg(test)]
42-
use self::test_eval_mode::{
43-
NullRestrictionEvalMode, null_restriction_eval_mode,
44-
set_null_restriction_eval_mode_for_test, with_null_restriction_eval_mode_for_test,
45-
};
42+
use self::test_eval_mode::*;
4643

4744
/// Returns true if `expr` contains all columns in `schema_cols`
4845
pub(crate) fn has_all_column_refs(expr: &Expr, schema_cols: &HashSet<Column>) -> bool {

datafusion/optimizer/src/utils/null_restriction.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,13 @@ pub(super) fn syntactic_restrict_null_predicate(
3636
predicate: &Expr,
3737
join_cols: &HashSet<&Column>,
3838
) -> Option<bool> {
39-
match syntactic_null_substitution_value(predicate, join_cols) {
40-
Some(NullSubstitutionValue::Boolean(value)) => Some(!value),
41-
Some(NullSubstitutionValue::Null) => Some(true),
42-
Some(NullSubstitutionValue::NonNull) | None => None,
43-
}
39+
syntactic_null_substitution_value(predicate, join_cols).and_then(
40+
|value| match value {
41+
NullSubstitutionValue::Boolean(value) => Some(!value),
42+
NullSubstitutionValue::Null => Some(true),
43+
NullSubstitutionValue::NonNull => None,
44+
},
45+
)
4446
}
4547

4648
fn not(value: Option<NullSubstitutionValue>) -> Option<NullSubstitutionValue> {
@@ -49,7 +51,7 @@ fn not(value: Option<NullSubstitutionValue>) -> Option<NullSubstitutionValue> {
4951
Some(NullSubstitutionValue::Boolean(!value))
5052
}
5153
Some(NullSubstitutionValue::Null) => Some(NullSubstitutionValue::Null),
52-
Some(NullSubstitutionValue::NonNull) | None => None,
54+
_ => None,
5355
}
5456
}
5557

@@ -90,9 +92,7 @@ fn null_check_value(
9092
Some(NullSubstitutionValue::Null) => {
9193
Some(NullSubstitutionValue::Boolean(!is_not_null))
9294
}
93-
Some(NullSubstitutionValue::NonNull | NullSubstitutionValue::Boolean(_)) => {
94-
Some(NullSubstitutionValue::Boolean(is_not_null))
95-
}
95+
Some(_) => Some(NullSubstitutionValue::Boolean(is_not_null)),
9696
None => None,
9797
}
9898
}

0 commit comments

Comments
 (0)