englefly commented on code in PR #25180:
URL: https://github.com/apache/doris/pull/25180#discussion_r1354594753
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyArithmeticComparisonRule.java:
##########
@@ -40,68 +58,90 @@
public class SimplifyArithmeticComparisonRule extends
AbstractExpressionRewriteRule {
public static final SimplifyArithmeticComparisonRule INSTANCE = new
SimplifyArithmeticComparisonRule();
- @Override
- public Expression visit(Expression expr, ExpressionRewriteContext context)
{
- return expr;
- }
+ // don't rearrange multiplication because divide may loss precision
+ final Map<Class<? extends Expression>, Class<? extends Expression>>
rearrangementMap = ImmutableMap
+ .<Class<? extends Expression>, Class<? extends
Expression>>builder()
+ .put(Add.class, Subtract.class)
+ .put(Subtract.class, Add.class)
+ .put(Divide.class, Multiply.class)
+ .put(YearsSub.class, YearsAdd.class)
+ .put(YearsAdd.class, YearsSub.class)
+ .put(MonthsSub.class, MonthsAdd.class)
+ .put(MonthsAdd.class, MonthsSub.class)
+ .put(WeeksSub.class, WeeksAdd.class)
+ .put(WeeksAdd.class, WeeksSub.class)
+ .put(DaysSub.class, DaysAdd.class)
+ .put(DaysAdd.class, DaysSub.class)
+ .put(HoursSub.class, HoursAdd.class)
+ .put(HoursAdd.class, HoursSub.class)
+ .put(MinutesSub.class, MinutesAdd.class)
+ .put(MinutesAdd.class, MinutesSub.class)
+ .put(SecondsSub.class, SecondsAdd.class)
+ .put(SecondsAdd.class, SecondsSub.class)
+ .build();
- private Expression process(ComparisonPredicate predicate) {
- Expression left = predicate.left();
- Expression right = predicate.right();
- if (TypeUtils.isAddOrSubtract(left)) {
- Expression p = left.child(1);
- if (p.isConstant()) {
- if (TypeUtils.isAdd(left)) {
- right = new Subtract(right, p);
- }
- if (TypeUtils.isSubtract(left)) {
- right = new Add(right, p);
- }
- left = left.child(0);
+ @Override
+ public Expression visitComparisonPredicate(ComparisonPredicate comparison,
ExpressionRewriteContext context) {
+ ComparisonPredicate newComparison = comparison;
+ if (couldRearrange(comparison)) {
+ newComparison = normalize(comparison);
+ if (newComparison == null) {
+ return comparison;
}
- }
- if (TypeUtils.isDivide(left)) {
- Expression p = left.child(1);
- if (p.isLiteral()) {
- right = new Multiply(right, p);
- left = left.child(0);
- if (p.toString().startsWith("-")) {
- Expression tmp = right;
- right = left;
- left = tmp;
- }
+ try {
+ List<Expression> children =
tryRearrangeChildren(newComparison.left(), newComparison.right());
+ newComparison = (ComparisonPredicate)
newComparison.withChildren(children);
+ } catch (Exception e) {
+ return comparison;
}
}
- if (left != predicate.left() || right != predicate.right()) {
- predicate = (ComparisonPredicate) predicate.withChildren(left,
right);
- return TypeCoercionUtils.processComparisonPredicate(predicate);
- } else {
- return predicate;
- }
+ return TypeCoercionUtils.processComparisonPredicate(newComparison);
}
- @Override
- public Expression visitGreaterThan(GreaterThan greaterThan,
ExpressionRewriteContext context) {
- return process(greaterThan);
+ private boolean couldRearrange(ComparisonPredicate cmp) {
+ return rearrangementMap.containsKey(cmp.left().getClass())
+ && !cmp.left().isConstant()
+ &&
cmp.left().children().stream().anyMatch(Expression::isConstant);
}
- @Override
- public Expression visitGreaterThanEqual(GreaterThanEqual greaterThanEqual,
ExpressionRewriteContext context) {
- return process(greaterThanEqual);
- }
+ private List<Expression> tryRearrangeChildren(Expression left, Expression
right) throws Exception {
+ if (!left.child(1).isLiteral()) {
+ throw new RuntimeException(String.format("Expected literal when
arranging children for Expr %s", left));
+ }
+ Literal leftLiteral = (Literal) left.child(1);
+ Expression leftExpr = left.child(0);
- @Override
- public Expression visitEqualTo(EqualTo equalTo, ExpressionRewriteContext
context) {
- return process(equalTo);
- }
+ Class<? extends Expression> oppositeOperator =
rearrangementMap.get(left.getClass());
+ Expression newChild =
oppositeOperator.getConstructor(Expression.class, Expression.class)
+ .newInstance(right, leftLiteral);
- @Override
- public Expression visitLessThan(LessThan lessThan,
ExpressionRewriteContext context) {
- return process(lessThan);
+ if (left instanceof Divide && leftLiteral.compareTo(new
IntegerLiteral(0)) < 0) {
+ // Multiplying by a negative number will change the operator.
+ return Arrays.asList(newChild, leftExpr);
+ }
+ return Arrays.asList(leftExpr, newChild);
}
- @Override
- public Expression visitLessThanEqual(LessThanEqual lessThanEqual,
ExpressionRewriteContext context) {
- return process(lessThanEqual);
+ // Ensure that the second child must be Literal, such as
+ private @Nullable ComparisonPredicate normalize(ComparisonPredicate
comparison) {
Review Comment:
It would be better to make it public, because we could use it to generate
more effecient Runtime filter
--
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]
