alamb commented on a change in pull request #1401:
URL: https://github.com/apache/arrow-datafusion/pull/1401#discussion_r780489370
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -554,212 +416,250 @@ impl<'a> Simplifier<'a> {
false
}
- fn boolean_folding_for_or(
- const_bool: &Option<bool>,
- bool_expr: Box<Expr>,
- left_right_order: bool,
- ) -> Expr {
- // See if we can fold 'const_bool OR bool_expr' to a constant boolean
- match const_bool {
- // TRUE or expr (including NULL) = TRUE
- Some(true) => Expr::Literal(ScalarValue::Boolean(Some(true))),
- // FALSE or expr (including NULL) = expr
- Some(false) => *bool_expr,
- None => match *bool_expr {
- // NULL or TRUE = TRUE
- Expr::Literal(ScalarValue::Boolean(Some(true))) => {
- Expr::Literal(ScalarValue::Boolean(Some(true)))
- }
- // NULL or FALSE = NULL
- Expr::Literal(ScalarValue::Boolean(Some(false))) => {
- Expr::Literal(ScalarValue::Boolean(None))
- }
- // NULL or NULL = NULL
- Expr::Literal(ScalarValue::Boolean(None)) => {
- Expr::Literal(ScalarValue::Boolean(None))
- }
- // NULL or expr can be either NULL or TRUE
- // So let us not rewrite it
- _ => {
- let mut left =
-
Box::new(Expr::Literal(ScalarValue::Boolean(*const_bool)));
- let mut right = bool_expr;
- if !left_right_order {
- std::mem::swap(&mut left, &mut right);
- }
-
- Expr::BinaryExpr {
- left,
- op: Operator::Or,
- right,
- }
- }
- },
+ /// Returns true if expr is nullable
+ fn nullable(&self, expr: &Expr) -> Result<bool> {
+ for schema in &self.schemas {
Review comment:
Here is what I got to:
```rust
/// Returns true if expr is nullable
fn nullable(&self, expr: &Expr) -> Result<bool> {
self.schemas.iter()
.find_map(|schema| {
// expr may be from another input, so ignore errors
// by converting to None to keep trying
expr.nullable(schema.as_ref()).ok()
})
.ok_or_else(|| {
// This means we weren't able to compute `Expr::nullable`
with
// *any* input schemas, signalling a problem
DataFusionError::Internal(format!(
"Could not find find columns in '{}' during simplify",
expr
))
})
}
```
which isn't quite a one linter, but is a more functional style. Updated in
8642e56bb
--
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]
