This is an automated email from the ASF dual-hosted git repository.
kxiao pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new fde77e787ff [fix](Nereids) some expression not cast in InPredicate
(#24987)
fde77e787ff is described below
commit fde77e787ff32b5f7bc5aed5be9c7d828f84373b
Author: morrySnow <[email protected]>
AuthorDate: Wed Sep 27 20:56:19 2023 +0800
[fix](Nereids) some expression not cast in InPredicate (#24987)
1. should use castIfNotSameType in InPredicate and CaseWhen
2. castIfNotSameType should cast all type if two side is not exactly same
picked from master
PR: #24680
commit id: 320fc1481acbdc521961e74d0b23e321d66cce63
---
.../expression/rules/SimplifyInPredicate.java | 27 ++++++++++++++++++----
.../expressions/literal/StringLikeLiteral.java | 19 +++++++++++++++
.../org/apache/doris/nereids/types/CharType.java | 6 -----
.../org/apache/doris/nereids/types/StringType.java | 6 -----
.../apache/doris/nereids/types/VarcharType.java | 6 -----
.../doris/nereids/util/TypeCoercionUtils.java | 13 ++++-------
.../nereids/datasets/ssb/SSBJoinReorderTest.java | 4 ++--
.../rules/expression/SimplifyInPredicateTest.java | 7 ++++--
.../doris/nereids/util/TypeCoercionUtilsTest.java | 15 ++++++++++--
.../nereids_p0/join/bucket_shuffle_join.groovy | 2 +-
10 files changed, 66 insertions(+), 39 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyInPredicate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyInPredicate.java
index 8feb52cd4bc..846cdd2e5d2 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyInPredicate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyInPredicate.java
@@ -24,8 +24,9 @@ import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.InPredicate;
import org.apache.doris.nereids.trees.expressions.literal.DateTimeV2Literal;
import org.apache.doris.nereids.trees.expressions.literal.DateV2Literal;
+import org.apache.doris.nereids.types.DateTimeV2Type;
-import com.google.common.collect.Lists;
+import com.google.common.collect.ImmutableList;
import java.util.List;
@@ -46,11 +47,23 @@ public class SimplifyInPredicate extends
AbstractExpressionRewriteRule {
List<Expression> literals = expr.children().subList(1,
expr.children().size());
if (literals.stream().allMatch(literal -> literal
instanceof DateTimeV2Literal
&&
canLosslessConvertToDateV2Literal((DateTimeV2Literal) literal))) {
- List<Expression> children = Lists.newArrayList();
+ ImmutableList.Builder<Expression> children =
ImmutableList.builder();
children.add(cast.child());
- literals.stream().forEach(
- l ->
children.add(convertToDateV2Literal((DateTimeV2Literal) l)));
- return expr.withChildren(children);
+ literals.forEach(l ->
children.add(convertToDateV2Literal((DateTimeV2Literal) l)));
+ return expr.withChildren(children.build());
+ }
+ } else if (cast.child().getDataType().isDateTimeV2Type()
+ && expr.child(1) instanceof DateTimeV2Literal) {
+ List<Expression> literals = expr.children().subList(1,
expr.children().size());
+ DateTimeV2Type compareType = (DateTimeV2Type)
cast.child().getDataType();
+ if (literals.stream().allMatch(literal -> literal
instanceof DateTimeV2Literal
+ && canLosslessConvertToLowScaleLiteral(
+ (DateTimeV2Literal) literal,
compareType.getScale()))) {
+ ImmutableList.Builder<Expression> children =
ImmutableList.builder();
+ children.add(cast.child());
+ literals.forEach(l -> children.add(new
DateTimeV2Literal(compareType,
+ ((DateTimeV2Literal) l).getStringValue())));
+ return expr.withChildren(children.build());
}
}
}
@@ -75,4 +88,8 @@ public class SimplifyInPredicate extends
AbstractExpressionRewriteRule {
private DateV2Literal convertToDateV2Literal(DateTimeV2Literal literal) {
return new DateV2Literal(literal.getYear(), literal.getMonth(),
literal.getDay());
}
+
+ private static boolean
canLosslessConvertToLowScaleLiteral(DateTimeV2Literal literal, int targetScale)
{
+ return literal.getMicroSecond() % (1L << (DateTimeV2Type.MAX_SCALE -
targetScale)) == 0;
+ }
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/StringLikeLiteral.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/StringLikeLiteral.java
index 480ac996a16..5adf9f8623a 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/StringLikeLiteral.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/StringLikeLiteral.java
@@ -19,6 +19,8 @@ package org.apache.doris.nereids.trees.expressions.literal;
import org.apache.doris.nereids.types.DataType;
+import java.util.Objects;
+
/** StringLikeLiteral. */
public abstract class StringLikeLiteral extends Literal {
public final String value;
@@ -43,4 +45,21 @@ public abstract class StringLikeLiteral extends Literal {
}
return (double) v;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof StringLikeLiteral)) {
+ return false;
+ }
+ StringLikeLiteral that = (StringLikeLiteral) o;
+ return Objects.equals(value, that.value);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(value);
+ }
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/CharType.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/CharType.java
index 144e161cbea..15ed345a339 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/CharType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/CharType.java
@@ -19,7 +19,6 @@ package org.apache.doris.nereids.types;
import org.apache.doris.catalog.ScalarType;
import org.apache.doris.catalog.Type;
-import org.apache.doris.nereids.types.coercion.AbstractDataType;
import org.apache.doris.nereids.types.coercion.CharacterType;
import java.util.Objects;
@@ -52,11 +51,6 @@ public class CharType extends CharacterType {
return ScalarType.createChar(len);
}
- @Override
- public boolean acceptsType(AbstractDataType other) {
- return other instanceof CharType;
- }
-
@Override
public String simpleString() {
return "char";
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StringType.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StringType.java
index 8658278e073..9e130963854 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StringType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StringType.java
@@ -18,7 +18,6 @@
package org.apache.doris.nereids.types;
import org.apache.doris.catalog.Type;
-import org.apache.doris.nereids.types.coercion.AbstractDataType;
import org.apache.doris.nereids.types.coercion.CharacterType;
/**
@@ -42,11 +41,6 @@ public class StringType extends CharacterType {
return Type.STRING;
}
- @Override
- public boolean acceptsType(AbstractDataType other) {
- return other instanceof StringType || other instanceof VarcharType;
- }
-
@Override
public String simpleString() {
return "string";
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/VarcharType.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/VarcharType.java
index 5f7d7708c2f..0408dcf3ffa 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/VarcharType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/VarcharType.java
@@ -19,7 +19,6 @@ package org.apache.doris.nereids.types;
import org.apache.doris.catalog.ScalarType;
import org.apache.doris.catalog.Type;
-import org.apache.doris.nereids.types.coercion.AbstractDataType;
import org.apache.doris.nereids.types.coercion.CharacterType;
import java.util.Objects;
@@ -54,11 +53,6 @@ public class VarcharType extends CharacterType {
return catalogDataType;
}
- @Override
- public boolean acceptsType(AbstractDataType other) {
- return other instanceof VarcharType || other instanceof StringType;
- }
-
@Override
public String simpleString() {
return "varchar";
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java
index dc3bbf78ae8..2adf9c2a4bb 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java
@@ -243,8 +243,7 @@ public class TypeCoercionUtils {
if (input.isNullLiteral()) {
return new NullLiteral(targetType);
} else if (input.getDataType().equals(targetType) ||
isSubqueryAndDataTypeIsBitmap(input)
- || (isVarCharOrStringType(input.getDataType())
- && isVarCharOrStringType(targetType))) {
+ || (input.getDataType().isStringLikeType() &&
targetType.isStringLikeType())) {
return input;
} else {
checkCanCastTo(input.getDataType(), targetType);
@@ -256,10 +255,6 @@ public class TypeCoercionUtils {
return input instanceof SubqueryExpr &&
input.getDataType().isBitmapType();
}
- private static boolean isVarCharOrStringType(DataType dataType) {
- return dataType instanceof VarcharType || dataType instanceof
StringType;
- }
-
private static boolean canCastTo(DataType input, DataType target) {
return Type.canCastTo(input.toCatalogDataType(),
target.toCatalogDataType());
}
@@ -709,7 +704,7 @@ public class TypeCoercionUtils {
return optionalCommonType
.map(commonType -> {
List<Expression> newChildren =
inPredicate.children().stream()
- .map(e -> TypeCoercionUtils.castIfNotMatchType(e,
commonType))
+ .map(e -> TypeCoercionUtils.castIfNotSameType(e,
commonType))
.collect(Collectors.toList());
return inPredicate.withChildren(newChildren);
})
@@ -735,7 +730,7 @@ public class TypeCoercionUtils {
List<Expression> newChildren
= caseWhen.getWhenClauses().stream()
.map(wc -> {
- Expression valueExpr =
TypeCoercionUtils.castIfNotMatchType(
+ Expression valueExpr =
TypeCoercionUtils.castIfNotSameType(
wc.getResult(), commonType);
// we must cast every child to the common
type, and then
// FoldConstantRuleOnFe can eliminate some
branches and direct
@@ -748,7 +743,7 @@ public class TypeCoercionUtils {
.collect(Collectors.toList());
caseWhen.getDefaultValue()
.map(dv -> {
- Expression defaultExpr =
TypeCoercionUtils.castIfNotMatchType(dv, commonType);
+ Expression defaultExpr =
TypeCoercionUtils.castIfNotSameType(dv, commonType);
if
(!defaultExpr.getDataType().equals(commonType)) {
defaultExpr = new Cast(defaultExpr,
commonType);
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/datasets/ssb/SSBJoinReorderTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/datasets/ssb/SSBJoinReorderTest.java
index 49c3636a4f6..ca0c49d6d7f 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/datasets/ssb/SSBJoinReorderTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/datasets/ssb/SSBJoinReorderTest.java
@@ -39,7 +39,7 @@ public class SSBJoinReorderTest extends SSBTestBase
implements MemoPatternMatchS
ImmutableList.of(
"(c_region = 'AMERICA')",
"(s_region = 'AMERICA')",
- "p_mfgr IN ('MFGR#2', 'MFGR#1')"
+ "p_mfgr IN ('MFGR#1', 'MFGR#2')"
)
);
}
@@ -58,7 +58,7 @@ public class SSBJoinReorderTest extends SSBTestBase
implements MemoPatternMatchS
"d_year IN (1997, 1998)",
"(c_region = 'AMERICA')",
"(s_region = 'AMERICA')",
- "p_mfgr IN ('MFGR#2', 'MFGR#1')"
+ "p_mfgr IN ('MFGR#1', 'MFGR#2')"
)
);
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyInPredicateTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyInPredicateTest.java
index 01502bac522..87c57889b2f 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyInPredicateTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyInPredicateTest.java
@@ -39,7 +39,11 @@ public class SimplifyInPredicateTest extends
ExpressionRewriteTestHelper {
));
Map<String, Slot> mem = Maps.newHashMap();
Expression rewrittenExpression = PARSER.parseExpression("cast(CA as
DATETIME) in ('1992-01-31 00:00:00', '1992-02-01 00:00:00')");
+ // after parse and type coercion: CAST(CAST(CA AS DATETIMEV2(0)) AS
DATETIMEV2(6)) IN ('1992-01-31 00:00:00.000000', '1992-02-01 00:00:00.000000')
rewrittenExpression =
typeCoercion(replaceUnboundSlot(rewrittenExpression, mem));
+ // after first rewrite: CAST(CA AS DATETIMEV2(0)) IN ('1992-01-31
00:00:00', '1992-02-01 00:00:00')
+ rewrittenExpression = executor.rewrite(rewrittenExpression, context);
+ // after second rewrite: CA IN ('1992-01-31', '1992-02-01')
rewrittenExpression = executor.rewrite(rewrittenExpression, context);
Expression expectedExpression = PARSER.parseExpression("CA in
(cast('1992-01-31' as date), cast('1992-02-01' as date))");
expectedExpression = replaceUnboundSlot(expectedExpression, mem);
@@ -47,7 +51,6 @@ public class SimplifyInPredicateTest extends
ExpressionRewriteTestHelper {
FoldConstantRule.INSTANCE
));
expectedExpression = executor.rewrite(expectedExpression, context);
- Assertions.assertEquals(expectedExpression.toSql(),
rewrittenExpression.toSql());
+ Assertions.assertEquals(expectedExpression, rewrittenExpression);
}
-
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/util/TypeCoercionUtilsTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/util/TypeCoercionUtilsTest.java
index 2fb3930ecf6..6b229a16fe2 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/util/TypeCoercionUtilsTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/util/TypeCoercionUtilsTest.java
@@ -23,9 +23,12 @@ import org.apache.doris.nereids.trees.expressions.Divide;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.Multiply;
import org.apache.doris.nereids.trees.expressions.Subtract;
+import org.apache.doris.nereids.trees.expressions.literal.CharLiteral;
import org.apache.doris.nereids.trees.expressions.literal.DecimalLiteral;
import org.apache.doris.nereids.trees.expressions.literal.DecimalV3Literal;
import org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.StringLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral;
import org.apache.doris.nereids.types.ArrayType;
import org.apache.doris.nereids.types.BigIntType;
import org.apache.doris.nereids.types.BitmapType;
@@ -692,9 +695,17 @@ public class TypeCoercionUtilsTest {
@Test
public void testCastIfNotSameType() {
Assertions.assertEquals(new DoubleLiteral(5L),
- TypeCoercionUtils.castIfNotMatchType(new DoubleLiteral(5L),
DoubleType.INSTANCE));
+ TypeCoercionUtils.castIfNotSameType(new DoubleLiteral(5L),
DoubleType.INSTANCE));
Assertions.assertEquals(new Cast(new DoubleLiteral(5L),
BooleanType.INSTANCE),
- TypeCoercionUtils.castIfNotMatchType(new DoubleLiteral(5L),
BooleanType.INSTANCE));
+ TypeCoercionUtils.castIfNotSameType(new DoubleLiteral(5L),
BooleanType.INSTANCE));
+ Assertions.assertEquals(new StringLiteral("varchar"),
+ TypeCoercionUtils.castIfNotSameType(new
VarcharLiteral("varchar"), StringType.INSTANCE));
+ Assertions.assertEquals(new StringLiteral("char"),
+ TypeCoercionUtils.castIfNotSameType(new CharLiteral("char",
4), StringType.INSTANCE));
+ Assertions.assertEquals(new CharLiteral("char", 4),
+ TypeCoercionUtils.castIfNotSameType(new CharLiteral("char",
4), VarcharType.createVarcharType(100)));
+ Assertions.assertEquals(new StringLiteral("string"),
+ TypeCoercionUtils.castIfNotSameType(new
StringLiteral("string"), VarcharType.createVarcharType(100)));
}
@Test
diff --git a/regression-test/suites/nereids_p0/join/bucket_shuffle_join.groovy
b/regression-test/suites/nereids_p0/join/bucket_shuffle_join.groovy
index 786a478d204..9fb3606d69a 100644
--- a/regression-test/suites/nereids_p0/join/bucket_shuffle_join.groovy
+++ b/regression-test/suites/nereids_p0/join/bucket_shuffle_join.groovy
@@ -72,7 +72,7 @@ suite("bucket-shuffle-join") {
explain {
sql("select * from shuffle_join_t1 t1 left join shuffle_join_t2 t2 on
t1.a = t2.c;")
contains "BUCKET_SHUFFLE"
- contains "BUCKET_SHFFULE_HASH_PARTITIONED: expr_cast(c as VARCHAR(*))"
+ contains "BUCKET_SHFFULE_HASH_PARTITIONED: c"
}
sql """ DROP TABLE IF EXISTS shuffle_join_t1 """
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
