Skip to content

Commit

Permalink
fix(rust, python): projection_node always do projection locally if no…
Browse files Browse the repository at this point in the history
…t pruned
  • Loading branch information
ritchie46 committed Oct 3, 2022
1 parent c928b4a commit 8c8e247
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ impl ProjectionPushDown {

match logical_plan {
Projection { expr, input, .. } => {
let mut local_projection = Vec::with_capacity(expr.len());
// A projection can consist of a chain of expressions followed by an alias.
// We want to do the chain locally because it can have complicated side effects.
// The only thing we push down is the root name of the projection.
Expand Down Expand Up @@ -327,6 +328,10 @@ impl ProjectionPushDown {
}
}
}
// do local as we still need the effect of the projection
// e.g. a projection is more than selecting a column, it can
// also be a function/ complicated expression
local_projection.push(*e);

add_expr_to_accumulated(
*e,
Expand All @@ -344,27 +349,6 @@ impl ProjectionPushDown {
lp_arena,
expr_arena,
)?;
let lp = lp_arena.get(input);

let mut local_projection = Vec::with_capacity(expr.len());

// the projections should all be done at the latest projection node to keep the same schema order
if projections_seen == 0
|| expr.iter().any(|node| has_aexpr_alias(*node, expr_arena))
{
let schema = lp.schema(lp_arena);
for node in expr {
// Due to the pushdown, a lot of projections cannot be done anymore at the final
// node and should be skipped
if expr_arena
.get(node)
.to_field(&schema, Context::Default, expr_arena)
.is_ok()
{
local_projection.push(node);
}
}
}

let builder = ALogicalPlanBuilder::new(input, expr_arena, lp_arena);
let lp = self.finish_node(local_projection, builder);
Expand Down
4 changes: 0 additions & 4 deletions polars/polars-lazy/polars-plan/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ where
arena.iter(current_node).any(|(_node, e)| matches(e))
}

pub(crate) fn has_aexpr_alias(current_node: Node, arena: &Arena<AExpr>) -> bool {
has_aexpr(current_node, arena, |e| matches!(e, AExpr::Alias(_, _)))
}

pub fn has_aexpr_window(current_node: Node, arena: &Arena<AExpr>) -> bool {
has_aexpr(current_node, arena, |e| matches!(e, AExpr::Window { .. }))
}
Expand Down
33 changes: 33 additions & 0 deletions polars/tests/it/lazy/projection_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,36 @@ fn test_many_aliasing_projections_5070() -> PolarsResult<()> {

Ok(())
}

#[test]
fn test_projection_5086() -> PolarsResult<()> {
let df = df![
"a" => ["a", "a", "a", "b"],
"b" => [1, 0, 1, 0],
"c" => [0, 1, 2, 0],
]?;

let out = df
.lazy()
.select([
col("a"),
col("b").take("c").cumsum(false).over([col("a")]).gt(lit(0)),
])
.select([
col("a"),
col("b")
.xor(col("b").shift(1).over([col("a")]))
.fill_null(lit(true))
.alias("keep"),
])
.collect()?;

let expected = df![
"a" => ["a", "a", "a", "b"],
"keep" => [true, false, false, true]
]?;

assert!(out.frame_equal(&expected));

Ok(())
}

0 comments on commit 8c8e247

Please sign in to comment.