IMPALA-3155: Disable implicit casting of CHAR to STRING in CASE statements All CHAR children of a CASE expression were automaticallally implicitly cast to STRING. This commit removes this behavior for the THEN clause of a CASE statement, i.e., if all THEN exprs of a CASE expression are CHAR, then the type of the CASE expression will be CHAR.
Change-Id: I4aebac6849898693570bc3164fff40786c215358 Reviewed-on: http://gerrit.cloudera.org:8080/2762 Reviewed-by: Taras Bobrovytsky <[email protected]> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/2bb98a08 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/2bb98a08 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/2bb98a08 Branch: refs/heads/master Commit: 2bb98a08c03b9ac47246c1957cce58a2614cccfd Parents: a0d4249 Author: Taras Bobrovytsky <[email protected]> Authored: Mon Apr 11 17:43:38 2016 -0700 Committer: Tim Armstrong <[email protected]> Committed: Thu May 12 14:17:57 2016 -0700 ---------------------------------------------------------------------- be/src/exprs/expr-test.cc | 5 +++-- .../java/com/cloudera/impala/analysis/CaseExpr.java | 13 ++++++++++++- .../main/java/com/cloudera/impala/analysis/Expr.java | 12 ------------ .../com/cloudera/impala/analysis/AnalyzeExprsTest.java | 9 +++++++++ 4 files changed, 24 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2bb98a08/be/src/exprs/expr-test.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc index 82b0487..349946a 100644 --- a/be/src/exprs/expr-test.cc +++ b/be/src/exprs/expr-test.cc @@ -2083,9 +2083,10 @@ TEST_F(ExprTest, StringFunctions) { " " " ", ColumnType::CreateCharType(255)); - TestStringValue("CASE cast('1.1' as char(3)) when cast('1.1' as char(3)) then " + TestCharValue("CASE cast('1.1' as char(3)) when cast('1.1' as char(3)) then " "cast('1' as char(1)) when cast('2.22' as char(4)) then " - "cast('2' as char(1)) else cast('3' as char(1)) end", "1"); + "cast('2' as char(1)) else cast('3' as char(1)) end", "1", + ColumnType::CreateCharType(3)); // Test maximum VARCHAR value char query[ColumnType::MAX_VARCHAR_LENGTH + 1024]; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2bb98a08/fe/src/main/java/com/cloudera/impala/analysis/CaseExpr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/com/cloudera/impala/analysis/CaseExpr.java b/fe/src/main/java/com/cloudera/impala/analysis/CaseExpr.java index 113547c..5ae85d4 100644 --- a/fe/src/main/java/com/cloudera/impala/analysis/CaseExpr.java +++ b/fe/src/main/java/com/cloudera/impala/analysis/CaseExpr.java @@ -212,11 +212,16 @@ public class CaseExpr extends Expr { msg.case_expr = new TCaseExpr(hasCaseExpr_, hasElseExpr_); } + private void castCharToString(int childIndex) throws AnalysisException { + if (children_.get(childIndex).getType().isScalarType(PrimitiveType.CHAR)) { + children_.set(childIndex, children_.get(childIndex).castTo(ScalarType.STRING)); + } + } + @Override public void analyze(Analyzer analyzer) throws AnalysisException { if (isAnalyzed_) return; super.analyze(analyzer); - castChildCharsToStrings(analyzer); if (isDecode()) { Preconditions.checkState(!hasCaseExpr_); @@ -229,6 +234,10 @@ public class CaseExpr extends Expr { } } + // Since we have no BE implementation of a CaseExpr with CHAR types, + // we cast the CHAR-typed whenExprs and caseExprs to STRING, + // TODO: This casting is not always correct and needs to be fixed, see IMPALA-1652. + // Keep track of maximum compatible type of case expr and all when exprs. Type whenType = null; // Keep track of maximum compatible type of else expr and all then exprs. @@ -245,6 +254,7 @@ public class CaseExpr extends Expr { // Set loop start, and initialize returnType as type of castExpr. if (hasCaseExpr_) { loopStart = 1; + castCharToString(0); caseExpr = children_.get(0); caseExpr.analyze(analyzer); whenType = caseExpr.getType(); @@ -256,6 +266,7 @@ public class CaseExpr extends Expr { // Go through when/then exprs and determine compatible types. for (int i = loopStart; i < loopEnd; i += 2) { + castCharToString(i); Expr whenExpr = children_.get(i); if (hasCaseExpr_) { // Determine maximum compatible type of the case expr, http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2bb98a08/fe/src/main/java/com/cloudera/impala/analysis/Expr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/com/cloudera/impala/analysis/Expr.java b/fe/src/main/java/com/cloudera/impala/analysis/Expr.java index 489f188..b931e85 100644 --- a/fe/src/main/java/com/cloudera/impala/analysis/Expr.java +++ b/fe/src/main/java/com/cloudera/impala/analysis/Expr.java @@ -321,18 +321,6 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl } /** - * Casts any child which is CHAR to a STRING. - */ - public void castChildCharsToStrings(Analyzer analyzer) throws AnalysisException { - for (int i = 0; i < children_.size(); ++i) { - if (children_.get(i).getType().isScalarType(PrimitiveType.CHAR)) { - children_.set(i, children_.get(i).castTo(ScalarType.STRING)); - children_.get(i).analyze(analyzer); - } - } - } - - /** * Looks up in the catalog the builtin for 'name' and 'argTypes'. * Returns null if the function is not found. */ http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2bb98a08/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java index 249e255..eb56c02 100644 --- a/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java +++ b/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java @@ -1697,6 +1697,15 @@ public class AnalyzeExprsTest extends AnalyzerTest { AnalyzesOk("select case 1 when 2 then NULL else 3 end"); AnalyzesOk("select case 1 when 2 then 3 else NULL end"); AnalyzesOk("select case NULL when NULL then NULL else NULL end"); + + // IMPALA-3155: Verify that a CHAR type is returned when all THEN + // exprs are of type CHAR. + checkReturnType("select case when 5 < 3 then cast('L' as char(1)) else " + + "cast('M' as char(1)) end", ScalarType.createCharType(1)); + // IMPALA-3155: Verify that a STRING type is returned when at least one THEN + // expr is of type STRING. + checkReturnType("select case when 5 < 3 then cast('L' as string) else " + + "cast('M' as char(1)) end", ScalarType.STRING); } @Test
