IMPALA-1559: FIRST_VALUE rewrite fn type might not match slot type

In some cases, the lookup for the first_value_rewrite function could
return a different fn with the wrong (yet technically compatible) type.

first_value_rewrite takes 2 parameters, the first is the parameter to
first_value, and the second is an integer explicitly added by the FE during
analysis. first_value_rewrite has signatures where the first parameter can
be of any type and the second is always a BIGINT. When the FE inserts the
second parameter, it may not actually be a BIGINT (e.g. it could be a
TINYINT, SMALLINT, etc.). In this case, the function arguments will not
match one of the registered signatures exactly and some compatable function
will be returned. For example, FIRST_VALUE(1.1) has a DECIMAL parameter
and if a TINYINT/SMALLINT/INT parameter is added for the rewrite, then
the first_value_rewrite fn lookup happened to match the fn taking a
FLOAT and BIGINT. (Ideally DECIMAL would not be implicitly castable to
a FLOAT/DOUBLE, but NumericLiterals do allow this casting.)

As a result, the agg fn actually returned a FLOAT while the AnalyticExpr
was of the type DECIMAL, and the analytic tuple contained a DECIMAL slot
which would throw a DCHECK in the BE (or perhaps crash in retail builds).

This fixes the issue by setting the NumericLiterals to be explicitlyCast,
i.e. the type will not change if re-analyzed. Then the correct fn signature
is found.

Change-Id: I1cefa3e29734ae647bd690263bb63f08f10ea8b9
Reviewed-on: http://gerrit.cloudera.org:8080/136
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins

fe/src/main/java/com/cloudera/impala/analysis/AnalyticExpr.java

@@ -474,13 +474,15 @@ public class AnalyticExpr extends Expr {
         if (window_.getRightBoundary().getType() == BoundaryType.PRECEDING) {
           // The number of rows preceding for the end bound determines the number of
           // rows at the beginning of each partition that should have a NULL value.
-          paramExprs.add(window_.getRightBoundary().getExpr());
+          paramExprs.add(new NumericLiteral(window_.getRightBoundary().getOffsetValue(),
+              Type.BIGINT));
         } else {
           // -1 indicates that no NULL values are inserted even though we set the end
           // bound to the start bound (which is PRECEDING) below; this is different from
           // the default behavior of windows with an end bound PRECEDING.
           paramExprs.add(new NumericLiteral(BigInteger.valueOf(-1), Type.BIGINT));
         }
+
         window_ = new AnalyticWindow(window_.getType(),
             new Boundary(BoundaryType.UNBOUNDED_PRECEDING, null),
             window_.getLeftBoundary());
@@ -490,6 +492,7 @@ public class AnalyticExpr extends Expr {
       }
       fnCall_.setIsAnalyticFnCall(true);
       fnCall_.analyzeNoThrow(analyzer);
+      type_ = fnCall_.getReturnType();
       analyticFnName = getFnCall().getFnName();
     }
 

fe/src/main/java/com/cloudera/impala/analysis/AnalyticWindow.java

@@ -121,6 +121,7 @@ public class AnalyticWindow {
 
     public BoundaryType getType() { return type_; }
     public Expr getExpr() { return expr_; }
+    public BigDecimal getOffsetValue() { return offsetValue_; }
 
     public Boundary(BoundaryType type, Expr e) {
       this(type, e, null);

fe/src/main/java/com/cloudera/impala/analysis/NumericLiteral.java

@@ -78,18 +78,21 @@ public class NumericLiteral extends LiteralExpr {
 
   /**
    * The versions of the ctor that take types assume the type is correct
-   * and the NumericLiteral is created as analyzed with that type.
+   * and the NumericLiteral is created as analyzed with that type. The specified
+   * type is preserved across substitutions and re-analysis.
    */
   public NumericLiteral(BigInteger value, Type type) {
     isAnalyzed_ = true;
     value_ = new BigDecimal(value);
     type_ = type;
+    explicitlyCast_ = true;
   }
 
   public NumericLiteral(BigDecimal value, Type type) {
     isAnalyzed_ = true;
     value_ = value;
     type_ = type;
+    explicitlyCast_ = true;
   }
 
   /**

testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test

@@ -1079,3 +1079,25 @@ select max(t3.c1) from
 ---- TYPES
 STRING
 ====
+---- QUERY
+# IMPALA-1559: FIRST_VALUE rewrite function intermediate type not matching slot type
+select
+first_value(c3) over (order by c3 rows between 92 preceding and current row),
+first_value(c2) over (order by c3 rows between 92 preceding and 1 preceding),
+first_value(-32.9) over (order by c3 rows between 92 preceding and unbounded following),
+first_value(1.1) over (order by c3 rows between 92 preceding and 1 preceding)
+from decimal_tiny where c3 = 0.0
+---- RESULTS
+0.0,NULL,-32.9,NULL
+0.0,100.00000,-32.9,1.1
+0.0,100.00000,-32.9,1.1
+0.0,100.00000,-32.9,1.1
+0.0,100.00000,-32.9,1.1
+0.0,100.00000,-32.9,1.1
+0.0,100.00000,-32.9,1.1
+0.0,100.00000,-32.9,1.1
+0.0,100.00000,-32.9,1.1
+0.0,100.00000,-32.9,1.1
+---- TYPES
+DECIMAL, DECIMAL, DECIMAL, DECIMAL
+====