HIVE-11328: Avoid String representation of expression nodes in ConstantPropagateProcFactory unless necessary (Jesus Camacho Rodriguez, reviewed by Ashutosh Chauhan/Hari Sankar Sivarama Subramaniyan)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/83be12fb Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/83be12fb Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/83be12fb Branch: refs/heads/hbase-metastore Commit: 83be12fbb4d7cc7ef19779e1d14cac85023c8197 Parents: 72f97fc Author: Jesus Camacho Rodriguez <[email protected]> Authored: Wed Jul 22 10:08:42 2015 +0100 Committer: Jesus Camacho Rodriguez <[email protected]> Committed: Wed Jul 22 10:08:42 2015 +0100 ---------------------------------------------------------------------- .../optimizer/ConstantPropagateProcFactory.java | 100 ++++++++++++++----- 1 file changed, 75 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/83be12fb/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java b/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java index 286c042..410735c 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java @@ -155,7 +155,9 @@ public final class ConstantPropagateProcFactory { // ExprNodeConstantDesc return null; } - LOG.debug("Casting " + desc + " to type " + ti); + if (LOG.isDebugEnabled()) { + LOG.debug("Casting " + desc + " to type " + ti); + } ExprNodeConstantDesc c = (ExprNodeConstantDesc) desc; if (null != c.getFoldedFromVal() && priti.getTypeName().equals(serdeConstants.STRING_TYPE_NAME)) { // avoid double casting to preserve original string representation of constant. @@ -243,7 +245,9 @@ public final class ConstantPropagateProcFactory { // Don't evalulate nondeterministic function since the value can only calculate during runtime. if (!isDeterministicUdf(udf)) { - LOG.debug("Function " + udf.getClass() + " is undeterministic. Don't evalulating immediately."); + if (LOG.isDebugEnabled()) { + LOG.debug("Function " + udf.getClass() + " is undeterministic. Don't evalulating immediately."); + } ((ExprNodeGenericFuncDesc) desc).setChildren(newExprs); return desc; } @@ -251,7 +255,9 @@ public final class ConstantPropagateProcFactory { // Check if the function can be short cut. ExprNodeDesc shortcut = shortcutFunction(udf, newExprs, op); if (shortcut != null) { - LOG.debug("Folding expression:" + desc + " -> " + shortcut); + if (LOG.isDebugEnabled()) { + LOG.debug("Folding expression:" + desc + " -> " + shortcut); + } return shortcut; } ((ExprNodeGenericFuncDesc) desc).setChildren(newExprs); @@ -293,20 +299,26 @@ public final class ConstantPropagateProcFactory { // Don't evalulate nondeterministic function since the value can only calculate during runtime. if (!isDeterministicUdf(udf)) { - LOG.debug("Function " + udf.getClass() + " is undeterministic. Don't evalulating immediately."); + if (LOG.isDebugEnabled()) { + LOG.debug("Function " + udf.getClass() + " is undeterministic. Don't evalulating immediately."); + } ((ExprNodeGenericFuncDesc) desc).setChildren(newExprs); return desc; } else { // If all child expressions of deterministic function are constants, evaluate such UDF immediately ExprNodeDesc constant = evaluateFunction(udf, newExprs, desc.getChildren()); if (constant != null) { - LOG.debug("Folding expression:" + desc + " -> " + constant); + if (LOG.isDebugEnabled()) { + LOG.debug("Folding expression:" + desc + " -> " + constant); + } return constant; } else { // Check if the function can be short cut. ExprNodeDesc shortcut = shortcutFunction(udf, newExprs, op); if (shortcut != null) { - LOG.debug("Folding expression:" + desc + " -> " + shortcut); + if (LOG.isDebugEnabled()) { + LOG.debug("Folding expression:" + desc + " -> " + shortcut); + } return shortcut; } ((ExprNodeGenericFuncDesc) desc).setChildren(newExprs); @@ -328,7 +340,9 @@ public final class ConstantPropagateProcFactory { Operator<? extends Serializable> parent = op.getParentOperators().get(tag); ExprNodeDesc col = evaluateColumn((ExprNodeColumnDesc) desc, cppCtx, parent); if (col != null) { - LOG.debug("Folding expression:" + desc + " -> " + col); + if (LOG.isDebugEnabled()) { + LOG.debug("Folding expression:" + desc + " -> " + col); + } return col; } } @@ -406,7 +420,9 @@ public final class ConstantPropagateProcFactory { } ColumnInfo ci = resolveColumn(rs, c); if (ci != null) { - LOG.debug("Filter " + udf + " is identified as a value assignment, propagate it."); + if (LOG.isDebugEnabled()) { + LOG.debug("Filter " + udf + " is identified as a value assignment, propagate it."); + } if (!v.getTypeInfo().equals(ci.getType())) { v = typeCast(v, ci.getType()); } @@ -417,7 +433,9 @@ public final class ConstantPropagateProcFactory { } else if (udf instanceof GenericUDFOPNull) { ExprNodeDesc operand = newExprs.get(0); if (operand instanceof ExprNodeColumnDesc) { - LOG.debug("Filter " + udf + " is identified as a value assignment, propagate it."); + if (LOG.isDebugEnabled()) { + LOG.debug("Filter " + udf + " is identified as a value assignment, propagate it."); + } ExprNodeColumnDesc c = (ExprNodeColumnDesc) operand; ColumnInfo ci = resolveColumn(rs, c); if (ci != null) { @@ -641,11 +659,15 @@ public final class ConstantPropagateProcFactory { RowSchema rs = parent.getSchema(); ColumnInfo ci = rs.getColumnInfo(desc.getColumn()); if (ci == null) { - LOG.error("Reverse look up of column " + desc + " error!"); + if (LOG.isErrorEnabled()) { + LOG.error("Reverse look up of column " + desc + " error!"); + } ci = rs.getColumnInfo(desc.getTabAlias(), desc.getColumn()); } if (ci == null) { - LOG.error("Can't resolve " + desc.getTabAlias() + "." + desc.getColumn()); + if (LOG.isErrorEnabled()) { + LOG.error("Can't resolve " + desc.getTabAlias() + "." + desc.getColumn()); + } return null; } ExprNodeDesc constant = null; @@ -723,7 +745,9 @@ public final class ConstantPropagateProcFactory { try { ObjectInspector oi = udf.initialize(argois); Object o = udf.evaluate(arguments); - LOG.debug(udf.getClass().getName() + "(" + exprs + ")=" + o); + if (LOG.isDebugEnabled()) { + LOG.debug(udf.getClass().getName() + "(" + exprs + ")=" + o); + } if (o == null) { return new ExprNodeConstantDesc(TypeInfoUtils.getTypeInfoFromObjectInspector(oi), o); } @@ -740,7 +764,9 @@ public final class ConstantPropagateProcFactory { } else if (PrimitiveObjectInspectorUtils.isPrimitiveJavaClass(clz)) { } else { - LOG.error("Unable to evaluate " + udf + ". Return value unrecoginizable."); + if (LOG.isErrorEnabled()) { + LOG.error("Unable to evaluate " + udf + ". Return value unrecoginizable."); + } return null; } String constStr = null; @@ -771,7 +797,9 @@ public final class ConstantPropagateProcFactory { for (ColumnInfo col : schema.getSignature()) { ExprNodeDesc constant = constants.get(col); if (constant != null) { - LOG.debug("Replacing column " + col + " with constant " + constant + " in " + op); + if (LOG.isDebugEnabled()) { + LOG.debug("Replacing column " + col + " with constant " + constant + " in " + op); + } if (!col.getType().equals(constant.getTypeInfo())) { constant = typeCast(constant, col.getType()); } @@ -807,22 +835,30 @@ public final class ConstantPropagateProcFactory { cppCtx.getOpToConstantExprs().put(op, constants); ExprNodeDesc condn = op.getConf().getPredicate(); - LOG.debug("Old filter FIL[" + op.getIdentifier() + "] conditions:" + condn.getExprString()); + if (LOG.isDebugEnabled()) { + LOG.debug("Old filter FIL[" + op.getIdentifier() + "] conditions:" + condn.getExprString()); + } ExprNodeDesc newCondn = foldExpr(condn, constants, cppCtx, op, 0, true); if (newCondn instanceof ExprNodeConstantDesc) { ExprNodeConstantDesc c = (ExprNodeConstantDesc) newCondn; if (Boolean.TRUE.equals(c.getValue())) { cppCtx.addOpToDelete(op); - LOG.debug("Filter expression " + condn + " holds true. Will delete it."); + if (LOG.isDebugEnabled()) { + LOG.debug("Filter expression " + condn + " holds true. Will delete it."); + } } else if (Boolean.FALSE.equals(c.getValue())) { - LOG.warn("Filter expression " + condn + " holds false!"); + if (LOG.isWarnEnabled()) { + LOG.warn("Filter expression " + condn + " holds false!"); + } } } if (newCondn instanceof ExprNodeConstantDesc && ((ExprNodeConstantDesc)newCondn).getValue() == null) { // where null is same as where false newCondn = new ExprNodeConstantDesc(Boolean.FALSE); } - LOG.debug("New filter FIL[" + op.getIdentifier() + "] conditions:" + newCondn.getExprString()); + if (LOG.isDebugEnabled()) { + LOG.debug("New filter FIL[" + op.getIdentifier() + "] conditions:" + newCondn.getExprString()); + } // merge it with the downstream col list op.getConf().setPredicate(newCondn); @@ -948,7 +984,9 @@ public final class ConstantPropagateProcFactory { columnExprMap.put(columnNames.get(i), newCol); } } - LOG.debug("New column list:(" + StringUtils.join(colList, " ") + ")"); + if (LOG.isDebugEnabled()) { + LOG.debug("New column list:(" + StringUtils.join(colList, " ") + ")"); + } } return null; } @@ -1026,7 +1064,9 @@ public final class ConstantPropagateProcFactory { Operator<?> op = (Operator<?>) nd; ConstantPropagateProcCtx cppCtx = (ConstantPropagateProcCtx) ctx; cppCtx.getOpToConstantExprs().put(op, new HashMap<ColumnInfo, ExprNodeDesc>()); - LOG.debug("Stop propagate constants on op " + op.getOperatorId()); + if (LOG.isDebugEnabled()) { + LOG.debug("Stop propagate constants on op " + op.getOperatorId()); + } return null; } } @@ -1058,7 +1098,9 @@ public final class ConstantPropagateProcFactory { && op.getChildOperators().get(0) instanceof JoinOperator) { JoinOperator joinOp = (JoinOperator) op.getChildOperators().get(0); if (skipFolding(joinOp.getConf())) { - LOG.debug("Skip folding in outer join " + op); + if (LOG.isDebugEnabled()) { + LOG.debug("Skip folding in outer join " + op); + } cppCtx.getOpToConstantExprs().put(op, new HashMap<ColumnInfo, ExprNodeDesc>()); return null; } @@ -1066,7 +1108,9 @@ public final class ConstantPropagateProcFactory { if (rsDesc.getDistinctColumnIndices() != null && !rsDesc.getDistinctColumnIndices().isEmpty()) { - LOG.debug("Skip folding in distinct subqueries " + op); + if (LOG.isDebugEnabled()) { + LOG.debug("Skip folding in distinct subqueries " + op); + } cppCtx.getOpToConstantExprs().put(op, new HashMap<ColumnInfo, ExprNodeDesc>()); return null; } @@ -1150,7 +1194,9 @@ public final class ConstantPropagateProcFactory { LOG.debug("Skip JOIN-RS structure."); return null; } - LOG.info("Old exprs " + conf.getExprs()); + if (LOG.isInfoEnabled()) { + LOG.info("Old exprs " + conf.getExprs()); + } Iterator<Entry<Byte, List<ExprNodeDesc>>> itr = conf.getExprs().entrySet().iterator(); while (itr.hasNext()) { Entry<Byte, List<ExprNodeDesc>> e = itr.next(); @@ -1163,14 +1209,18 @@ public final class ConstantPropagateProcFactory { for (ExprNodeDesc expr : exprs) { ExprNodeDesc newExpr = foldExpr(expr, constants, cppCtx, op, tag, false); if (newExpr instanceof ExprNodeConstantDesc) { - LOG.info("expr " + newExpr + " fold from " + expr + " is removed."); + if (LOG.isInfoEnabled()) { + LOG.info("expr " + newExpr + " fold from " + expr + " is removed."); + } continue; } newExprs.add(newExpr); } e.setValue(newExprs); } - LOG.info("New exprs " + conf.getExprs()); + if (LOG.isInfoEnabled()) { + LOG.info("New exprs " + conf.getExprs()); + } for (List<ExprNodeDesc> v : conf.getFilters().values()) { for (int i = 0; i < v.size(); i++) {
