Repository: incubator-impala Updated Branches: refs/heads/master 502220c69 -> c6fc89913
IMPALA-4301: Fix IGNORE NULLS with subquery rewriting. AnayticExpr.analyze() replaces the original FIRST/LAST_VALUE function with a FIRST/LAST_VALUE_IGNORE_NULLS function if the IGNORE NULLS clause is specified. The bug was that several places in AnalyticExpr.analyze() assumed and asserted that only the original FIRST/LAST_VALUE function could be encountered during analysis. However, with subquery rewriting the IGNORE NULLS version of the function may also be seen because the whole statement is re-analyzed after rewriting. The fix is to unset the IGNORE NULLS flag of the function params after changing the analytic function name. Change-Id: I708de7925fe6aeef582fd7510da93d24c71229d9 Reviewed-on: http://gerrit.cloudera.org:8080/4732 Reviewed-by: Alex Behm <[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/c6fc8991 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c6fc8991 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c6fc8991 Branch: refs/heads/master Commit: c6fc89913453befbf03d206d20c35323288702f4 Parents: 502220c Author: Alex Behm <[email protected]> Authored: Sat Oct 15 21:35:56 2016 -0700 Committer: Internal Jenkins <[email protected]> Committed: Thu Oct 20 09:14:19 2016 +0000 ---------------------------------------------------------------------- .../java/org/apache/impala/analysis/AnalyticExpr.java | 10 ++++++---- .../java/org/apache/impala/analysis/FunctionParams.java | 3 ++- .../org/apache/impala/analysis/AnalyzeExprsTest.java | 11 ++++++++--- 3 files changed, 16 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c6fc8991/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java b/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java index 7eead02..40fee99 100644 --- a/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java @@ -22,9 +22,6 @@ import java.math.BigInteger; import java.util.ArrayList; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.analysis.AnalyticWindow.Boundary; import org.apache.impala.analysis.AnalyticWindow.BoundaryType; import org.apache.impala.catalog.AggregateFunction; @@ -38,6 +35,9 @@ import org.apache.impala.service.FeSupport; import org.apache.impala.thrift.TColumnValue; import org.apache.impala.thrift.TExprNode; import org.apache.impala.util.TColumnValueUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Joiner; import com.google.common.base.Objects; import com.google.common.base.Preconditions; @@ -750,7 +750,8 @@ public class AnalyticExpr extends Expr { window_.getRightBoundary()); } - // 8. Append IGNORE NULLS to fn name if set. + // 8. Change fn name to the IGNORE NULLS version. Also unset the IGNORE NULLS flag + // to allow statement rewriting for subqueries. if (getFnCall().getParams().isIgnoreNulls()) { if (analyticFnName.getFunction().equals(LAST_VALUE)) { fnCall_ = new FunctionCallExpr(new FunctionName(LAST_VALUE_IGNORE_NULLS), @@ -760,6 +761,7 @@ public class AnalyticExpr extends Expr { fnCall_ = new FunctionCallExpr(new FunctionName(FIRST_VALUE_IGNORE_NULLS), getFnCall().getParams()); } + getFnCall().getParams().setIsIgnoreNulls(false); fnCall_.setIsAnalyticFnCall(true); fnCall_.setIsInternalFnCall(true); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c6fc8991/fe/src/main/java/org/apache/impala/analysis/FunctionParams.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/FunctionParams.java b/fe/src/main/java/org/apache/impala/analysis/FunctionParams.java index 4a1effb..c0a78c7 100644 --- a/fe/src/main/java/org/apache/impala/analysis/FunctionParams.java +++ b/fe/src/main/java/org/apache/impala/analysis/FunctionParams.java @@ -53,9 +53,10 @@ class FunctionParams implements Cloneable { public boolean isStar() { return isStar_; } public boolean isDistinct() { return isDistinct_; } + public void setIsDistinct(boolean v) { isDistinct_ = v; } public boolean isIgnoreNulls() { return isIgnoreNulls_; } + public void setIsIgnoreNulls(boolean b) { isIgnoreNulls_ = b; } public List<Expr> exprs() { return exprs_; } - public void setIsDistinct(boolean v) { isDistinct_ = v; } public int size() { return exprs_ == null ? 0 : exprs_.size(); } // c'tor for <agg>(*) http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c6fc8991/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java index 72b5163..2b77f39 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java @@ -26,9 +26,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import org.junit.Assert; -import org.junit.Test; - import org.apache.impala.analysis.TimestampArithmeticExpr.TimeUnit; import org.apache.impala.authorization.Privilege; import org.apache.impala.catalog.Catalog; @@ -47,6 +44,9 @@ import org.apache.impala.common.AnalysisException; import org.apache.impala.thrift.TExpr; import org.apache.impala.thrift.TFunctionBinaryType; import org.apache.impala.thrift.TQueryOptions; +import org.junit.Assert; +import org.junit.Test; + import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -729,6 +729,11 @@ public class AnalyzeExprsTest extends AnalyzerTest { + "functional.alltypesagg"); AnalyzesOk("select last_value(tinyint_col ignore nulls) over (order by id) from " + "functional.alltypesagg"); + // IMPALA-4301: Test IGNORE NULLS with subqueries. + AnalyzesOk("select first_value(tinyint_col ignore nulls) over (order by id)," + + "last_value(tinyint_col ignore nulls) over (order by id)" + + "from functional.alltypesagg a " + + "where exists (select 1 from functional.alltypes b where a.id = b.id)"); // legal combinations of analytic and agg functions AnalyzesOk("select sum(count(id)) over (partition by min(int_col) "
