hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jcama...@apache.org
Subject hive git commit: HIVE-11627: Reduce the number of accesses to hashmaps in PPD (Jesus Camacho Rodriguez, reviewed by Ashutosh Chauhan)
Date Fri, 28 Aug 2015 08:14:35 GMT
Repository: hive
Updated Branches:
  refs/heads/branch-1 c1528bfb2 -> 39db96382


HIVE-11627: Reduce the number of accesses to hashmaps in PPD (Jesus Camacho Rodriguez, reviewed
by Ashutosh Chauhan)


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

Branch: refs/heads/branch-1
Commit: 39db963828489a40dc1b47af56c20bed771e6bb3
Parents: c1528bf
Author: Jesus Camacho Rodriguez <jcamacho@apache.org>
Authored: Fri Aug 28 10:14:09 2015 +0200
Committer: Jesus Camacho Rodriguez <jcamacho@apache.org>
Committed: Fri Aug 28 10:14:09 2015 +0200

----------------------------------------------------------------------
 .../hadoop/hive/ql/ppd/ExprWalkerInfo.java      | 127 ++++---------------
 .../hive/ql/ppd/ExprWalkerProcFactory.java      |  90 ++++++++-----
 .../hadoop/hive/ql/ppd/OpProcFactory.java       |  11 +-
 3 files changed, 93 insertions(+), 135 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/39db9638/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerInfo.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerInfo.java b/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerInfo.java
