Skip to content

Commit

Permalink
fix: predicate push-down remove predicate refers to alias for more br…
Browse files Browse the repository at this point in the history
…anch (#11887)
  • Loading branch information
reswqa authored Oct 21, 2023
1 parent 6155e7f commit ff358ca
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ fn rename_predicate_columns_due_to_aliased_projection(
) -> LoopBehavior {
let projection_aexpr = expr_arena.get(projection_node);
if let AExpr::Alias(_, alias_name) = projection_aexpr {
let alias_name = alias_name.as_ref();
let alias_name = alias_name.clone();
let projection_leaves = aexpr_to_leaf_names(projection_node, expr_arena);

// this means the leaf is a literal
Expand All @@ -223,9 +223,10 @@ fn rename_predicate_columns_due_to_aliased_projection(

// if this alias refers to one of the predicates in the upper nodes
// we rename the column of the predicate before we push it downwards.
if let Some(predicate) = acc_predicates.remove(alias_name) {
if let Some(predicate) = acc_predicates.remove(&alias_name) {
if projection_maybe_boundary {
local_predicates.push(predicate);
remove_predicate_refers_to_alias(acc_predicates, local_predicates, &alias_name);
return LoopBehavior::Continue;
}
if projection_leaves.len() == 1 {
Expand All @@ -240,28 +241,36 @@ fn rename_predicate_columns_due_to_aliased_projection(
// on this projected column so we do filter locally.
local_predicates.push(predicate)
}
} else {
// we could not find the alias name
// that could still mean that a predicate that is a complicated binary expression
// refers to the aliased name. If we find it, we remove it for now
// TODO! rename the expression.
let mut remove_names = vec![];
for (composed_name, _) in acc_predicates.iter() {
if key_has_name(composed_name, alias_name) {
remove_names.push(composed_name.clone());
break;
}
}

for composed_name in remove_names {
let predicate = acc_predicates.remove(&composed_name).unwrap();
local_predicates.push(predicate)
}
}

remove_predicate_refers_to_alias(acc_predicates, local_predicates, &alias_name);
}
LoopBehavior::Nothing
}

/// we could not find the alias name
/// that could still mean that a predicate that is a complicated binary expression
/// refers to the aliased name. If we find it, we remove it for now
/// TODO! rename the expression.
fn remove_predicate_refers_to_alias(
acc_predicates: &mut PlHashMap<Arc<str>, Node>,
local_predicates: &mut Vec<Node>,
alias_name: &str,
) {
let mut remove_names = vec![];
for (composed_name, _) in acc_predicates.iter() {
if key_has_name(composed_name, alias_name) {
remove_names.push(composed_name.clone());
break;
}
}

for composed_name in remove_names {
let predicate = acc_predicates.remove(&composed_name).unwrap();
local_predicates.push(predicate)
}
}

/// Implementation for both Hstack and Projection
pub(super) fn rewrite_projection_node(
expr_arena: &mut Arena<AExpr>,
Expand Down
11 changes: 11 additions & 0 deletions py-polars/tests/unit/test_predicates.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,14 @@ def test_predicate_pushdown_group_by_keys() -> None:
.filter(pl.col("group") == 1)
.explain()
)


def test_no_predicate_push_down_with_cast_and_alias_11883() -> None:
df = pl.DataFrame({"a": [1, 2, 3]})
out = (
df.lazy()
.select(pl.col("a").cast(pl.Int64).alias("b"))
.filter(pl.col("b") == 1)
.filter((pl.col("b") >= 1) & (pl.col("b") < 1))
)
assert 'SELECTION: "None"' in out.explain(predicate_pushdown=True)

0 comments on commit ff358ca

Please sign in to comment.