This is an automated email from the ASF dual-hosted git repository.
yiguolei 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 429a3ed4df1 [fix](Nereids) simplify range produce true when reference
is nullable (#28386)
429a3ed4df1 is described below
commit 429a3ed4df1bd354b706a1679510540fbbb85855
Author: morrySnow <[email protected]>
AuthorDate: Thu Dec 14 18:10:17 2023 +0800
[fix](Nereids) simplify range produce true when reference is nullable
(#28386)
if reference is nullable, even if range is all, we should not return
true, but should return reference is not null. for example,
before simplify: c1 > 5 or c1 < 10
after simplify:
c1 is nullable: c1 IS NOT NULL
c1 is not nullable: TRUE
---
.../rules/expression/rules/SimplifyRange.java | 15 ++++++++--
.../rules/expression/SimplifyRangeTest.java | 32 ++++++++++++++++++++--
2 files changed, 42 insertions(+), 5 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java
index 2fd7f4167d8..c0cb834b2f4 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java
@@ -28,8 +28,10 @@ import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.GreaterThan;
import org.apache.doris.nereids.trees.expressions.GreaterThanEqual;
import org.apache.doris.nereids.trees.expressions.InPredicate;
+import org.apache.doris.nereids.trees.expressions.IsNull;
import org.apache.doris.nereids.trees.expressions.LessThan;
import org.apache.doris.nereids.trees.expressions.LessThanEqual;
+import org.apache.doris.nereids.trees.expressions.Not;
import org.apache.doris.nereids.trees.expressions.Or;
import org.apache.doris.nereids.trees.expressions.literal.BooleanLiteral;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
@@ -204,7 +206,7 @@ public class SimplifyRange extends
AbstractExpressionRewriteRule {
public static ValueDesc intersect(RangeValue range, DiscreteValue
discrete) {
DiscreteValue result = new DiscreteValue(discrete.reference,
discrete.expr);
discrete.values.stream().filter(x ->
range.range.contains(x)).forEach(result.values::add);
- if (result.values.size() > 0) {
+ if (!result.values.isEmpty()) {
return result;
}
return new EmptyValue(range.reference,
ExpressionUtils.and(range.expr, discrete.expr));
@@ -333,12 +335,19 @@ public class SimplifyRange extends
AbstractExpressionRewriteRule {
result.add(new LessThan(reference, range.upperEndpoint()));
}
}
- return result.isEmpty() ? BooleanLiteral.TRUE :
ExpressionUtils.and(result);
+ if (!result.isEmpty()) {
+ return ExpressionUtils.and(result);
+ } else if (reference.nullable()) {
+ // when reference is nullable, we should filter null slot.
+ return new Not(new IsNull(reference));
+ } else {
+ return BooleanLiteral.TRUE;
+ }
}
@Override
public String toString() {
- return range == null ? "UnknwonRange" : range.toString();
+ return range == null ? "UnknownRange" : range.toString();
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java
index 33a22dae78d..175a300eb1d 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java
@@ -67,11 +67,13 @@ public class SimplifyRangeTest {
assertRewrite("TA = 1 and TA > 10", "FALSE");
assertRewrite("TA > 5 or TA < 1", "TA > 5 or TA < 1");
assertRewrite("TA > 5 or TA > 1 or TA > 10", "TA > 1");
- assertRewrite("TA > 5 or TA > 1 or TA < 10", "TRUE");
+ assertRewrite("TA > 5 or TA > 1 or TA < 10", "TA IS NOT NULL");
+ assertRewriteNotNull("TA > 5 or TA > 1 or TA < 10", "TRUE");
assertRewrite("TA > 5 and TA > 1 and TA > 10", "TA > 10");
assertRewrite("TA > 5 and TA > 1 and TA < 10", "TA > 5 and TA < 10");
assertRewrite("TA > 1 or TA < 1", "TA > 1 or TA < 1");
- assertRewrite("TA > 1 or TA < 10", "TRUE");
+ assertRewrite("TA > 1 or TA < 10", "TA IS NOT NULL");
+ assertRewriteNotNull("TA > 1 or TA < 10", "TRUE");
assertRewrite("TA > 5 and TA < 10", "TA > 5 and TA < 10");
assertRewrite("TA > 5 and TA > 10", "TA > 10");
assertRewrite("TA > 5 + 1 and TA > 10", "TA > 5 + 1 and TA > 10");
@@ -115,6 +117,14 @@ public class SimplifyRangeTest {
Assertions.assertEquals(expectedExpression, rewrittenExpression);
}
+ private void assertRewriteNotNull(String expression, String expected) {
+ Map<String, Slot> mem = Maps.newHashMap();
+ Expression needRewriteExpression =
replaceNotNullUnboundSlot(PARSER.parseExpression(expression), mem);
+ Expression expectedExpression =
replaceNotNullUnboundSlot(PARSER.parseExpression(expected), mem);
+ Expression rewrittenExpression =
executor.rewrite(needRewriteExpression, context);
+ Assertions.assertEquals(expectedExpression, rewrittenExpression);
+ }
+
private Expression replaceUnboundSlot(Expression expression, Map<String,
Slot> mem) {
List<Expression> children = Lists.newArrayList();
boolean hasNewChildren = false;
@@ -133,6 +143,24 @@ public class SimplifyRangeTest {
return hasNewChildren ? expression.withChildren(children) : expression;
}
+ private Expression replaceNotNullUnboundSlot(Expression expression,
Map<String, Slot> mem) {
+ List<Expression> children = Lists.newArrayList();
+ boolean hasNewChildren = false;
+ for (Expression child : expression.children()) {
+ Expression newChild = replaceNotNullUnboundSlot(child, mem);
+ if (newChild != child) {
+ hasNewChildren = true;
+ }
+ children.add(newChild);
+ }
+ if (expression instanceof UnboundSlot) {
+ String name = ((UnboundSlot) expression).getName();
+ mem.putIfAbsent(name, new SlotReference(name,
getType(name.charAt(0)), false));
+ return mem.get(name);
+ }
+ return hasNewChildren ? expression.withChildren(children) : expression;
+ }
+
private DataType getType(char t) {
switch (t) {
case 'T':
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
