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 b539787..43abb60 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 @@ -19,7 +19,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; -import java.util.HashMap; +import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -28,7 +28,7 @@ import java.util.Set; import java.util.Stack; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.ql.exec.ColumnInfo; import org.apache.hadoop.hive.ql.exec.FileSinkOperator; @@ -106,7 +106,8 @@ * Factory for generating the different node processors used by ConstantPropagate. */ public final class ConstantPropagateProcFactory { - protected static final Logger LOG = LoggerFactory.getLogger(ConstantPropagateProcFactory.class.getName()); + protected static final Logger LOG = + LoggerFactory.getLogger(ConstantPropagateProcFactory.class); protected static Set> propagatableUdfs = new HashSet>(); static { @@ -130,9 +131,6 @@ public static ColumnInfo resolveColumn(RowSchema rs, if (ci == null) { ci = rs.getColumnInfo(desc.getColumn()); } - if (ci == null) { - return null; - } return ci; } @@ -182,9 +180,7 @@ private static ExprNodeConstantDesc typeCast(ExprNodeDesc desc, TypeInfo ti, boo if (unSupportedTypes.contains(priti.getPrimitiveCategory()) || unSupportedTypes.contains(descti.getPrimitiveCategory())) { // FIXME: support template types. It currently has conflict with ExprNodeConstantDesc - if (LOG.isDebugEnabled()) { - LOG.debug("Unsupported types " + priti + "; " + descti); - } + LOG.debug("Unsupported types {}; {}", priti, descti); return null; } @@ -195,15 +191,12 @@ private static ExprNodeConstantDesc typeCast(ExprNodeDesc desc, TypeInfo ti, boo priti.getPrimitiveCategory()) && !unsafeConversionTypes.contains( descti.getPrimitiveCategory()); if (performSafeTypeCast && brokenDataTypesCombination) { - if (LOG.isDebugEnabled()) { - LOG.debug("Unsupported cast " + priti + "; " + descti); - } + LOG.debug("Unsupported cast {}; {}", priti, descti); return null; } - if (LOG.isDebugEnabled()) { - LOG.debug("Casting " + desc + " to type " + ti); - } + LOG.debug("Casting {} to type {},", desc, 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. @@ -220,13 +213,13 @@ private static ExprNodeConstantDesc typeCast(ExprNodeDesc desc, TypeInfo ti, boo if (convObj instanceof Integer) { switch (priti.getPrimitiveCategory()) { case BYTE: - convObj = new Byte((byte) (((Integer) convObj).intValue())); + convObj = Byte.valueOf(((Integer) convObj).byteValue()); break; case SHORT: - convObj = new Short((short) ((Integer) convObj).intValue()); + convObj = Short.valueOf(((Integer) convObj).shortValue()); break; case LONG: - convObj = new Long(((Integer) convObj).intValue()); + convObj = Long.valueOf(((Integer) convObj).longValue()); default: } } @@ -297,7 +290,7 @@ private static ExprNodeDesc foldNegative(ExprNodeDesc desc) throws UDFArgumentEx for(ExprNodeDesc grandChild : grandChildren) { newGrandChildren.add(foldNegative( ExprNodeGenericFuncDesc.newInstance(new GenericUDFOPNot(), - Arrays.asList(grandChild)))); + Collections.singletonList(grandChild)))); } return ExprNodeGenericFuncDesc.newInstance( @@ -355,9 +348,8 @@ private static ExprNodeDesc foldExprShortcut(ExprNodeDesc desc, Map " + shortcut); - } + LOG.debug("Folding expression: {} -> {}", desc, shortcut); return shortcut; } ((ExprNodeGenericFuncDesc) desc).setChildren(newExprs); @@ -414,26 +404,22 @@ private static ExprNodeDesc foldExprFull(ExprNodeDesc desc, Map " + constant); - } + 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) { - if (LOG.isDebugEnabled()) { - LOG.debug("Folding expression:" + desc + " -> " + shortcut); - } + LOG.debug("Folding expression: {} -> {}", desc, shortcut); return shortcut; } ((ExprNodeGenericFuncDesc) desc).setChildren(newExprs); @@ -449,15 +435,13 @@ private static ExprNodeDesc foldExprFull(ExprNodeDesc desc, Map parent = op.getParentOperators().get(tag); ExprNodeDesc col = evaluateColumn((ExprNodeColumnDesc) desc, cppCtx, parent); if (col != null) { - if (LOG.isDebugEnabled()) { - LOG.debug("Folding expression:" + desc + " -> " + col); - } + LOG.debug("Folding expression: {} -> {}", desc, col); return col; } } @@ -540,9 +524,9 @@ private static void propagate(GenericUDF udf, List newExprs, RowSc } ColumnInfo ci = resolveColumn(rs, c); if (ci != null) { - if (LOG.isDebugEnabled()) { - LOG.debug("Filter " + udf + " is identified as a value assignment, propagate it."); - } + LOG.debug( + "Filter {} is identified as a value assignment, propagate it.", + udf); if (!v.getTypeInfo().equals(ci.getType())) { v = typeCast(v, ci.getType(), true); } @@ -553,9 +537,9 @@ private static void propagate(GenericUDF udf, List newExprs, RowSc } else if (udf instanceof GenericUDFOPNull) { ExprNodeDesc operand = newExprs.get(0); if (operand instanceof ExprNodeColumnDesc) { - if (LOG.isDebugEnabled()) { - LOG.debug("Filter " + udf + " is identified as a value assignment, propagate it."); - } + LOG.debug( + "Filter {} is identified as a value assignment, propagate it.", + udf); ExprNodeColumnDesc c = (ExprNodeColumnDesc) operand; ColumnInfo ci = resolveColumn(rs, c); if (ci != null) { @@ -760,16 +744,14 @@ private static ExprNodeDesc shortcutFunction(GenericUDF udf, List } else if(thenVal.equals(elseVal)){ return thenExpr; } else if (thenVal instanceof Boolean && elseVal instanceof Boolean) { - List children = new ArrayList<>(); - children.add(whenExpr); - children.add(new ExprNodeConstantDesc(false)); + List children = + Arrays.asList(whenExpr, new ExprNodeConstantDesc(false)); ExprNodeGenericFuncDesc func = ExprNodeGenericFuncDesc.newInstance(new GenericUDFCoalesce(), children); if (Boolean.TRUE.equals(thenVal)) { return func; } else { - List exprs = new ArrayList<>(); - exprs.add(func); + List exprs = Collections.singletonList(func); return ExprNodeGenericFuncDesc.newInstance(new GenericUDFOPNot(), exprs); } } else { @@ -813,16 +795,14 @@ private static ExprNodeDesc shortcutFunction(GenericUDF udf, List } else if (thenVal instanceof Boolean && elseVal instanceof Boolean) { ExprNodeGenericFuncDesc equal = ExprNodeGenericFuncDesc.newInstance( new GenericUDFOPEqual(), newExprs.subList(0, 2)); - List children = new ArrayList<>(); - children.add(equal); - children.add(new ExprNodeConstantDesc(false)); + List children = + Arrays.asList(equal, new ExprNodeConstantDesc(false)); ExprNodeGenericFuncDesc func = ExprNodeGenericFuncDesc.newInstance(new GenericUDFCoalesce(), children); if (Boolean.TRUE.equals(thenVal)) { return func; } else { - List exprs = new ArrayList<>(); - exprs.add(func); + List exprs = Collections.singletonList(func); return ExprNodeGenericFuncDesc.newInstance(new GenericUDFOPNot(), exprs); } } else { @@ -855,15 +835,11 @@ private static ExprNodeDesc evaluateColumn(ExprNodeColumnDesc desc, RowSchema rs = parent.getSchema(); ColumnInfo ci = rs.getColumnInfo(desc.getColumn()); if (ci == null) { - if (LOG.isErrorEnabled()) { - LOG.error("Reverse look up of column " + desc + " error!"); - } + LOG.error("Reverse look up of column " + desc + " error!"); ci = rs.getColumnInfo(desc.getTabAlias(), desc.getColumn()); } if (ci == null) { - if (LOG.isErrorEnabled()) { - LOG.error("Can't resolve " + desc.getTabAlias() + "." + desc.getColumn()); - } + LOG.error("Cannot resolve " + desc.getTabAlias() + "." + desc.getColumn()); return null; } ExprNodeDesc constant = null; @@ -906,7 +882,7 @@ private static ExprNodeDesc evaluateFunction(GenericUDF udf, List for (int i = 0; i < exprs.size(); i++) { ExprNodeDesc desc = exprs.get(i); if (desc instanceof ExprNodeConstantDesc) { - ExprNodeConstantDesc constant = (ExprNodeConstantDesc) exprs.get(i); + ExprNodeConstantDesc constant = (ExprNodeConstantDesc) desc; if (!constant.getTypeInfo().equals(oldExprs.get(i).getTypeInfo())) { constant = typeCast(constant, oldExprs.get(i).getTypeInfo()); if (constant == null) { @@ -949,9 +925,7 @@ private static ExprNodeDesc evaluateFunction(GenericUDF udf, List try { ObjectInspector oi = udf.initialize(argois); Object o = udf.evaluate(arguments); - if (LOG.isDebugEnabled()) { - LOG.debug(udf.getClass().getName() + "(" + exprs + ")=" + o); - } + LOG.debug("{} ({})={}", udf.getClass(), exprs, o); if (o == null) { return new ExprNodeConstantDesc( TypeInfoUtils.getTypeInfoFromObjectInspector(oi), o); @@ -975,10 +949,7 @@ private static ExprNodeDesc evaluateFunction(GenericUDF udf, List return new ExprNodeConstantDesc(structType, ObjectInspectorUtils.copyToStandardJavaObject(o, coi)); } else if (!PrimitiveObjectInspectorUtils.isPrimitiveJavaClass(clz)) { - if (LOG.isErrorEnabled()) { - LOG.error("Unable to evaluate " + udf - + ". Return value unrecoginizable."); - } + LOG.error("Unable to evaluate {}. Return value unrecoginizable.", udf); return null; } else { // fall through @@ -1011,9 +982,8 @@ private static void foldOperator(Operator op, for (ColumnInfo col : schema.getSignature()) { ExprNodeDesc constant = constants.get(col); if (constant != null) { - if (LOG.isDebugEnabled()) { - LOG.debug("Replacing column " + col + " with constant " + constant + " in " + op); - } + LOG.debug("Replacing column {} with constant {} in {}", col, constant, + op); if (!col.getType().equals(constant.getTypeInfo())) { constant = typeCast(constant, col.getType()); } @@ -1028,9 +998,7 @@ private static void foldOperator(Operator op, if (colExprMap != null) { for (Entry e : constants.entrySet()) { String internalName = e.getKey().getInternalName(); - if (colExprMap.containsKey(internalName)) { - colExprMap.put(internalName, e.getValue()); - } + colExprMap.computeIfPresent(internalName, (k, v) -> e.getValue()); } } } @@ -1057,13 +1025,9 @@ public Object process(Node nd, Stack stack, NodeProcessorCtx ctx, Object.. ExprNodeConstantDesc c = (ExprNodeConstantDesc) newCondn; if (Boolean.TRUE.equals(c.getValue())) { cppCtx.addOpToDelete(op); - if (LOG.isDebugEnabled()) { - LOG.debug("Filter expression " + condn + " holds true. Will delete it."); - } + LOG.debug("Filter expression {} holds true. Will delete it.", condn); } else if (Boolean.FALSE.equals(c.getValue())) { - if (LOG.isWarnEnabled()) { - LOG.warn("Filter expression " + condn + " holds false!"); - } + LOG.warn("Filter expression {} holds false!", condn); } } if (newCondn instanceof ExprNodeConstantDesc && ((ExprNodeConstantDesc)newCondn).getValue() == null) { @@ -1224,9 +1188,7 @@ public Object process(Node nd, Stack stack, NodeProcessorCtx ctx, Object.. columnExprMap.put(columnNames.get(i), newCol); } } - if (LOG.isDebugEnabled()) { - LOG.debug("New column list:(" + StringUtils.join(colList, " ") + ")"); - } + LOG.debug("New column list: {}", colList); } return null; } @@ -1302,10 +1264,8 @@ public Object process(Node nd, Stack stack, NodeProcessorCtx ctx, Object.. throws SemanticException { Operator op = (Operator) nd; ConstantPropagateProcCtx cppCtx = (ConstantPropagateProcCtx) ctx; - cppCtx.getOpToConstantExprs().put(op, new HashMap()); - if (LOG.isDebugEnabled()) { - LOG.debug("Stop propagate constants on op " + op.getOperatorId()); - } + cppCtx.getOpToConstantExprs().put(op, Collections.emptyMap()); + LOG.debug("Stop propagate constants on op {}", op); return null; } } @@ -1348,20 +1308,16 @@ public Object process(Node nd, Stack stack, NodeProcessorCtx ctx, Object.. && op.getChildOperators().get(0) instanceof JoinOperator) { JoinOperator joinOp = (JoinOperator) op.getChildOperators().get(0); if (skipFolding(joinOp.getConf())) { - if (LOG.isDebugEnabled()) { - LOG.debug("Skip folding in outer join " + op); - } - cppCtx.getOpToConstantExprs().put(op, new HashMap()); + LOG.debug("Skip folding in outer join {}", op); + cppCtx.getOpToConstantExprs().put(op, Collections.emptyMap()); return null; } } if (rsDesc.getDistinctColumnIndices() != null && !rsDesc.getDistinctColumnIndices().isEmpty()) { - if (LOG.isDebugEnabled()) { - LOG.debug("Skip folding in distinct subqueries " + op); - } - cppCtx.getOpToConstantExprs().put(op, new HashMap()); + LOG.debug("Skip folding in distinct subqueries {}", op); + cppCtx.getOpToConstantExprs().put(op, Collections.emptyMap()); return null; } @@ -1405,11 +1361,14 @@ public Object process(Node nd, Stack stack, NodeProcessorCtx ctx, Object.. */ private boolean skipFolding(JoinDesc joinDesc) { for (JoinCondDesc cond : joinDesc.getConds()) { - if (cond.getType() == JoinDesc.INNER_JOIN || cond.getType() == JoinDesc.UNIQUE_JOIN - || cond.getType() == JoinDesc.LEFT_SEMI_JOIN) { + switch (cond.getType()) { + case JoinDesc.INNER_JOIN: + case JoinDesc.UNIQUE_JOIN: + case JoinDesc.LEFT_SEMI_JOIN: continue; + default: + return true; } - return true; } return false; } @@ -1445,9 +1404,7 @@ public Object process(Node nd, Stack stack, NodeProcessorCtx ctx, Object.. LOG.debug("Skip JOIN-RS structure."); return null; } - if (LOG.isInfoEnabled()) { - LOG.info("Old exprs " + conf.getExprs()); - } + LOG.debug("Old exprs {}", conf.getExprs()); Iterator>> itr = conf.getExprs().entrySet().iterator(); while (itr.hasNext()) { Entry> e = itr.next(); @@ -1460,18 +1417,14 @@ public Object process(Node nd, Stack stack, NodeProcessorCtx ctx, Object.. for (ExprNodeDesc expr : exprs) { ExprNodeDesc newExpr = foldExpr(expr, constants, cppCtx, op, tag, false); if (newExpr instanceof ExprNodeConstantDesc) { - if (LOG.isInfoEnabled()) { - LOG.info("expr " + newExpr + " fold from " + expr + " is removed."); - } + LOG.debug("expr {} fold from {} is removed.", newExpr, expr); continue; } newExprs.add(newExpr); } e.setValue(newExprs); } - if (LOG.isInfoEnabled()) { - LOG.info("New exprs " + conf.getExprs()); - } + LOG.debug("New exprs {}", conf.getExprs()); for (List v : conf.getFilters().values()) { for (int i = 0; i < v.size(); i++) {