From 6ab858bb10ea0ce0fee178323c35ee277ebde236 Mon Sep 17 00:00:00 2001
From: xy720 <22125576+xy720@users.noreply.github.com>
Date: Sat, 10 Oct 2020 21:16:00 +0800
Subject: [PATCH] [Bug] Fix duplicate columns in case when statement (#4693)

Fix #4692
The reason of this bug is case-when statement may produce implicit CastExpr<SlotRef>
as SelectListItem in analyze step.
And these CastExpr<SlotRef> in SelectList will not be re-anlyze after rewrite step,
which will result in the incorrect number of self-incrementing SlotDescriptor ID in resultExprs .

So we need to reset the analysis state of CastExpr<SlotRef> in rewrite step.
---
 .../java/org/apache/doris/analysis/CaseExpr.java   | 12 ++++++++++++
 .../java/org/apache/doris/analysis/CastExpr.java   |  4 +++-
 .../java/org/apache/doris/analysis/SlotRef.java    |  1 +
 .../org/apache/doris/planner/QueryPlanTest.java    | 14 ++++++++++++--
 4 files changed, 28 insertions(+), 3 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 21e5c115a6..5bc742f886 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
@@ -281,6 +281,18 @@ public class CaseExpr extends Expr {
         LiteralExpr caseExpr;
         int startIndex = 0;
         int endIndex = expr.getChildren().size();
+
+        // CastExpr contains SlotRef child should be reset to re-analyze in selectListItem
+        for (Expr child : expr.getChildren()) {
+            if (child instanceof CastExpr && (child.contains(SlotRef.class))) {
+                List<CastExpr> castExprList = Lists.newArrayList();
+                child.collect(CastExpr.class, castExprList);
+                for (CastExpr castExpr : castExprList) {
+                    castExpr.resetAnalysisState();
+                }
+            }
+        }
+
         if (expr.hasCaseExpr()) {
             // just deal literal here
             // and avoid `float compute` in java,float should be dealt in be
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
index 83480a9599..0257e6b6b2 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
@@ -207,7 +207,9 @@ public class CastExpr extends Expr {
 
     @Override
     public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
-        Preconditions.checkState(!isImplicit);
+        if (isImplicit) {
+            return;
+        }
         // When cast target type is string and it's length is default -1, the result length
         // of cast is decided by child.
         if (targetTypeDef.getType().isScalarType()) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
index 16793f259f..ad0606d8dc 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
@@ -93,6 +93,7 @@ public class SlotRef extends Expr {
     }
 
     public SlotDescriptor getDesc() {
+        Preconditions.checkState(isAnalyzed);
         Preconditions.checkNotNull(desc);
         return desc;
     }
diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
index bc702bd20a..970a82e8e4 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
@@ -917,10 +917,20 @@ public class QueryPlanTest {
         Assert.assertTrue(StringUtils.containsIgnoreCase(UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "explain " + sql52),
                 "OUTPUT EXPRS: `k7`"));
 
-        // 5.3 test different in then expr and else expr, and return CastExpr<SlotRef>
+        // 5.3 test different type in then expr and else expr, and return CastExpr<SlotRef>
         String sql53 = "select case when 2 < 1 then 'all' else k1 end as col53 from test.baseall group by col53";
         Assert.assertTrue(StringUtils.containsIgnoreCase(UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "explain " + sql53),
-                "OUTPUT EXPRS:<slot 0> `k1`"));
+                "OUTPUT EXPRS: `k1`"));
+
+        // 5.4 test return CastExpr<SlotRef> with other SlotRef in selectListItem
+        String sql54 = "select k2, case when 2 < 1 then 'all' else k1 end as col54, k7 from test.baseall group by k2, col54, k7";
+        Assert.assertTrue(StringUtils.containsIgnoreCase(UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "explain " + sql54),
+                "OUTPUT EXPRS:<slot 3> `k2` | <slot 4> `k1` | <slot 5> `k7`"));
+
+        // 5.5 test return CastExpr<CastExpr<SlotRef>> with other SlotRef in selectListItem
+        String sql55 = "select case when 2 < 1 then 'all' else cast(k1 as int) end as col55, k7 from test.baseall group by col55, k7";
+        Assert.assertTrue(StringUtils.containsIgnoreCase(UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "explain " + sql55),
+                "OUTPUT EXPRS:<slot 2> CAST(`k1` AS INT) | <slot 3> `k7`"));
     }
 
     @Test