Skip to content

Commit

Permalink
[fix](planner) unnecessary cast will be added on children in CaseExpr…
Browse files Browse the repository at this point in the history
… sometimes (apache#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`
  • Loading branch information
morrySnow authored and minghong.zhou committed May 23, 2022
1 parent 6583f21 commit f83d110
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<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);
}
}
6 changes: 6 additions & 0 deletions regression-test/data/correctness/test_case_when.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,9 @@
2 01 -1
3 00 1

-- !select_agg --
1 90020004
2 45010002
3 90020004
4 90020004

14 changes: 13 additions & 1 deletion regression-test/suites/correctness/test_case_when.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,16 @@ suite("test_case_when") {
and channel_id = '00'
group by hour_time, station_type;
"""
}

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;
"""
}

0 comments on commit f83d110

Please sign in to comment.