alamb commented on code in PR #8971:
URL: https://github.com/apache/arrow-datafusion/pull/8971#discussion_r1464006615
##########
datafusion/optimizer/src/simplify_expressions/inlist_simplifier.rs:
##########
@@ -52,71 +52,72 @@ impl TreeNodeRewriter for InListSimplifier {
type N = Expr;
fn mutate(&mut self, expr: Expr) -> Result<Expr> {
- if let Expr::BinaryExpr(BinaryExpr { left, op, right }) = &expr {
- if let (Expr::InList(l1), Operator::And, Expr::InList(l2)) =
- (left.as_ref(), op, right.as_ref())
- {
- if l1.expr == l2.expr && !l1.negated && !l2.negated {
- return inlist_intersection(l1, l2, false);
- } else if l1.expr == l2.expr && l1.negated && l2.negated {
- return inlist_union(l1, l2, true);
- } else if l1.expr == l2.expr && !l1.negated && l2.negated {
- return inlist_except(l1, l2);
- } else if l1.expr == l2.expr && l1.negated && !l2.negated {
- return inlist_except(l2, l1);
- }
- } else if let (Expr::InList(l1), Operator::Or, Expr::InList(l2)) =
- (left.as_ref(), op, right.as_ref())
- {
- if l1.expr == l2.expr && l1.negated && l2.negated {
- return inlist_intersection(l1, l2, true);
- }
+ if let Expr::BinaryExpr(BinaryExpr { left, op, right }) = expr {
+ match (*left, op, *right) {
+ (Expr::InList(l1), Operator::And, Expr::InList(l2)) if l1.expr
== l2.expr && !l1.negated && !l2.negated =>
+ inlist_intersection(l1, l2, false),
+ (Expr::InList(l1), Operator::And, Expr::InList(l2)) if l1.expr
== l2.expr && l1.negated && l2.negated =>
+ inlist_union(l1, l2, true),
+ (Expr::InList(l1), Operator::And, Expr::InList(l2)) if l1.expr
== l2.expr && !l1.negated && l2.negated =>
+ inlist_except(l1, l2),
+ (Expr::InList(l1), Operator::And, Expr::InList(l2)) if l1.expr
== l2.expr && l1.negated && !l2.negated =>
+ inlist_except(l2, l1),
+ (Expr::InList(l1), Operator::Or, Expr::InList(l2)) if l1.expr
== l2.expr && l1.negated && l2.negated =>
+ inlist_intersection(l1, l2, true),
+ (left, op, right) => {
+ // put the expression back together
+ Ok(Expr::BinaryExpr(BinaryExpr {
+ left: Box::new(left),
+ op,
+ right: Box::new(right),
+ }))
+ }
}
+ } else {
+ Ok(expr)
}
-
- Ok(expr)
}
}
-fn inlist_union(l1: &InList, l2: &InList, negated: bool) -> Result<Expr> {
- let mut seen: HashSet<Expr> = HashSet::new();
- let list = l1
- .list
- .iter()
- .chain(l2.list.iter())
- .filter(|&e| seen.insert(e.to_owned()))
- .cloned()
- .collect::<Vec<_>>();
- let merged_inlist = InList {
- expr: l1.expr.clone(),
- list,
- negated,
- };
- Ok(Expr::InList(merged_inlist))
+/// Return the union of two inlist expressions
+/// maintaining the order of the elements in the two lists
+fn inlist_union(mut l1: InList, l2: InList, negated: bool) -> Result<Expr> {
Review Comment:
These functions now take owned `InList` and modify l1 in place
##########
datafusion/optimizer/src/simplify_expressions/inlist_simplifier.rs:
##########
@@ -52,71 +52,72 @@ impl TreeNodeRewriter for InListSimplifier {
type N = Expr;
fn mutate(&mut self, expr: Expr) -> Result<Expr> {
- if let Expr::BinaryExpr(BinaryExpr { left, op, right }) = &expr {
- if let (Expr::InList(l1), Operator::And, Expr::InList(l2)) =
- (left.as_ref(), op, right.as_ref())
- {
- if l1.expr == l2.expr && !l1.negated && !l2.negated {
- return inlist_intersection(l1, l2, false);
- } else if l1.expr == l2.expr && l1.negated && l2.negated {
- return inlist_union(l1, l2, true);
- } else if l1.expr == l2.expr && !l1.negated && l2.negated {
- return inlist_except(l1, l2);
- } else if l1.expr == l2.expr && l1.negated && !l2.negated {
- return inlist_except(l2, l1);
- }
- } else if let (Expr::InList(l1), Operator::Or, Expr::InList(l2)) =
- (left.as_ref(), op, right.as_ref())
- {
- if l1.expr == l2.expr && l1.negated && l2.negated {
- return inlist_intersection(l1, l2, true);
- }
+ if let Expr::BinaryExpr(BinaryExpr { left, op, right }) = expr {
Review Comment:
This pattern allows getting an owned `left` and `right` and avoiding the
clone
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
