Move parts of InListSimplifier simplify rules to Simplifier
#9628
Conversation
Signed-off-by: jayzhan211 <[email protected]>
Signed-off-by: jayzhan211 <[email protected]>
Signed-off-by: jayzhan211 <[email protected]>
Signed-off-by: jayzhan211 <[email protected]>
| left, | ||
| op: Operator::Or, | ||
| right, | ||
| }) if are_inlist_and_eq(left.as_ref(), right.as_ref()) => { |
There was a problem hiding this comment.
After deref pattern is supported in stable rust, we can done the matching easier
There was a problem hiding this comment.
Thank you @jayzhan211 -- this looks like a nice incremental step to me
| .rewrite(&mut guarantee_rewriter) | ||
| .data()? | ||
| // run both passes twice to try an minimize simplifications that we missed | ||
| .rewrite(&mut const_evaluator) | ||
| .data()? | ||
| .rewrite(&mut simplifier) | ||
| .data()? | ||
| // shorten inlist should be started after other inlist rules are applied | ||
| .rewrite(&mut shorten_in_list_simplifier) |
| let left = as_inlist(left); | ||
| let right = as_inlist(right); | ||
| if let (Some(lhs), Some(rhs)) = (left, right) { | ||
| lhs.expr.try_into_col().is_ok() |
There was a problem hiding this comment.
try_into_col().is_ok() I think returns an error/string which is then ignored
Maybe we could do something like matches!(lhs.expr, Expr:Column(..) 🤔
| Expr::BinaryExpr(BinaryExpr { left, op, right }) if *op == Operator::Eq => { | ||
| match (left.as_ref(), right.as_ref()) { | ||
| (Expr::Column(_), Expr::Literal(_)) => Some(Cow::Owned(InList { | ||
| expr: left.clone(), |
There was a problem hiding this comment.
it would be great to avoid these clones but since this PR just moves the code we can do it as a follow on PR I think
There was a problem hiding this comment.
Of course, removing cloned is my main goal, but it does not seem trivial now so not done in this PR
There was a problem hiding this comment.
I agree -- thank you @jayzhan211
Signed-off-by: jayzhan211 <[email protected]>
Which issue does this PR close?
Parts of #9140
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?