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 2022/05/18 14:45:00 UTC
[incubator-doris] branch master updated: [fix](planner) unnecessary cast will be added on children in CaseExpr sometimes (#9600)
This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
The following commit(s) were added to refs/heads/master by this push:
new a3183ec45c [fix](planner) unnecessary cast will be added on children in CaseExpr sometimes (#9600)
a3183ec45c is described below
commit a3183ec45cf9ef6a02d7e03b62dcfde7583de1d7
Author: morrySnow <10...@users.noreply.github.com>
AuthorDate: Wed May 18 22:44:51 2022 +0800
[fix](planner) unnecessary cast will be added on children in CaseExpr sometimes (#9600)
unnecessary cast will be added on children in CaseExpr because use symbolized equal to compare to `Expr`'s type.
it will lead to expression compare mistake and then lead to expression substitute failed when use `ExprSubstitutionMap`
---
.../java/org/apache/doris/analysis/CaseExpr.java | 8 ++--
.../org/apache/doris/analysis/CaseExprTest.java | 44 ++++++++++++++++++++++
.../data/correctness/test_case_when.out | 6 +++
.../suites/correctness/test_case_when.groovy | 14 ++++++-
4 files changed, 67 insertions(+), 5 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CaseExpr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CaseExpr.java
index 8a41e91d35..b4daa214ee 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CaseExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CaseExpr.java
@@ -235,25 +235,25 @@ public class CaseExpr extends Expr {
// Add casts to case expr to compatible type.
if (hasCaseExpr) {
// Cast case expr.
- if (children.get(0).type != whenType) {
+ if (!children.get(0).getType().equals(whenType)) {
castChild(whenType, 0);
}
// Add casts to when exprs to compatible type.
for (int i = loopStart; i < loopEnd; i += 2) {
- if (children.get(i).type != whenType) {
+ if (!children.get(i).getType().equals(whenType)) {
castChild(whenType, i);
}
}
}
// Cast then exprs to compatible type.
for (int i = loopStart + 1; i < children.size(); i += 2) {
- if (children.get(i).type != returnType) {
+ if (!children.get(i).getType().equals(returnType)) {
castChild(returnType, i);
}
}
// Cast else expr to compatible type.
if (hasElseExpr) {
- if (children.get(children.size() - 1).type != returnType) {
+ if (!children.get(children.size() - 1).getType().equals(returnType)) {
castChild(returnType, children.size() - 1);
}
}
diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/CaseExprTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/CaseExprTest.java
index c768be5030..72bf214c36 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/analysis/CaseExprTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/CaseExprTest.java
@@ -17,12 +17,16 @@
package org.apache.doris.analysis;
+import org.apache.doris.catalog.PrimitiveType;
+import org.apache.doris.catalog.ScalarType;
+
import com.clearspring.analytics.util.Lists;
import mockit.Expectations;
import mockit.Injectable;
import org.junit.Assert;
import org.junit.Test;
+import java.lang.reflect.Constructor;
import java.util.List;
public class CaseExprTest {
@@ -65,4 +69,44 @@ public class CaseExprTest {
};
Assert.assertTrue(caseExpr3.isNullable());
}
+
+ @Test
+ public void testTypeCast(
+ @Injectable Expr caseExpr,
+ @Injectable Expr whenExpr,
+ @Injectable Expr thenExpr,
+ @Injectable Expr elseExpr) throws Exception {
+ CaseWhenClause caseWhenClause = new CaseWhenClause(whenExpr, thenExpr);
+ List<CaseWhenClause> caseWhenClauseList = Lists.newArrayList();
+ caseWhenClauseList.add(caseWhenClause);
+ CaseExpr expr = new CaseExpr(caseExpr, caseWhenClauseList, elseExpr);
+ Class<?> clazz = Class.forName("org.apache.doris.catalog.ScalarType");
+ Constructor<?> constructor = clazz.getDeclaredConstructor(PrimitiveType.class);
+ constructor.setAccessible(true);
+ ScalarType intType = (ScalarType) constructor.newInstance(PrimitiveType.INT);
+ ScalarType tinyIntType = (ScalarType) constructor.newInstance(PrimitiveType.TINYINT);
+ new Expectations() {
+ {
+ caseExpr.getType();
+ result = intType;
+
+ whenExpr.getType();
+ result = ScalarType.createType(PrimitiveType.INT);
+ whenExpr.castTo(intType);
+ times = 0;
+
+ thenExpr.getType();
+ result = tinyIntType;
+ thenExpr.castTo(tinyIntType);
+ times = 0;
+
+ elseExpr.getType();
+ result = ScalarType.createType(PrimitiveType.TINYINT);
+ elseExpr.castTo(tinyIntType);
+ times = 0;
+ }
+ };
+ Analyzer analyzer = new Analyzer(null, null);
+ expr.analyzeImpl(analyzer);
+ }
}
diff --git a/regression-test/data/correctness/test_case_when.out b/regression-test/data/correctness/test_case_when.out
index d4db120b76..fba725b132 100644
--- a/regression-test/data/correctness/test_case_when.out
+++ b/regression-test/data/correctness/test_case_when.out
@@ -3,3 +3,9 @@
2 01 -1
3 00 1
+-- !select_agg --
+1 90020004
+2 45010002
+3 90020004
+4 90020004
+
diff --git a/regression-test/suites/correctness/test_case_when.groovy b/regression-test/suites/correctness/test_case_when.groovy
index f5afe3026f..bdade75ead 100644
--- a/regression-test/suites/correctness/test_case_when.groovy
+++ b/regression-test/suites/correctness/test_case_when.groovy
@@ -64,4 +64,16 @@ suite("test_case_when") {
and channel_id = '00'
group by hour_time, station_type;
"""
-}
\ No newline at end of file
+
+ qt_select_agg """
+ SELECT
+ hour_time,
+ sum((CASE WHEN TRUE THEN merchant_id ELSE 0 END)) mid
+ FROM
+ dws_scan_qrcode_user_ts
+ GROUP BY
+ hour_time
+ ORDER BY
+ hour_time;
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org