diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java index 38316bf..42c6ec6 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -34,6 +35,8 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.common.StatsSetupConst; import org.apache.hadoop.hive.ql.CompilationOpContext; @@ -156,6 +159,7 @@ public void setChildOperators( public Configuration getConfiguration() { return configuration; } + public List> getChildOperators() { return childOperators; } @@ -168,18 +172,9 @@ public int getNumChild() { * Implements the getChildren function for the Node Interface. */ @Override - public ArrayList getChildren() { - - if (getChildOperators() == null) { - return null; - } - - ArrayList ret_vec = new ArrayList(); - for (Operator op : getChildOperators()) { - ret_vec.add(op); - } - - return ret_vec; + public List getChildren() { + List> childOps = getChildOperators(); + return (childOps == null) ? null : Collections.unmodifiableList(childOps); } public void setParentOperators( @@ -244,7 +239,6 @@ public RowSchema getSchema() { // for output rows of this operator protected transient ObjectInspector outputObjInspector; - /** * This function is not named getId(), to make sure java serialization does * NOT serialize it. Some TestParse tests will fail if we serialize this @@ -299,11 +293,7 @@ public void setAlias(String alias) { */ protected boolean areAllParentsInitialized() { for (Operator parent : parentOperators) { - if (parent == null) { - //return true; - continue; - } - if (parent.state != State.INIT) { + if (parent != null && parent.state != State.INIT) { return false; } } @@ -324,8 +314,6 @@ protected boolean areAllParentsInitialized() { public final void initialize(Configuration hconf, ObjectInspector[] inputOIs) throws HiveException { - // String className = this.getClass().getName(); - this.done = false; this.runTimeNumRows = 0; // initializeOp can be overridden // Initializing data structures for vectorForward @@ -339,9 +327,7 @@ public final void initialize(Configuration hconf, ObjectInspector[] inputOIs) return; } - if (LOG.isInfoEnabled()) { - LOG.info("Initializing operator " + this); - } + LOG.info("Initializing Operator: {}", this); if (inputOIs != null) { inputObjInspectors = inputOIs; @@ -378,9 +364,8 @@ public final void initialize(Configuration hconf, ObjectInspector[] inputOIs) || childOperatorsArray.length != childOperators.size()) { throw new AssertionError("Internal error during operator initialization"); } - if (LOG.isDebugEnabled()) { - LOG.debug("Initialization Done " + id + " " + getName()); - } + + LOG.debug("Initialization Done: {}", this); initializeChildren(hconf); isInitOk = true; @@ -391,9 +376,7 @@ public final void initialize(Configuration hconf, ObjectInspector[] inputOIs) } } - if (LOG.isDebugEnabled()) { - LOG.debug("Initialization Done " + id + " " + getName() + " done is reset."); - } + LOG.debug("Initialization Done - Reset: {}", this); // let's wait on the async ops before continuing completeInitialization(asyncInitOperations); @@ -453,7 +436,6 @@ private void completeInitialization(Collection> fs) throws HiveExcepti } } } - } } @@ -465,7 +447,6 @@ private void completeInitialization(Collection> fs) throws HiveExcepti throw new HiveException("Async Initialization failed. abortRequested=" + abortOp.get(), asyncEx); } - completeInitializationOp(os); } @@ -499,7 +480,7 @@ protected void initializeOp(Configuration hconf) throws HiveException { public String getCounterName(Counter counter, Configuration hconf) { String context = hconf.get(Operator.CONTEXT_NAME_KEY, ""); - if (context != null && !context.isEmpty()) { + if (StringUtils.isNotEmpty(context)) { context = "_" + context.replace(" ", "_"); } return counter + context; @@ -511,15 +492,14 @@ public String getCounterName(Counter counter, Configuration hconf) { */ protected void initializeChildren(Configuration hconf) throws HiveException { state = State.INIT; - if (LOG.isDebugEnabled()) { - LOG.debug("Operator " + id + " " + getName() + " initialized"); - } - if (childOperators == null || childOperators.isEmpty()) { + LOG.debug("Operator Initialized: {}", this); + + if (CollectionUtils.isEmpty(childOperators)) { return; } - if (LOG.isDebugEnabled()) { - LOG.debug("Initializing children of " + id + " " + getName()); - } + + LOG.debug("Initializing Children: {}", this); + for (int i = 0; i < childOperatorsArray.length; i++) { childOperatorsArray[i].initialize(hconf, outputObjInspector, childOperatorsTag[i]); if (reporter != null) { @@ -529,7 +509,7 @@ protected void initializeChildren(Configuration hconf) throws HiveException { } public void abort() { - LOG.info("Received abort in operator: {}", getName()); + LOG.info("Received Abort in Operator: {}", this); abortOp.set(true); } @@ -539,7 +519,7 @@ public void abort() { public void passExecContext(ExecMapperContext execContext) { this.setExecContext(execContext); for (int i = 0; i < childOperators.size(); i++) { - childOperators.get(i).passExecContext(execContext); + childOperators.get(i).passExecContext(execContext); } } @@ -556,9 +536,7 @@ public void passExecContext(ExecMapperContext execContext) { */ protected void initialize(Configuration hconf, ObjectInspector inputOI, int parentId) throws HiveException { - if (LOG.isDebugEnabled()) { - LOG.debug("Initializing child " + id + " " + getName()); - } + LOG.debug("Initializing Child : {}", this); // Double the size of the array if needed if (parentId >= inputObjInspectors.length) { int newLength = inputObjInspectors.length * 2; @@ -597,45 +575,35 @@ public ObjectInspector getOutputObjInspector() { public abstract void process(Object row, int tag) throws HiveException; protected final void defaultStartGroup() throws HiveException { - if (LOG.isDebugEnabled()) { - LOG.debug("Starting group"); - } + LOG.debug("Starting group"); if (childOperators == null) { return; } - if (LOG.isDebugEnabled()) { - LOG.debug("Starting group for children:"); - } + LOG.debug("Starting group for children"); + for (Operator op : childOperators) { op.startGroup(); } - if (LOG.isDebugEnabled()) { - LOG.debug("Start group Done"); - } + LOG.debug("Start group Done"); } protected final void defaultEndGroup() throws HiveException { - if (LOG.isDebugEnabled()) { - LOG.debug("Ending group"); - } + LOG.debug("Ending group"); if (childOperators == null) { return; } - if (LOG.isDebugEnabled()) { - LOG.debug("Ending group for children:"); - } + LOG.debug("Ending group for children:"); + for (Operator op : childOperators) { op.endGroup(); } - if (LOG.isDebugEnabled()) { - LOG.debug("End group Done"); - } + LOG.debug("End group Done"); } // If a operator wants to do some work at the beginning of a group @@ -663,33 +631,29 @@ public void flush() throws HiveException { // Recursive flush to flush all the tree operators public void flushRecursive() throws HiveException { flush(); - if (childOperators == null) { - return; - } - - for (Operator child : childOperators) { - child.flushRecursive(); + if (childOperators != null) { + for (Operator child : childOperators) { + child.flushRecursive(); + } } } public void processGroup(int tag) throws HiveException { - if (childOperators == null || childOperators.isEmpty()) { - return; - } - for (int i = 0; i < childOperatorsArray.length; i++) { - childOperatorsArray[i].processGroup(childOperatorsTag[i]); + if (CollectionUtils.isNotEmpty(childOperators)) { + for (int i = 0; i < childOperatorsArray.length; i++) { + childOperatorsArray[i].processGroup(childOperatorsTag[i]); + } } } protected boolean allInitializedParentsAreClosed() { if (parentOperators != null) { for (Operator parent : parentOperators) { - if(parent==null){ + if (parent == null) { continue; } - if (LOG.isDebugEnabled()) { - LOG.debug("allInitializedParentsAreClosed? parent.state = " + parent.state); - } + LOG.debug("allInitializedParentsAreClosed? parent.state = {}", + parent.state); if (!(parent.state == State.CLOSE || parent.state == State.UNINIT)) { return false; } @@ -702,9 +666,7 @@ protected boolean allInitializedParentsAreClosed() { // since it is called by its parents' main thread, so no // more than 1 thread should call this close() function. public void close(boolean abort) throws HiveException { - if (LOG.isDebugEnabled()) { - LOG.debug("close called for operator " + this); - } + LOG.debug("Close Called for Operator: {}", this); if (state == State.CLOSE) { return; @@ -712,22 +674,17 @@ public void close(boolean abort) throws HiveException { // check if all parents are finished if (!allInitializedParentsAreClosed()) { - if (LOG.isDebugEnabled()) { - LOG.debug("Not all parent operators are closed. Not closing."); - } + LOG.debug("Not all parent operators are closed. Not closing."); return; } // set state as CLOSE as long as all parents are closed // state == CLOSE doesn't mean all children are also in state CLOSE state = State.CLOSE; - if (LOG.isDebugEnabled()) { - LOG.info("Closing operator " + this); - } + LOG.info("Closing Operator: {}", this); abort |= abortOp.get(); - // call the operator specific close routine closeOp(abort); @@ -750,17 +707,13 @@ public void close(boolean abort) throws HiveException { } for (Operator op : childOperators) { - if (LOG.isDebugEnabled()) { - LOG.debug("Closing child = " + op); - } + LOG.debug("Closing Child: {} ", op); op.close(abort); } - if (LOG.isDebugEnabled()) { - LOG.debug(id + " Close done"); - } + LOG.debug("Close Done: {}", this); } catch (HiveException e) { - LOG.warn("Caught exception while closing operator: " + e.getMessage(), e); + LOG.warn("Caught exception while closing operator", e); throw e; } } @@ -1008,7 +961,6 @@ public void reset(){ o.reset(); } } - } /** @@ -1052,14 +1004,14 @@ static public String getOperatorName() { * @return null if the operator doesn't change columns */ public Map getColumnExprMap() { - if(this.getConf() == null) { + if (this.getConf() == null) { return null; } return this.getConf().getColumnExprMap(); } public void setColumnExprMap(Map colExprMap) { - if(this.getConf() != null) { + if (this.getConf() != null) { this.getConf().setColumnExprMap(colExprMap); } } @@ -1173,7 +1125,7 @@ public void setMarker(String marker) { } public void initOperatorId() { - this.operatorId = getName() + "_" + this.id; + this.operatorId = getName() + '_' + this.id; } /* @@ -1268,8 +1220,8 @@ public boolean supportSkewJoinOptimization() { T descClone = (T)conf.clone(); // also clone the colExprMap by default // we need a deep copy - ArrayList colInfos = new ArrayList<>(); - colInfos.addAll(getSchema().getSignature()); + ArrayList colInfos = + new ArrayList<>(getSchema().getSignature()); Map map = null; if (getColumnExprMap() != null) { map = new HashMap<>(); @@ -1413,7 +1365,7 @@ public boolean acceptLimitPushdown() { @Override public String toString() { - return getName() + "[" + getIdentifier() + "]"; + return getName() + '[' + getIdentifier() + ']'; } public static String toString(Collection top) { @@ -1456,40 +1408,28 @@ static boolean toString(StringBuilder builder, Set visited, Operator } public Statistics getStatistics() { - if (conf != null) { - return conf.getStatistics(); - } - - return null; + return (conf == null) ? null : conf.getStatistics(); } public OpTraits getOpTraits() { - if (conf != null) { - return conf.getTraits(); - } - - return null; + return (conf == null) ? null : conf.getTraits(); } public void setOpTraits(OpTraits metaInfo) { - if (LOG.isInfoEnabled()) { - LOG.debug("Setting traits (" + metaInfo + ") on " + this); - } + LOG.debug("Setting traits ({}) on: {}", metaInfo, this); if (conf != null) { conf.setTraits(metaInfo); } else { - LOG.warn("Cannot set traits when there's no descriptor: " + this); + LOG.warn("Cannot set traits when there is no descriptor: {}", this); } } public void setStatistics(Statistics stats) { - if (LOG.isInfoEnabled()) { - LOG.debug("Setting stats (" + stats + ") on " + this); - } + LOG.debug("Setting stats ({}) on: {}", stats, this); if (conf != null) { conf.setStatistics(stats); } else { - LOG.warn("Cannot set stats when there's no descriptor: " + this); + LOG.warn("Cannot set stats when there is no descriptor: {}", this); } }