Skip to content

Commit

Permalink
refactor[rust]: Expr::Reverse -> Expr::Function(FunctionExpr::Reverse) (
Browse files Browse the repository at this point in the history
  • Loading branch information
ritchie46 committed Sep 2, 2022
1 parent 9496a92 commit 622c924
Show file tree
Hide file tree
Showing 12 changed files with 20 additions and 31 deletions.
1 change: 0 additions & 1 deletion polars/polars-lazy/src/dsl/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,6 @@ pub enum Expr {
function: FunctionExpr,
options: FunctionOptions,
},
Reverse(Box<Expr>),
Duplicated(Box<Expr>),
IsUnique(Box<Expr>),
Explode(Box<Expr>),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@ use super::*;
pub(super) fn shift(s: &Series, periods: i64) -> Result<Series> {
Ok(s.shift(periods))
}

pub(super) fn reverse(s: &Series) -> Result<Series> {
Ok(s.reverse())
}
7 changes: 5 additions & 2 deletions polars/polars-lazy/src/dsl/function_expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
mod arg_where;
#[cfg(feature = "round_series")]
mod clip;
mod dispatch;
mod fill_null;
#[cfg(feature = "is_in")]
mod is_in;
Expand All @@ -16,7 +17,6 @@ mod row_hash;
mod schema;
#[cfg(feature = "search_sorted")]
mod search_sorted;
mod shift;
mod shift_and_fill;
#[cfg(feature = "sign")]
mod sign;
Expand Down Expand Up @@ -98,6 +98,7 @@ pub enum FunctionExpr {
reverse: bool,
},
Shift(i64),
Reverse,
}

