This is an automated email from the ASF dual-hosted git repository.
jakevin 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 983f948ec4 [fix](Nereids): fix SimplifyComparisonPredicate (#24899)
983f948ec4 is described below
commit 983f948ec4a74638555d449a92bc67ffa7884be7
Author: jakevin <jakevin...@gmail.com>
AuthorDate: Tue Sep 26 14:28:03 2023 +0800
[fix](Nereids): fix SimplifyComparisonPredicate (#24899)
#16061 exist BUG, it causes wrong cast
---
.../rules/SimplifyComparisonPredicate.java | 71 ++++------------------
.../rules/SimplifyComparisonPredicateTest.java | 7 ++-
2 files changed, 17 insertions(+), 61 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicate.java
index 5c6025f060..74627eae9f 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicate.java
@@ -17,7 +17,6 @@
package org.apache.doris.nereids.rules.expression.rules;
-import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.expression.AbstractExpressionRewriteRule;
import org.apache.doris.nereids.rules.expression.ExpressionRewriteContext;
import org.apache.doris.nereids.trees.expressions.And;
@@ -76,8 +75,14 @@ public class SimplifyComparisonPredicate extends
AbstractExpressionRewriteRule {
@Override
public Expression visitComparisonPredicate(ComparisonPredicate cp,
ExpressionRewriteContext context) {
- Expression left = rewrite(cp.left(), context);
- Expression right = rewrite(cp.right(), context);
+ cp = (ComparisonPredicate) visit(cp, context);
+
+ if (cp.left() instanceof Literal && !(cp.right() instanceof Literal)) {
+ cp = cp.commute();
+ }
+
+ Expression left = cp.left();
+ Expression right = cp.right();
// float like type: float, double
if (left.getDataType().isFloatLikeType() &&
right.getDataType().isFloatLikeType()) {
@@ -94,11 +99,7 @@ public class SimplifyComparisonPredicate extends
AbstractExpressionRewriteRule {
return processDateLikeTypeCoercion(cp, left, right);
}
- if (left != cp.left() || right != cp.right()) {
- return cp.withChildren(left, right);
- } else {
- return cp;
- }
+ return cp;
}
private static Expression processComparisonPredicateDateTimeV2Literal(
@@ -145,14 +146,6 @@ public class SimplifyComparisonPredicate extends
AbstractExpressionRewriteRule {
}
private Expression processDateLikeTypeCoercion(ComparisonPredicate cp,
Expression left, Expression right) {
- Expression originalRight = right;
- if (left instanceof DateLiteral) {
- cp = cp.commute();
- Expression temp = left;
- left = right;
- right = temp;
- }
-
if (left instanceof Cast && right instanceof DateLiteral) {
Cast cast = (Cast) left;
if (cast.child().getDataType() instanceof DateTimeType) {
@@ -164,8 +157,7 @@ public class SimplifyComparisonPredicate extends
AbstractExpressionRewriteRule {
if (cast.child().getDataType() instanceof DateTimeV2Type) {
if (right instanceof DateTimeV2Literal) {
left = cast.child();
- return processComparisonPredicateDateTimeV2Literal(cp,
left,
- (DateTimeV2Literal) right);
+ return processComparisonPredicateDateTimeV2Literal(cp,
left, (DateTimeV2Literal) right);
}
}
// datetime to datev2
@@ -200,9 +192,10 @@ public class SimplifyComparisonPredicate extends
AbstractExpressionRewriteRule {
// Date cp Date is not supported in BE storage engine. So cast to
DateTime
left = new Cast(left, DateTimeType.INSTANCE);
if (right instanceof DateLiteral) {
- right = migrateLiteralToDateTime((DateLiteral) originalRight);
+ DateLiteral l = (DateLiteral) right;
+ right = new DateTimeLiteral(l.getYear(), l.getMonth(),
l.getDay(), 0, 0, 0);
} else {
- right = new Cast(originalRight, DateTimeType.INSTANCE);
+ right = new Cast(right, DateTimeType.INSTANCE);
}
}
if (left != cp.left() || right != cp.right()) {
@@ -214,13 +207,6 @@ public class SimplifyComparisonPredicate extends
AbstractExpressionRewriteRule {
private Expression processFloatLikeTypeCoercion(ComparisonPredicate
comparisonPredicate,
Expression left, Expression right) {
- if (left instanceof Literal) {
- comparisonPredicate = comparisonPredicate.commute();
- Expression temp = left;
- left = right;
- right = temp;
- }
-
if (left instanceof Cast &&
left.child(0).getDataType().isIntegerLikeType()
&& (right instanceof DoubleLiteral || right instanceof
FloatLiteral)) {
Cast cast = (Cast) left;
@@ -234,13 +220,6 @@ public class SimplifyComparisonPredicate extends
AbstractExpressionRewriteRule {
private Expression processDecimalV3TypeCoercion(ComparisonPredicate
comparisonPredicate,
Expression left, Expression right) {
- if (left instanceof DecimalV3Literal) {
- comparisonPredicate = comparisonPredicate.commute();
- Expression temp = left;
- left = right;
- right = temp;
- }
-
if (left instanceof Cast && right instanceof DecimalV3Literal) {
Cast cast = (Cast) left;
left = cast.child();
@@ -348,30 +327,6 @@ public class SimplifyComparisonPredicate extends
AbstractExpressionRewriteRule {
}
}
- /*
- derive tree:
- DateLiteral
- |
- +--->DateTimeLiteral
- | |
- | +----->DateTimeV2Literal
- +--->DateV2Literal
- */
- private Expression migrateLiteralToDateTime(DateLiteral l) {
- if (l instanceof DateV2Literal) {
- return new DateTimeLiteral(l.getYear(), l.getMonth(), l.getDay(),
0, 0, 0);
- } else if (l instanceof DateTimeV2Literal) {
- DateTimeV2Literal dtv2 = (DateTimeV2Literal) l;
- return new DateTimeLiteral(dtv2.getYear(), dtv2.getMonth(),
dtv2.getDay(),
- dtv2.getHour(), dtv2.getMinute(), dtv2.getSecond());
- } else if (l instanceof DateTimeLiteral) {
- return l;
- } else if (l instanceof DateLiteral) {
- return new DateTimeLiteral(l.getYear(), l.getMonth(), l.getDay(),
0, 0, 0);
- }
- throw new AnalysisException("cannot convert" + l.toSql() + " to
DateTime");
- }
-
private Expression migrateToDateTime(DateTimeV2Literal l) {
return new DateTimeLiteral(l.getYear(), l.getMonth(), l.getDay(),
l.getHour(), l.getMinute(), l.getSecond());
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicateTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicateTest.java
index b06d914b65..898bc2b619 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicateTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicateTest.java
@@ -39,7 +39,8 @@ class SimplifyComparisonPredicateTest extends
ExpressionRewriteTestHelper {
@Test
void testSimplifyComparisonPredicateRule() {
- executor = new
ExpressionRuleExecutor(ImmutableList.of(SimplifyCastRule.INSTANCE,
SimplifyComparisonPredicate.INSTANCE));
+ executor = new ExpressionRuleExecutor(
+ ImmutableList.of(SimplifyCastRule.INSTANCE,
SimplifyComparisonPredicate.INSTANCE));
Expression dtv2 = new DateTimeV2Literal(1, 1, 1, 1, 1, 1, 0);
Expression dt = new DateTimeLiteral(1, 1, 1, 1, 1, 1);
@@ -67,10 +68,10 @@ class SimplifyComparisonPredicateTest extends
ExpressionRewriteTestHelper {
// DateTimeV2 -> Date
assertRewrite(
new GreaterThan(new Cast(d, DateTimeV2Type.SYSTEM_DEFAULT),
dtv2),
- new GreaterThan(new Cast(d, DateTimeType.INSTANCE), dt));
+ new GreaterThan(new Cast(d, DateTimeType.INSTANCE), new
DateTimeLiteral(1, 1, 1, 0, 0, 0)));
assertRewrite(
new LessThan(new Cast(d, DateTimeV2Type.SYSTEM_DEFAULT), dtv2),
- new LessThan(new Cast(d, DateTimeType.INSTANCE), dt));
+ new LessThan(new Cast(d, DateTimeType.INSTANCE), new
DateTimeLiteral(1, 1, 2, 0, 0, 0)));
assertRewrite(
new EqualTo(new Cast(d, DateTimeV2Type.SYSTEM_DEFAULT), dtv2),
new EqualTo(new Cast(d, DateTimeV2Type.SYSTEM_DEFAULT), dtv2));
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
