joroKr21 commented on code in PR #21668:
URL: https://github.com/apache/datafusion/pull/21668#discussion_r3273777734
##########
datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs:
##########
@@ -52,28 +52,27 @@ pub fn simplify_predicates(predicates: Vec<Expr>) ->
Result<Vec<Expr>> {
let mut other_predicates = Vec::new();
for pred in predicates {
- match &pred {
- Expr::BinaryExpr(BinaryExpr {
- left,
- op:
- Operator::Gt
- | Operator::GtEq
- | Operator::Lt
- | Operator::LtEq
- | Operator::Eq,
- right,
- }) => {
- let left_col = extract_column_from_expr(left);
- let right_col = extract_column_from_expr(right);
- if let (Some(col), Some(_)) = (&left_col, right.as_literal()) {
-
column_predicates.entry(col.clone()).or_default().push(pred);
- } else if let (Some(_), Some(col)) = (left.as_literal(),
&right_col) {
-
column_predicates.entry(col.clone()).or_default().push(pred);
- } else {
- other_predicates.push(pred);
- }
+ use Operator::*;
Review Comment:
There is an optimization here - remove cloning, but I'm hitting the annoying
rustfmt error (not sure why they decided it's a good idea for a code formatter
to fail on valid code):
```
Error[internal]: left behind trailing whitespace
```
##########
datafusion/optimizer/src/simplify_expressions/simplify_predicates.rs:
##########
@@ -52,28 +52,27 @@ pub fn simplify_predicates(predicates: Vec<Expr>) ->
Result<Vec<Expr>> {
let mut other_predicates = Vec::new();
for pred in predicates {
- match &pred {
- Expr::BinaryExpr(BinaryExpr {
- left,
- op:
- Operator::Gt
- | Operator::GtEq
- | Operator::Lt
- | Operator::LtEq
- | Operator::Eq,
- right,
- }) => {
- let left_col = extract_column_from_expr(left);
- let right_col = extract_column_from_expr(right);
- if let (Some(col), Some(_)) = (&left_col, right.as_literal()) {
-
column_predicates.entry(col.clone()).or_default().push(pred);
- } else if let (Some(_), Some(col)) = (left.as_literal(),
&right_col) {
-
column_predicates.entry(col.clone()).or_default().push(pred);
- } else {
- other_predicates.push(pred);
- }
+ use Operator::*;
Review Comment:
There is an optimization here - removed cloning, but I'm hitting the
annoying rustfmt error (not sure why they decided it's a good idea for a code
formatter to fail on valid code):
```
Error[internal]: left behind trailing whitespace
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
