You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2023/01/04 03:10:21 UTC
[doris] branch master updated: [fix](Nereids) SimplifyArithmeticRule generate wrong expression after process (#15580)
This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 7728794b4a [fix](Nereids) SimplifyArithmeticRule generate wrong expression after process (#15580)
7728794b4a is described below
commit 7728794b4a4820465e1a548c8787ebce7cbcf8ac
Author: morrySnow <10...@users.noreply.github.com>
AuthorDate: Wed Jan 4 11:10:15 2023 +0800
[fix](Nereids) SimplifyArithmeticRule generate wrong expression after process (#15580)
in the case of 'a / b', if a is constant, after apple SimplifyArithmeticRule, expression will be convert to 'b * a' by mistake.
---
.../rules/expression/rewrite/rules/SimplifyArithmeticRule.java | 9 ++++++++-
.../org/apache/doris/nereids/trees/expressions/Expression.java | 2 +-
.../doris/nereids/trees/expressions/functions/agg/Count.java | 5 +++++
.../rules/expression/rewrite/SimplifyArithmeticRuleTest.java | 2 ++
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/SimplifyArithmeticRule.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/SimplifyArithmeticRule.java
index 00c9189117..d85d53fadc 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/SimplifyArithmeticRule.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/SimplifyArithmeticRule.java
@@ -39,6 +39,8 @@ import java.util.Optional;
* a + 1 + b - 2 - ((c - d) + 1) => a + b - c + d + (1 - 2 - 1)
* After `FoldConstantRule`:
* a + b - c + d + (1 - 2 - 1) => a + b - c + d - 2
+ *
+ * TODO: handle cases like: '1 - IA < 1' to 'IA > 0'
*/
public class SimplifyArithmeticRule extends AbstractExpressionRewriteRule {
public static final SimplifyArithmeticRule INSTANCE = new SimplifyArithmeticRule();
@@ -100,7 +102,12 @@ public class SimplifyArithmeticRule extends AbstractExpressionRewriteRule {
}
return Operand.of(true, expr);
});
- variables.add(Operand.of(!isOpposite, c.get().expression));
+ boolean firstVariableFlag = variables.isEmpty() || variables.get(0).flag;
+ if (isOpposite || firstVariableFlag) {
+ variables.add(Operand.of(!isOpposite, c.get().expression));
+ } else {
+ variables.add(0, Operand.of(!isOpposite, c.get().expression));
+ }
}
Optional<Operand> result = variables.stream().reduce((x, y) -> !y.flag
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java
index 6668bd2ad2..37e98a0baa 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java
@@ -122,7 +122,7 @@ public abstract class Expression extends AbstractTreeNode<Expression> implements
/**
* Whether the expression is a constant.
*/
- public final boolean isConstant() {
+ public boolean isConstant() {
if (this instanceof LeafExpression) {
return this instanceof Literal;
} else {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Count.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Count.java
index 1dfe0cca17..7e38248c49 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Count.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Count.java
@@ -65,6 +65,11 @@ public class Count extends AggregateFunction implements AlwaysNotNullable, Custo
return FunctionSignature.of(BigIntType.INSTANCE, (List) getArgumentsTypes());
}
+ @Override
+ public boolean isConstant() {
+ return false;
+ }
+
@Override
protected List<DataType> intermediateTypes() {
return ImmutableList.of(BigIntType.INSTANCE);
diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rewrite/SimplifyArithmeticRuleTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rewrite/SimplifyArithmeticRuleTest.java
index 68a8efbbe7..a31107dca2 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rewrite/SimplifyArithmeticRuleTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rewrite/SimplifyArithmeticRuleTest.java
@@ -36,6 +36,8 @@ public class SimplifyArithmeticRuleTest extends ExpressionRewriteTestHelper {
assertRewriteAfterTypeCoercion("IA", "IA");
assertRewriteAfterTypeCoercion("IA + 1", "IA + 1");
assertRewriteAfterTypeCoercion("IA + IB", "IA + IB");
+ assertRewriteAfterTypeCoercion("1 * 3 / IA", "(3.0 / cast(IA as DOUBLE))");
+ assertRewriteAfterTypeCoercion("1 - IA", "1 - IA");
assertRewriteAfterTypeCoercion("1 + 1", "2");
assertRewriteAfterTypeCoercion("IA + 2 - 1", "cast(IA as bigint) + 1");
assertRewriteAfterTypeCoercion("IA + 2 - (1 - 1)", "cast(IA as bigint) + 2");
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org