From 56c717ba48fd9045976c800bd3826f7e4bb95263 Mon Sep 17 00:00:00 2001 From: morrySnow Date: Mon, 16 May 2022 23:43:17 +0800 Subject: [PATCH 1/4] [fix](planner)unnecessary cast will be added on children in CaseExpr sometimes --- .../org/apache/doris/analysis/CaseExpr.java | 8 ++-- .../apache/doris/analysis/CaseExprTest.java | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+), 4 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 8a41e91d35f2ca..b4daa214ee557f 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 void analyzeImpl(Analyzer analyzer) throws AnalysisException { // 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 c768be50304a97..72bf214c361b4f 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 void testIsNullable(@Injectable Expr caseExpr, }; 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 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); + } } From aa56c820f2251d63777711f22b492df358d298d8 Mon Sep 17 00:00:00 2001 From: morrySnow Date: Tue, 17 May 2022 10:42:25 +0800 Subject: [PATCH 2/4] add regression test --- .../data/correctness/test_case_when.out | 6 ++++++ .../suites/correctness/test_case_when.groovy | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/regression-test/data/correctness/test_case_when.out b/regression-test/data/correctness/test_case_when.out index d4db120b762f22..9d11dac8320a61 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 f5afe3026fe7e3..bdade75ead3a9a 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; + """ +} From 06dcbba20c9a726f5e289b94bd701b5962229016 Mon Sep 17 00:00:00 2001 From: morrySnow Date: Wed, 18 May 2022 12:39:14 +0800 Subject: [PATCH 3/4] fix regression out file mistake --- regression-test/data/correctness/test_case_when.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/regression-test/data/correctness/test_case_when.out b/regression-test/data/correctness/test_case_when.out index 9d11dac8320a61..d5e54b6ce151d9 100644 --- a/regression-test/data/correctness/test_case_when.out +++ b/regression-test/data/correctness/test_case_when.out @@ -3,7 +3,7 @@ 2 01 -1 3 00 1 - -- !select_agg -- +-- !select_agg -- 1 90020004 2 45010002 3 90020004 From c97ed07d0718f9c0ca32177be14a9b35bf2bef93 Mon Sep 17 00:00:00 2001 From: morrySnow Date: Wed, 18 May 2022 16:49:54 +0800 Subject: [PATCH 4/4] fix regression out file mistake --- regression-test/data/correctness/test_case_when.out | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/regression-test/data/correctness/test_case_when.out b/regression-test/data/correctness/test_case_when.out index d5e54b6ce151d9..fba725b132d4a4 100644 --- a/regression-test/data/correctness/test_case_when.out +++ b/regression-test/data/correctness/test_case_when.out @@ -4,8 +4,8 @@ 3 00 1 -- !select_agg -- -1 90020004 -2 45010002 -3 90020004 -4 90020004 +1 90020004 +2 45010002 +3 90020004 +4 90020004