[2/3] impala git commit: IMPALA-6114: Require type equality for NumericLiteral::localEquals().

[ta...@apache.org]

IMPALA-6114: Require type equality for NumericLiteral::localEquals().

This patch fixes a regression introduced as part of IMPALA-1788, where
an expression like 'CAST(0 AS DECIMAL(14))' is rewritten as a
NumericLiteral expression of type DECIMAL(14,0). The query had
another NumericLiteral of type TINYINT. While analyzing the DISTINCT
aggregation clause of the SELECT query, AggregateInfo::create() removes
duplicate expressions from groupingExprs.

NumericLiteral::localEquals() is used to check for equality. Now
since the method does not consider expression types, a TINYINT literal
is considered to be duplicate of a DECIMAL literal. This results in a
query like the following to fail:

SELECT DISTINCT CAST(0 AS DECIMAL(14), 0 FROM functional.alltypes

We propose to fix the issue by accounting for types as well
when comparing analyzed numeric literals.

A test case has been added to AnalyzeStmtsTest.

Change-Id: Ia88d54088dfd128b103759dc01103b6c35bf6257
Reviewed-on: http://gerrit.cloudera.org:8080/8448
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/f4eb0012
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/f4eb0012
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/f4eb0012

Branch: refs/heads/master
Commit: f4eb00123ff6be770bf0fb75f71b6468cb5e7085
Parents: de29925
Author: Zoram Thanga <zo...@cloudera.com>
Authored: Wed Nov 1 14:43:47 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Dec 15 01:28:13 2017 +0000

----------------------------------------------------------------------
 .../main/java/org/apache/impala/analysis/NumericLiteral.java | 8 +++++++-
 .../java/org/apache/impala/analysis/AnalyzeStmtsTest.java    | 4 ++++
 2 files changed, 11 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/f4eb0012/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java b/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
index 80d2347..b720778 100644
--- a/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
+++ b/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
@@ -132,7 +132,13 @@ public class NumericLiteral extends LiteralExpr {
 
   @Override
   public boolean localEquals(Expr that) {
-    return super.localEquals(that) && ((NumericLiteral) that).value_.equals(value_);
+    if (!super.localEquals(that)) return false;
+
+    NumericLiteral tmp = (NumericLiteral) that;
+    if (!tmp.value_.equals(value_)) return false;
+    // Analyzed Numeric literals of different types are distinct.
+    if ((isAnalyzed() && tmp.isAnalyzed()) && (!getType().equals(tmp.getType()))) return false;
+    return true;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/impala/blob/f4eb0012/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
index 133b6e2..f9d5cf5 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
@@ -2052,6 +2052,10 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
     AnalyzesOk("select tinyint_col, count(distinct int_col),"
         + "min(distinct smallint_col), max(distinct string_col) "
         + "from functional.alltypesagg group by 1");
+
+    // IMPALA-6114: Test that numeric literals having the same value, but different types are
+    // considered distinct.
+    AnalyzesOk("select distinct cast(0 as decimal(14)), 0 from functional.alltypes");
   }
 
   @Test