impl Display for FunctionExpr {
Expand Down Expand Up @@ -144,6 +145,7 @@ impl Display for FunctionExpr {
#[cfg(feature = "top_k")]
FunctionExpr::TopK { .. } => write!(f, "top_k"),
FunctionExpr::Shift(_) => write!(f, "shift"),
FunctionExpr::Reverse => write!(f, "reverse"),
}
}
}
Expand Down Expand Up @@ -296,7 +298,8 @@ impl From<FunctionExpr> for SpecialEq<Arc<dyn SeriesUdf>> {
TopK { k, reverse } => {
map!(top_k, k, reverse)
}
Shift(periods) => map!(shift::shift, periods),
Shift(periods) => map!(dispatch::shift, periods),
Reverse => map!(dispatch::reverse),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion polars/polars-lazy/src/dsl/function_expr/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl FunctionExpr {
}
#[cfg(feature = "top_k")]
TopK { .. } => same_type(),
Shift(..) => same_type(),
Shift(..) | Reverse => same_type(),
}
}
}
2 changes: 1 addition & 1 deletion polars/polars-lazy/src/dsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ impl Expr {

/// Reverse column
pub fn reverse(self) -> Self {
Expr::Reverse(Box::new(self))
self.apply_private(FunctionExpr::Reverse)
}

/// Apply a function/closure once the logical plan get executed.
Expand Down
3 changes: 0 additions & 3 deletions polars/polars-lazy/src/logical_plan/aexpr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ pub enum AAggExpr {
pub enum AExpr {
IsUnique(Node),
Duplicated(Node),
Reverse(Node),
Explode(Node),
Alias(Node, Arc<str>),
Column(Arc<str>),
Expand Down Expand Up @@ -135,7 +134,6 @@ impl AExpr {
| Take { .. }
| Nth(_)
| IsUnique(_) => true,
Reverse(_)
| Alias(_, _)
| Explode(_)
| Column(_)
Expand Down Expand Up @@ -216,7 +214,6 @@ impl AExpr {
let field = arena.get(*expr).to_field(schema, ctxt, arena)?;
Ok(Field::new(field.name(), DataType::Boolean))
}
Reverse(expr) => arena.get(*expr).to_field(schema, ctxt, arena),
Explode(expr) => {
let field = arena.get(*expr).to_field(schema, ctxt, arena)?;

Expand Down
2 changes: 0 additions & 2 deletions polars/polars-lazy/src/logical_plan/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ pub(crate) fn to_aexpr(expr: Expr, arena: &mut Arena<AExpr>) -> Node {
let v = match expr {
Expr::IsUnique(expr) => AExpr::IsUnique(to_aexpr(*expr, arena)),
Expr::Duplicated(expr) => AExpr::Duplicated(to_aexpr(*expr, arena)),
Expr::Reverse(expr) => AExpr::Reverse(to_aexpr(*expr, arena)),
Expr::Explode(expr) => AExpr::Explode(to_aexpr(*expr, arena)),
Expr::Alias(e, name) => AExpr::Alias(to_aexpr(*e, arena), name),
Expr::Literal(value) => AExpr::Literal(value),
Expand Down Expand Up @@ -450,7 +449,6 @@ pub(crate) fn node_to_expr(node: Node, expr_arena: &Arena<AExpr>) -> Expr {
match expr {
AExpr::Duplicated(node) => Expr::Duplicated(Box::new(node_to_expr(node, expr_arena))),
AExpr::IsUnique(node) => Expr::IsUnique(Box::new(node_to_expr(node, expr_arena))),
AExpr::Reverse(node) => Expr::Reverse(Box::new(node_to_expr(node, expr_arena))),
AExpr::Explode(node) => Expr::Explode(Box::new(node_to_expr(node, expr_arena))),
AExpr::Alias(expr, name) => {
let exp = node_to_expr(expr, expr_arena);
Expand Down
1 change: 0 additions & 1 deletion polars/polars-lazy/src/logical_plan/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ impl fmt::Debug for Expr {
IsUnique(expr) => write!(f, "{:?}.unique()", expr),
Explode(expr) => write!(f, "{:?}.explode()", expr),
Duplicated(expr) => write!(f, "{:?}.is_duplicate()", expr),
Reverse(expr) => write!(f, "{:?}.reverse()", expr),
Alias(expr, name) => write!(f, "{:?}.alias(\"{}\")", expr, name),
Column(name) => write!(f, "col(\"{}\")", name),
Literal(v) => {
Expand Down
2 changes: 0 additions & 2 deletions polars/polars-lazy/src/logical_plan/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ macro_rules! push_expr {
// as the root columns/ input columns by `_suffix` and `_keep_name` etc.
AnonymousFunction { input, .. } => input.$iter().rev().for_each(|e| $push(e)),
Function { input, .. } => input.$iter().rev().for_each(|e| $push(e)),
Reverse(e) => $push(e),
Duplicated(e) => $push(e),
IsUnique(e) => $push(e),
Explode(e) => $push(e),
Expand Down Expand Up @@ -234,7 +233,6 @@ impl AExpr {
{
input.iter().rev().for_each(push)
}
Reverse(e) => push(e),
Duplicated(e) => push(e),
IsUnique(e) => push(e),
Explode(e) => push(e),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ pub(super) fn predicate_is_pushdown_boundary(node: Node, expr_arena: &Arena<AExp
e,
AExpr::Sort { .. } | AExpr::SortBy { .. }
| AExpr::Agg(_) // an aggregation needs all rows
| AExpr::Reverse(_)
// Apply groups can be something like shift, sort, or an aggregation like skew
// both need all values
| AExpr::AnonymousFunction {options: FunctionOptions { collect_groups: ApplyOptions::ApplyGroups, .. }, ..}
Expand Down Expand Up @@ -144,7 +143,6 @@ pub(super) fn project_other_column_is_predicate_pushdown_boundary(
e,
AExpr::Sort { .. } | AExpr::SortBy { .. }
| AExpr::Agg(_) // an aggregation needs all rows
| AExpr::Reverse(_)
// Apply groups can be something like shift, sort, or an aggregation like skew
// both need all values
| AExpr::AnonymousFunction {options: FunctionOptions { collect_groups: ApplyOptions::ApplyGroups, .. }, ..}
Expand Down Expand Up @@ -177,7 +175,6 @@ pub(super) fn projection_column_is_predicate_pushdown_boundary(
e,
AExpr::Sort { .. } | AExpr::SortBy { .. }
| AExpr::Agg(_) // an aggregation needs all rows
| AExpr::Reverse(_)
// everything that works on groups likely changes to order of elements w/r/t the other columns
| AExpr::AnonymousFunction {..}
| AExpr::Function {..}
Expand Down
11 changes: 9 additions & 2 deletions polars/polars-lazy/src/logical_plan/optimizer/simplify_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use polars_utils::arena::Arena;

use crate::logical_plan::optimizer::stack_opt::OptimizationRule;
use crate::logical_plan::*;
use crate::prelude::function_expr::FunctionExpr;

macro_rules! eval_binary_same_type {
($lhs:expr, $operand: tt, $rhs:expr) => {{
Expand Down Expand Up @@ -383,8 +384,14 @@ impl OptimizationRule for SimplifyExprRule {
_ => None,
}
}
AExpr::Reverse(expr) => {
let input = expr_arena.get(*expr);
// sort().reverse() -> sort(reverse)
// sort_by().reverse() -> sort_by(reverse)
AExpr::Function {
input,
function: FunctionExpr::Reverse,
..
} => {
let input = expr_arena.get(input[0]);
match input {
AExpr::Sort { expr, options } => {
let mut options = *options;
Expand Down
13 changes: 0 additions & 13 deletions polars/polars-lazy/src/physical_plan/planner/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,19 +545,6 @@ impl PhysicalPlanner {
expr: node_to_expr(expression, expr_arena),
}))
}
Reverse(expr) => {
let input = self.create_physical_expr(expr, ctxt, expr_arena)?;
let function = SpecialEq::new(Arc::new(move |s: &mut [Series]| {
let s = std::mem::take(&mut s[0]);
Ok(s.reverse())
}) as Arc<dyn SeriesUdf>);
Ok(Arc::new(ApplyExpr::new_minimal(
vec![input],
function,
node_to_expr(expression, expr_arena),
ApplyOptions::ApplyGroups,
)))
}
Duplicated(expr) => {
let input = self.create_physical_expr(expr, ctxt, expr_arena)?;
let function = SpecialEq::new(Arc::new(move |s: &mut [Series]| {
Expand Down

0 comments on commit 622c924

Please sign in to comment.