You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2021/04/13 02:21:05 UTC

[impala] branch master updated: IMPALA-10619: Minor refactoring of analytic function methods

This is an automated email from the ASF dual-hosted git repository.

stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new de764eb  IMPALA-10619: Minor refactoring of analytic function methods
de764eb is described below

commit de764ebecf9afbc123da0e47758a08ebbb8219d8
Author: Aman Sinha <am...@cloudera.com>
AuthorDate: Mon Mar 29 17:19:28 2021 -0700

    IMPALA-10619: Minor refactoring of analytic function methods
    
    The FIRST_VALUE, LAST_VALUE functions go through standardization
    process in AnalyticExpr where they may be rewritten with different
    number of parameters or with different window frame. In order for an
    external FE to leverage this standardization, this patch creates a
    wrapper method for FunctionCallExpr creation and does minor refactoring.
    
    Also added accessor methods to AnalyticEvalNode and changed visibility
    of couple of methods in PlanNode for use by external FE.
    
    Testing: Ran PlannerTests. No new tests are added since this does not
    change the existing behavior.
    
    Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2
    Reviewed-on: http://gerrit.cloudera.org:8080/17237
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Aman Sinha <am...@cloudera.com>
    Reviewed-by: Aman Sinha <am...@cloudera.com>
---
 .../java/org/apache/impala/analysis/AnalyticExpr.java   | 17 ++++++++++++-----
 .../org/apache/impala/planner/AnalyticEvalNode.java     |  4 ++++
 .../main/java/org/apache/impala/planner/PlanNode.java   |  4 ++--
 3 files changed, 18 insertions(+), 7 deletions(-)

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 09223fb..22a4520 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
@@ -657,7 +657,7 @@ public class AnalyticExpr extends Expr {
       if (window_.getLeftBoundary().getType() != BoundaryType.PRECEDING) {
         window_ = new AnalyticWindow(window_.getType(), window_.getLeftBoundary(),
             window_.getLeftBoundary());
-        fnCall_ = new FunctionCallExpr(new FunctionName(LAST_VALUE),
+        fnCall_ = createRewrittenFunction(new FunctionName(LAST_VALUE),
             getFnCall().getParams());
       } else {
         List<Expr> paramExprs = Expr.cloneList(getFnCall().getParams().exprs());
@@ -676,7 +676,7 @@ public class AnalyticExpr extends Expr {
         window_ = new AnalyticWindow(window_.getType(),
             new Boundary(BoundaryType.UNBOUNDED_PRECEDING, null),
             window_.getLeftBoundary());
-        fnCall_ = new FunctionCallExpr(new FunctionName(FIRST_VALUE_REWRITE),
+        fnCall_ = createRewrittenFunction(new FunctionName(FIRST_VALUE_REWRITE),
             new FunctionParams(paramExprs));
         fnCall_.setIsInternalFnCall(true);
       }
@@ -708,7 +708,7 @@ public class AnalyticExpr extends Expr {
         reversedFnName = new FunctionName(FIRST_VALUE);
       }
       if (reversedFnName != null) {
-        fnCall_ = new FunctionCallExpr(reversedFnName, getFnCall().getParams());
+        fnCall_ = createRewrittenFunction(reversedFnName, getFnCall().getParams());
         fnCall_.setIsAnalyticFnCall(true);
         fnCall_.analyzeNoThrow(analyzer);
       }
@@ -744,11 +744,11 @@ public class AnalyticExpr extends Expr {
     // 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),
+        fnCall_ = createRewrittenFunction(new FunctionName(LAST_VALUE_IGNORE_NULLS),
             getFnCall().getParams());
       } else {
         Preconditions.checkState(analyticFnName.getFunction().equals(FIRST_VALUE));
-        fnCall_ = new FunctionCallExpr(new FunctionName(FIRST_VALUE_IGNORE_NULLS),
+        fnCall_ = createRewrittenFunction(new FunctionName(FIRST_VALUE_IGNORE_NULLS),
             getFnCall().getParams());
       }
       getFnCall().getParams().setIsIgnoreNulls(false);
@@ -827,4 +827,11 @@ public class AnalyticExpr extends Expr {
     ((AnalyticExpr) e).syncWithChildren();
     return e;
   }
+
+  // A wrapper method to create a FunctionCallExpr for a rewritten analytic
+  // function. This can be overridden in a derived class.
+  protected FunctionCallExpr createRewrittenFunction(FunctionName funcName,
+    FunctionParams funcParams) {
+    return new FunctionCallExpr(funcName, funcParams);
+  }
 }
diff --git a/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java b/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
index 252df3b..b0dd698 100644
--- a/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
@@ -671,4 +671,8 @@ public class AnalyticEvalNode extends PlanNode {
     return new LimitPushdownInfo(includeTies, (int)analyticLimit);
   }
 
+  public TupleDescriptor getOutputTupleDesc() { return outputTupleDesc_; }
+  public List<Expr> getAnalyticFnCalls() { return analyticFnCalls_; }
+  public AnalyticWindow getAnalyticWindow() { return analyticWindow_; }
+
 }
diff --git a/fe/src/main/java/org/apache/impala/planner/PlanNode.java b/fe/src/main/java/org/apache/impala/planner/PlanNode.java
index fe2dd3e..7744aa6 100644
--- a/fe/src/main/java/org/apache/impala/planner/PlanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/PlanNode.java
@@ -609,7 +609,7 @@ abstract public class PlanNode extends TreeNode<PlanNode> {
     return cardinality;
   }
 
-  static long capCardinalityAtLimit(long cardinality, long limit) {
+  public static long capCardinalityAtLimit(long cardinality, long limit) {
     return cardinality == -1 ? limit : Math.min(cardinality, limit);
   }
 
@@ -747,7 +747,7 @@ abstract public class PlanNode extends TreeNode<PlanNode> {
   /**
    * Returns true if stats-related variables are valid.
    */
-  protected boolean hasValidStats() {
+  public boolean hasValidStats() {
     return (numNodes_ == -1 || numNodes_ >= 0) &&
            (numInstances_ == -1 || numInstances_ >= 0) &&
            (cardinality_ == -1 || cardinality_ >= 0);