Fix bugs with nullability during rewrites: Combine simplify and Simplifier

[alamb]

Builds on #1376

Re #1160 (more sophisticated expression simplification)

Note that this PR adds test coverage while removing code 🎉

Rationale

There is redundant code in simplify and Simplifier which makes it hard to know what is simplified and what is not

Previously, the code had several places where it had to put Expr back together if some rewrite wasn't possible. The new formulation separates the checks from the rewrites, making everything consistent and I think easier to understand what rewrites are happening

Changes

1. Combine simplify and Simplifier code
2. Remove rules in Simplifier that are redundant with ConstEvaluator
3. Fixes several rewrites to only happen if the expressions are not nullable (e.g. A / A = 1, only if A is not null)

[alamb]

as c1 is a string column, it is a runtime error to try and evaluate c1 = true -- I fixed the case and removed the nonsensical tests

[alamb]

this was removed for the same reason as the tests above - they don't make sense to compare a string column to a boolean constant (it will get a run time error)

[alamb]

not sure why this test was here -- now that constant propagation is implemented, these cases do indeed work

[alamb]

Now the simplification rules are in one (large) standardized match statement which I find easier to understand what is going on

[alamb]

the non redundant rules are now found in rewrite below

[alamb]

These first 4 cases are now what was in boolean_folding_for_or

[alamb]

on master the nullable() check is not performed, leading to incorrect results in some cases

[alamb]

These first 4 cases are now what was in boolean_folding_for_and

[alamb]

new test coverage showing one can't simplify A / A unless you know A isn't null

[alamb]

additional coverage showing that this rewrite is not possible unless you know c1 < 6 can not be null

[alamb]

FYI @jgoday @matthewmturner @NGA-TRAN and @houqp -- this PR simplifies the simplification code, which you have all been a part of.

[alamb]

@houqp or @Dandandan -- any concern if I merge this PR?

[Dandandan]

I found a one liner, something ike?:

return self.schemas.iter().find_map(Result::ok)

[alamb]

Here is what I got to:

    /// 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 8642e56

[houqp]

LGTM, I don't have the full context, so only skimmed through the code :)

[alamb]

LGTM, I don't have the full context, so only skimmed through the code :)

Thanks @houqp ! I am fairly confident that these rewrites are correct (and there is good test coverage of this code) -- I think this expression simplification code is one of the key optimizations offered by DataFusion so I want to make it as easy to work with (and extend) as possible.

[alamb]

If anyone else has a chance to review and has comments, please let me know