index f23facf..e4b768e 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerInfo.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerInfo.java
@@ -38,29 +38,21 @@ import org.apache.hadoop.hive.ql.plan.OperatorDesc;
 public class ExprWalkerInfo implements NodeProcessorCtx {
 
   /** Information maintained for an expr while walking an expr tree. */
-  private static class ExprInfo {
+  protected class ExprInfo {
     /**
      * true if expr rooted at this node doesn't contain more than one table.
      * alias
      */
-    public boolean isCandidate = false;
+    protected boolean isCandidate = false;
     /** alias that this expression refers to. */
-    public String alias = null;
+    protected String alias = null;
     /** new expr for this expression. */
-    public ExprNodeDesc convertedExpr = null;
+    protected ExprNodeDesc convertedExpr = null;
 
-    public ExprInfo() {
-    }
 
-    public ExprInfo(boolean isCandidate, String alias, ExprNodeDesc replacedNode) {
-      this.isCandidate = isCandidate;
-      this.alias = alias;
-      convertedExpr = replacedNode;
-    }
   }
 
-  protected static final Log LOG = LogFactory.getLog(OpProcFactory.class
-      .getName());;
+  protected static final Log LOG = LogFactory.getLog(OpProcFactory.class.getName());
   private Operator<? extends OperatorDesc> op = null;
 
   /**
@@ -127,105 +119,33 @@ public class ExprWalkerInfo implements NodeProcessorCtx {
   }
 
   /**
-   * @return converted expression for give node. If there is none then returns
-   *         null.
-   */
-  public ExprNodeDesc getConvertedNode(ExprNodeDesc nd) {
-    ExprInfo ei = exprInfoMap.get(nd);
-    if (ei == null) {
-      return null;
-    }
-    return ei.convertedExpr;
-  }
-
-  /**
-   * adds a replacement node for this expression.
-   *
-   * @param oldNode
-   *          original node
-   * @param newNode
-   *          new node
+   * Get additional info for a given expression node
    */
-  public void addConvertedNode(ExprNodeDesc oldNode, ExprNodeDesc newNode) {
-    ExprInfo ei = exprInfoMap.get(oldNode);
-    if (ei == null) {
-      ei = new ExprInfo();
-      exprInfoMap.put(oldNode, ei);
-    }
-    ei.convertedExpr = newNode;
-    exprInfoMap.put(newNode, new ExprInfo(ei.isCandidate, ei.alias, null));
+  public ExprInfo getExprInfo(ExprNodeDesc expr) {
+    return exprInfoMap.get(expr);
   }
 
   /**
-   * Returns true if the specified expression is pushdown candidate else false.
-   *
-   * @param expr
-   * @return true or false
+   * Get additional info for a given expression node if it
+   * exists, or create a new one and store it if it does not
    */
-  public boolean isCandidate(ExprNodeDesc expr) {
-    ExprInfo ei = exprInfoMap.get(expr);
-    if (ei == null) {
-      return false;
-    }
-    return ei.isCandidate;
+  public ExprInfo addExprInfo(ExprNodeDesc expr) {
+    ExprInfo exprInfo = new ExprInfo();
+    exprInfoMap.put(expr, exprInfo);
+    return exprInfo;
   }
 
   /**
-   * Marks the specified expr to the specified value.
-   *
-   * @param expr
-   * @param b
-   *          can
+   * Get additional info for a given expression node if it
+   * exists, or create a new one and store it if it does not
    */
-  public void setIsCandidate(ExprNodeDesc expr, boolean b) {
-    ExprInfo ei = exprInfoMap.get(expr);
-    if (ei == null) {
-      ei = new ExprInfo();
-      exprInfoMap.put(expr, ei);
+  public ExprInfo addOrGetExprInfo(ExprNodeDesc expr) {
+    ExprInfo exprInfo = exprInfoMap.get(expr);
+    if (exprInfo == null) {
+      exprInfo = new ExprInfo();
+      exprInfoMap.put(expr, exprInfo);
     }
-    ei.isCandidate = b;
-  }
-
-  /**
-   * Returns the alias of the specified expr.
-   *
-   * @param expr
-   * @return The alias of the expression
-   */
-  public String getAlias(ExprNodeDesc expr) {
-    ExprInfo ei = exprInfoMap.get(expr);
-    if (ei == null) {
-      return null;
-    }
-    return ei.alias;
-  }
-
-  /**
-   * Adds the specified alias to the specified expr.
-   *
-   * @param expr
-   * @param alias
-   */
-  public void addAlias(ExprNodeDesc expr, String alias) {
-    if (alias == null) {
-      return;
-    }
-    ExprInfo ei = exprInfoMap.get(expr);
-    if (ei == null) {
-      ei = new ExprInfo();
-      exprInfoMap.put(expr, ei);
-    }
-    ei.alias = alias;
-  }
-
-  /**
-   * Adds the specified expr as the top-most pushdown expr (ie all its children
-   * can be pushed).
-   *
-   * @param expr
-   */
-  public void addFinalCandidate(ExprNodeDesc expr) {
-    addFinalCandidate(getAlias(expr), expr);
+    return exprInfo;
   }
 
   public void addFinalCandidate(String alias, ExprNodeDesc expr) {
@@ -278,8 +198,7 @@ public class ExprWalkerInfo implements NodeProcessorCtx {
    *
    * @param expr
    */
-  public void addNonFinalCandidate(ExprNodeDesc expr) {
-    String alias = getAlias(expr);
+  public void addNonFinalCandidate(String alias, ExprNodeDesc expr) {
     if (nonFinalPreds.get(alias) == null) {
       nonFinalPreds.put(alias, new ArrayList<ExprNodeDesc>());
     }

http://git-wip-us.apache.org/repos/asf/hive/blob/39db9638/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java b/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java
index 3a07b17..4df33cb 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java
@@ -45,6 +45,7 @@ import org.apache.hadoop.hive.ql.plan.ExprNodeDesc;
 import org.apache.hadoop.hive.ql.plan.ExprNodeFieldDesc;
 import org.apache.hadoop.hive.ql.plan.ExprNodeGenericFuncDesc;
 import org.apache.hadoop.hive.ql.plan.OperatorDesc;
+import org.apache.hadoop.hive.ql.ppd.ExprWalkerInfo.ExprInfo;
 
 /**
  * Expression factory for predicate pushdown processing. Each processor
@@ -53,8 +54,7 @@ import org.apache.hadoop.hive.ql.plan.OperatorDesc;
  */
 public final class ExprWalkerProcFactory {
 
-  private static final Log LOG = LogFactory
-      .getLog(ExprWalkerProcFactory.class.getName());
+  private static final Log LOG = LogFactory.getLog(ExprWalkerProcFactory.class.getName());
 
   /**
    * ColumnExprProcessor.
@@ -78,6 +78,7 @@ public final class ExprWalkerProcFactory {
         tabAlias = ci.getTabAlias();
       }
 
+      ExprInfo colExprInfo = null;
       boolean isCandidate = true;
       if (op.getColumnExprMap() != null) {
         // replace the output expression with the input expression so that
@@ -86,7 +87,8 @@ public final class ExprWalkerProcFactory {
         if (exp == null) {
           // means that expression can't be pushed either because it is value in
           // group by
-          ctx.setIsCandidate(colref, false);
+          colExprInfo = ctx.addOrGetExprInfo(colref);
+          colExprInfo.isCandidate = false;
           return false;
         } else {
           if (exp instanceof ExprNodeGenericFuncDesc) {
@@ -97,16 +99,25 @@ public final class ExprWalkerProcFactory {
             tabAlias = column.getTabAlias();
           }
         }
-        ctx.addConvertedNode(colref, exp);
-        ctx.setIsCandidate(exp, isCandidate);
-        ctx.addAlias(exp, tabAlias);
+        colExprInfo = ctx.addOrGetExprInfo(colref);
+        colExprInfo.convertedExpr = exp;
+        ExprInfo expInfo = ctx.addExprInfo(exp);
+        expInfo.isCandidate = isCandidate;
+        if (tabAlias != null) {
+          expInfo.alias = tabAlias;
+        } else {
+          expInfo.alias = colExprInfo.alias;
+        }
       } else {
         if (ci == null) {
           return false;
         }
-        ctx.addAlias(colref, tabAlias);
+        colExprInfo = ctx.addOrGetExprInfo(colref);
+        if (tabAlias != null) {
+          colExprInfo.alias = tabAlias;
+        }
       }
-      ctx.setIsCandidate(colref, isCandidate);
+      colExprInfo.isCandidate = isCandidate;
       return isCandidate;
     }
 
@@ -125,30 +136,37 @@ public final class ExprWalkerProcFactory {
       String alias = null;
       ExprNodeFieldDesc expr = (ExprNodeFieldDesc) nd;
 
-      boolean isCandidate = true;
       assert (nd.getChildren().size() == 1);
       ExprNodeDesc ch = (ExprNodeDesc) nd.getChildren().get(0);
-      ExprNodeDesc newCh = ctx.getConvertedNode(ch);
+      ExprInfo chExprInfo = ctx.getExprInfo(ch);
+      ExprNodeDesc newCh = chExprInfo != null ? chExprInfo.convertedExpr : null;
       if (newCh != null) {
         expr.setDesc(newCh);
         ch = newCh;
+        chExprInfo = ctx.getExprInfo(ch);
       }
-      String chAlias = ctx.getAlias(ch);
 
-      isCandidate = isCandidate && ctx.isCandidate(ch);
+      boolean isCandidate;
+      String chAlias;
+      if (chExprInfo != null) {
+        chAlias = chExprInfo.alias;
+        isCandidate = chExprInfo.isCandidate;
+      } else {
+        chAlias = null;
+        isCandidate = false;
+      }
       // need to iterate through all children even if one is found to be not a
       // candidate
       // in case if the other children could be individually pushed up
       if (isCandidate && chAlias != null) {
-        if (alias == null) {
-          alias = chAlias;
-        } else if (!chAlias.equalsIgnoreCase(alias)) {
-          isCandidate = false;
-        }
+        alias = chAlias;
       }
 
-      ctx.addAlias(expr, alias);
-      ctx.setIsCandidate(expr, isCandidate);
+      ExprInfo exprInfo = ctx.addOrGetExprInfo(expr);
+      if (alias != null) {
+        exprInfo.alias = alias;
+      }
+      exprInfo.isCandidate = isCandidate;
       return isCandidate;
     }
 
@@ -170,7 +188,8 @@ public final class ExprWalkerProcFactory {
 
       if (!FunctionRegistry.isDeterministic(expr.getGenericUDF())) {
         // this GenericUDF can't be pushed down
-        ctx.setIsCandidate(expr, false);
+        ExprInfo exprInfo = ctx.addOrGetExprInfo(expr);
+        exprInfo.isCandidate = false;
         ctx.setDeterministic(false);
         return false;
       }
@@ -178,14 +197,22 @@ public final class ExprWalkerProcFactory {
       boolean isCandidate = true;
       for (int i = 0; i < nd.getChildren().size(); i++) {
         ExprNodeDesc ch = (ExprNodeDesc) nd.getChildren().get(i);
-        ExprNodeDesc newCh = ctx.getConvertedNode(ch);
+        ExprInfo chExprInfo = ctx.getExprInfo(ch);
+        ExprNodeDesc newCh = chExprInfo != null ? chExprInfo.convertedExpr : null;
         if (newCh != null) {
           expr.getChildren().set(i, newCh);
           ch = newCh;
+          chExprInfo = ctx.getExprInfo(ch);
         }
-        String chAlias = ctx.getAlias(ch);
 
-        isCandidate = isCandidate && ctx.isCandidate(ch);
+        String chAlias;
+        if (chExprInfo != null) {
+          chAlias = chExprInfo.alias;
+          isCandidate = isCandidate && chExprInfo.isCandidate;
+        } else {
+          chAlias = null;
+          isCandidate = false;
+        }
         // need to iterate through all children even if one is found to be not a
         // candidate
         // in case if the other children could be individually pushed up
@@ -201,8 +228,11 @@ public final class ExprWalkerProcFactory {
           break;
         }
       }
-      ctx.addAlias(expr, alias);
-      ctx.setIsCandidate(expr, isCandidate);
+      ExprInfo exprInfo = ctx.addOrGetExprInfo(expr);
+      if (alias != null) {
+        exprInfo.alias = alias;
+      }
+      exprInfo.isCandidate = isCandidate;
       return isCandidate;
     }
 
@@ -217,7 +247,8 @@ public final class ExprWalkerProcFactory {
     public Object process(Node nd, Stack<Node> stack, NodeProcessorCtx procCtx,
         Object... nodeOutputs) throws SemanticException {
       ExprWalkerInfo ctx = (ExprWalkerInfo) procCtx;
-      ctx.setIsCandidate((ExprNodeDesc) nd, true);
+      ExprInfo exprInfo = ctx.addOrGetExprInfo((ExprNodeDesc) nd);
+      exprInfo.isCandidate = true;
       return true;
     }
   }
@@ -327,12 +358,13 @@ public final class ExprWalkerProcFactory {
       return;
     }
 
-    if (ctx.isCandidate(expr)) {
-      ctx.addFinalCandidate(expr);
+    ExprInfo exprInfo = ctx.getExprInfo(expr);
+    if (exprInfo != null && exprInfo.isCandidate) {
+      ctx.addFinalCandidate(exprInfo.alias, expr);
       return;
     } else if (!FunctionRegistry.isOpAnd(expr) &&
         HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVEPPDREMOVEDUPLICATEFILTERS)) {
-      ctx.addNonFinalCandidate(expr);
+      ctx.addNonFinalCandidate(exprInfo != null ? exprInfo.alias : null, expr);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/hive/blob/39db9638/ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java b/ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java
index 6f9df53..dbd021b 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java
@@ -66,6 +66,7 @@ import org.apache.hadoop.hive.ql.plan.ptf.ValueBoundaryDef;
 import org.apache.hadoop.hive.ql.plan.ptf.WindowFrameDef;
 import org.apache.hadoop.hive.ql.plan.ptf.WindowFunctionDef;
 import org.apache.hadoop.hive.ql.plan.ptf.WindowTableFunctionDef;
+import org.apache.hadoop.hive.ql.ppd.ExprWalkerInfo.ExprInfo;
 import org.apache.hadoop.hive.ql.udf.generic.GenericUDAFDenseRank.GenericUDAFDenseRankEvaluator;
 import org.apache.hadoop.hive.ql.udf.generic.GenericUDAFLead.GenericUDAFLeadEvaluator;
 import org.apache.hadoop.hive.ql.udf.generic.GenericUDAFRank.GenericUDAFRankEvaluator;
@@ -483,8 +484,14 @@ public final class OpProcFactory {
             prunePreds.getFinalCandidates().get(alias)) {
             // add expr to the list of predicates rejected from further pushing
             // so that we know to add it in createFilter()
-            prunePreds.addAlias(expr, alias);
-            prunePreds.addNonFinalCandidate(expr);
+            ExprInfo exprInfo;
+            if (alias != null) {
+              exprInfo = prunePreds.addOrGetExprInfo(expr);
+              exprInfo.alias = alias;
+            } else {
+              exprInfo = prunePreds.getExprInfo(expr);
+            }
+            prunePreds.addNonFinalCandidate(exprInfo != null ? exprInfo.alias : null, expr);
           }
           prunePreds.getFinalCandidates().remove(alias);
         }


Mime
View raw message