DRILL-7502: Invalid codegen for typeof() with UNION

Also fixes DRILL-6362: typeof() reports NULL for primitive
columns with a NULL value.

typeof() is meant to return "NULL" if a UNION has a NULL
value, but the column type when known, such as for non-UNION
columns.

Also fixes DRILL-7499: sqltypeof() function with an array returns
"ARRAY", not type. This was due to treating REPEATED like LIST.

Handling of the Union vector in code gen is problematic
with about three special cases. Existing code handled two
of the cases. This change handles the third case.

Figuring out the change required poking around quite a bit
of unclear code. Added comments and restructuring to make
that code a bit more clear.

The fix modified code gen for the Union Holder. It can now
"go back in time" to add the union reader at the point we
need it.

closes #1945
diff --git a/common/src/main/java/org/apache/drill/common/types/Types.java b/common/src/main/java/org/apache/drill/common/types/Types.java
index 5cdae56..8660989 100644
--- a/common/src/main/java/org/apache/drill/common/types/Types.java
+++ b/common/src/main/java/org/apache/drill/common/types/Types.java
@@ -232,6 +232,7 @@
       case NULL:            return "NULL";
       case UNION:           return "UNION";
       case GENERIC_OBJECT:  return "JAVA_OBJECT";
+      case LIST:            return "LIST";
 
       // Internal types not actually used at level of SQL types(?):
 
@@ -254,9 +255,11 @@
    * if type is a decimal
    */
   public static String getExtendedSqlTypeName(MajorType type) {
-
-    String typeName = getSqlTypeName(type);
+    String typeName = getBaseSqlTypeName(type);
     switch (type.getMinorType()) {
+    case LIST:
+      typeName = "ARRAY";
+      break;
     case DECIMAL9:
     case DECIMAL18:
     case DECIMAL28SPARSE:
diff --git a/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveArrays.java b/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveArrays.java
index 9513cbb..3fccdce 100644
--- a/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveArrays.java
+++ b/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveArrays.java
@@ -390,7 +390,7 @@
             "EXPR$6", "EXPR$7"
         )
         .baselineValues(
-            "ARRAY", "ARRAY", // why not ARRAY<INTEGER> | ARRAY<ARRAY<INTEGER>> ?
+            "INTEGER", "ARRAY", // why not ARRAY<INTEGER> | ARRAY<ARRAY<INTEGER>> ?
             "INT", "LIST",    // todo: is it ok ?
             "ARRAY", "ARRAY",
             "INT", "LIST"    // todo: is it ok ?
diff --git a/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveMaps.java b/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveMaps.java
index 4f2118e..e2f166b 100644
--- a/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveMaps.java
+++ b/contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveMaps.java
@@ -810,12 +810,11 @@
         .sqlQuery("SELECT sqlTypeOf(%1$s) sto, typeOf(%1$s) to, modeOf(%1$s) mo, drillTypeOf(%1$s) dto " +
             "FROM hive.map_tbl LIMIT 1", "int_string")
         .unOrdered()
-        .baselineColumns("sto", "to", "mo", "dto")
-        .baselineValues("MAP", "DICT<INT,VARCHAR>", "NOT NULL", "DICT<INT,VARCHAR>")
+        .baselineColumns("sto", "to",                "mo",       "dto")
+        .baselineValues( "MAP", "DICT<INT,VARCHAR>", "NOT NULL", "DICT")
         .go();
   }
 
-
   @Test
   public void mapStringToUnion() throws Exception {
     testBuilder()
diff --git a/exec/java-exec/src/main/codegen/templates/TypeHelper.java b/exec/java-exec/src/main/codegen/templates/TypeHelper.java
index 0e1a2dc..85668d0 100644
--- a/exec/java-exec/src/main/codegen/templates/TypeHelper.java
+++ b/exec/java-exec/src/main/codegen/templates/TypeHelper.java
@@ -33,12 +33,14 @@
 import org.apache.drill.exec.vector.accessor.*;
 import org.apache.drill.exec.vector.complex.RepeatedMapVector;
 import org.apache.drill.exec.util.CallBack;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /*
  * This class is generated using freemarker and the ${.template_name} template.
  */
 public class TypeHelper extends BasicTypeHelper {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TypeHelper.class);
+  static final Logger logger = LoggerFactory.getLogger(TypeHelper.class);
 
   public static SqlAccessor getSqlAccessor(ValueVector vector){
     final MajorType type = vector.getField().getType();
@@ -63,10 +65,11 @@
     case LIST:
     case NULL:
       return new GenericAccessor(vector);
+    default:
+      throw new UnsupportedOperationException(buildErrorMessage("find sql accessor", type));
     }
-    throw new UnsupportedOperationException(buildErrorMessage("find sql accessor", type));
   }
-  
+
   public static JType getHolderType(JCodeModel model, MinorType type, DataMode mode){
     switch (type) {
     case UNION:
@@ -82,28 +85,27 @@
     case MAP:
     case LIST:
       return model._ref(ComplexHolder.class);
-      
+
 <#list vv.types as type>
   <#list type.minor as minor>
-      case ${minor.class?upper_case}:
-        switch (mode) {
-          case REQUIRED:
-            return model._ref(${minor.class}Holder.class);
-          case OPTIONAL:
-            return model._ref(Nullable${minor.class}Holder.class);
-          case REPEATED:
-            return model._ref(Repeated${minor.class}Holder.class);
-        }
+    case ${minor.class?upper_case}:
+      switch (mode) {
+        case REQUIRED:
+          return model._ref(${minor.class}Holder.class);
+        case OPTIONAL:
+          return model._ref(Nullable${minor.class}Holder.class);
+        case REPEATED:
+          return model._ref(Repeated${minor.class}Holder.class);
+      }
   </#list>
 </#list>
-      case GENERIC_OBJECT:
-        return model._ref(ObjectHolder.class);
+    case GENERIC_OBJECT:
+      return model._ref(ObjectHolder.class);
     case NULL:
       return model._ref(UntypedNullHolder.class);
-      default:
-        break;
-      }
+    default:
       throw new UnsupportedOperationException(buildErrorMessage("get holder type", type, mode));
+    }
   }
 
   public static JType getComplexHolderType(JCodeModel model, MinorType type, DataMode mode) {
diff --git a/exec/java-exec/src/main/codegen/templates/UnionFunctions.java b/exec/java-exec/src/main/codegen/templates/UnionFunctions.java
index 82aac2f..02609c4 100644
--- a/exec/java-exec/src/main/codegen/templates/UnionFunctions.java
+++ b/exec/java-exec/src/main/codegen/templates/UnionFunctions.java
@@ -43,15 +43,15 @@
  * Additional functions can be found in the class UnionFunctions
  */
 public class GUnionFunctions {
-
   <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
   <#assign fields = minor.fields!type.fields />
   <#assign uncappedName = name?uncap_first/>
 
   <#if !minor.class?starts_with("Decimal")>
-
-  @SuppressWarnings("unused")
-  @FunctionTemplate(name = "IS_${name?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
+  @FunctionTemplate(
+      name = "IS_${name?upper_case}",
+      scope = FunctionTemplate.FunctionScope.SIMPLE,
+      nulls = NullHandling.INTERNAL)
   public static class UnionIs${name} implements DrillSimpleFunc {
 
     @Param UnionHolder in;
@@ -68,8 +68,10 @@
     }
   }
 
-  @SuppressWarnings("unused")
-  @FunctionTemplate(name = "ASSERT_${name?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
+  @FunctionTemplate(
+      name = "ASSERT_${name?upper_case}",
+      scope = FunctionTemplate.FunctionScope.SIMPLE,
+      nulls = NullHandling.INTERNAL)
   public static class CastUnion${name} implements DrillSimpleFunc {
 
     @Param UnionHolder in;
@@ -85,9 +87,6 @@
       }
     }
   }
-
   </#if>
-
   </#list></#list>
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java
index 867b7bd..4c89994 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java
@@ -69,7 +69,7 @@
 
         // Generate class as plain-old Java
 
-         logger.trace(String.format("Class %s generated as plain Java", cg.getClassName()));
+        logger.trace(String.format("Class %s generated as plain Java", cg.getClassName()));
         return classBuilder.getImplementationClass(cg);
       } else {
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/exception/ClassTransformationException.java b/exec/java-exec/src/main/java/org/apache/drill/exec/exception/ClassTransformationException.java
index 14b3cd2..f0561c0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/exception/ClassTransformationException.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/exception/ClassTransformationException.java
@@ -19,12 +19,10 @@
 
 import org.apache.drill.common.exceptions.DrillException;
 
-public class ClassTransformationException extends DrillException{
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClassTransformationException.class);
+@SuppressWarnings("serial")
+public class ClassTransformationException extends DrillException {
 
-  public ClassTransformationException() {
-    super();
-  }
+  public ClassTransformationException() { }
 
   public ClassTransformationException(String message, Throwable cause, boolean enableSuppression,
       boolean writableStackTrace) {
@@ -42,6 +40,4 @@
   public ClassTransformationException(Throwable cause) {
     super(cause);
   }
-
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/BooleanPredicate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/BooleanPredicate.java
index 520b59f..09cbec9 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/BooleanPredicate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/BooleanPredicate.java
@@ -22,10 +22,13 @@
 import org.apache.drill.common.expression.LogicalExpression;
 import org.apache.drill.common.expression.visitors.ExprVisitor;
 import org.apache.drill.exec.expr.stat.RowsMatch;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.List;
 
 public abstract class BooleanPredicate<C extends Comparable<C>> extends BooleanOperator implements FilterPredicate<C> {
+  private static final Logger logger = LoggerFactory.getLogger(BooleanPredicate.class);
 
   private BooleanPredicate(String name, List<LogicalExpression> args, ExpressionPosition pos) {
     super(name, args, pos);
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java
index 969b506..2c2b634 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java
@@ -48,6 +48,7 @@
 import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
 import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
 import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+
 import com.sun.codemodel.JBlock;
 import com.sun.codemodel.JCatchBlock;
 import com.sun.codemodel.JClass;
@@ -66,14 +67,12 @@
 import com.sun.codemodel.JVar;
 import org.apache.drill.exec.server.options.OptionSet;
 
-public class ClassGenerator<T>{
+public class ClassGenerator<T> {
 
   public static final GeneratorMapping DEFAULT_SCALAR_MAP = GM("doSetup", "doEval", null, null);
   public static final GeneratorMapping DEFAULT_CONSTANT_MAP = GM("doSetup", "doSetup", null, null);
   public static final String INNER_CLASS_FIELD_NAME = "innerClassField";
 
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClassGenerator.class);
-
   public enum BlockType {SETUP, EVAL, RESET, CLEANUP}
 
   private final SignatureHolder sig;
@@ -97,36 +96,41 @@
   private JVar innerClassField;
 
   /**
-   * Assumed that field has 3 indexes within the constant pull: index of the CONSTANT_Fieldref_info +
-   * CONSTANT_Fieldref_info.name_and_type_index + CONSTANT_NameAndType_info.name_index.
-   * CONSTANT_NameAndType_info.descriptor_index has limited range of values, CONSTANT_Fieldref_info.class_index is
-   * the same for a single class, they will be taken into account later.
+   * Assume that field has 3 indexes within the constant pull: index of the<br>
+   * CONSTANT_Fieldref_info + CONSTANT_Fieldref_info.name_and_type_index + CONSTANT_NameAndType_info.name_index.
+   * CONSTANT_NameAndType_info.descriptor_index has limited range of values,
+   * CONSTANT_Fieldref_info.class_index is the same for a single class, they will
+   * be taken into account later.
    * <p>
    * Local variable has 1 index within the constant pool.
    * {@link org.objectweb.asm.MethodWriter#visitLocalVariable(String, String, String, Label, Label, int)}
    * <p>
-   * For upper estimation of max index value, suppose that each field and local variable uses different literal
-   * values that have two indexes, then the number of occupied indexes within the constant pull is
+   * For upper estimation of max index value, suppose that each field and local
+   * variable uses different literal values that have two indexes, then the
+   * number of occupied indexes within the constant pull is<br>
    * fieldCount * 3 + fieldCount * 2 + (index - fieldCount) * 3 => fieldCount * 2 + index * 3
    * <p>
-   * Assumed that method has 3 indexes within the constant pull: index of the CONSTANT_Methodref_info +
-   * CONSTANT_Methodref_info.name_and_type_index + CONSTANT_NameAndType_info.name_index.
+   * Assumed that method has 3 indexes within the constant pull: index of the
+   * CONSTANT_Methodref_info + CONSTANT_Methodref_info.name_and_type_index +
+   * CONSTANT_NameAndType_info.name_index.
    * <p>
-   * For the upper estimation of number of split methods suppose that each expression in the method uses single variable.
-   * Suppose that the max number of indexes within the constant pull occupied by fields and local variables is M,
-   * the number of split methods is N, number of abstract methods in the template is A, then splitted methods count is
+   * For the upper estimation of number of split methods suppose that each
+   * expression in the method uses single variable. Suppose that the max number
+   * of indexes within the constant pull occupied by fields and local variables
+   * is M, the number of split methods is N, number of abstract methods in the
+   * template is A, then splitted methods count is<br>
    * N = (M - A * N * 3) / 50 => N = M / (50 + A * 3)
    * <p>
-   * Additionally should be taken into account class references; fields and methods from the template,
-   * so reserves 1000 for them.
+   * Additionally should be taken into account class references; fields and
+   * methods from the template, so reserves 1000 for them.
    * <p>
-   * Then the size of the occupied part in the constant pull is
+   * Then the size of the occupied part in the constant pull is<br>
    * (fieldCount * 2 + index * 3 + 1000) * (1 + 3 / (50 + A * 3))
    */
   private long maxIndex;
 
-  private int index = 0;
-  private int labelIndex = 0;
+  private int index;
+  private int labelIndex;
   private MappingSet mappings;
 
   public static MappingSet getDefaultMapping() {
@@ -146,7 +150,7 @@
     this.optionManager = optionManager;
     constantVars = new HashMap<>();
 
-    blocks = (LinkedList<SizedJBlock>[]) new LinkedList[sig.size()];
+    blocks = new LinkedList[sig.size()];
     for (int i =0; i < sig.size(); i++) {
       blocks[i] = Lists.newLinkedList();
     }
@@ -210,12 +214,15 @@
   public JBlock getSetupBlock() {
     return getBlock(getCurrentMapping().getMethodName(BlockType.SETUP));
   }
+
   public JBlock getEvalBlock() {
     return getBlock(getCurrentMapping().getMethodName(BlockType.EVAL));
   }
+
   public JBlock getResetBlock() {
     return getBlock(getCurrentMapping().getMethodName(BlockType.RESET));
   }
+
   public JBlock getCleanupBlock() {
     return getBlock(getCurrentMapping().getMethodName(BlockType.CLEANUP));
   }
@@ -290,7 +297,6 @@
     JClass t = model.ref(SchemaChangeException.class);
     JType objClass = model.ref(Object.class);
     JBlock b = getSetupBlock();
-    //JExpr.newArray(model.INT).
 
     JVar fieldArr = b.decl(model.INT.array(), "fieldIds" + index++, JExpr.newArray(model.INT, fieldId.getFieldIds().length));
     int[] fieldIndices = fieldId.getFieldIds();
@@ -299,7 +305,7 @@
     }
 
     JInvocation invoke = batchName
-        .invoke("getValueAccessorById") //
+        .invoke("getValueAccessorById")
         .arg(vvClass.dotclass())
         .arg(fieldArr);
 
@@ -308,7 +314,8 @@
         getNextVar("tmp"),
         invoke.invoke(vectorAccess));
 
-    b._if(obj.eq(JExpr._null()))._then()._throw(JExpr._new(t).arg(JExpr.lit(String.format("Failure while loading vector %s with id: %s.", vv.name(), fieldId.toString()))));
+    b._if(obj.eq(JExpr._null()))._then()._throw(JExpr._new(t).arg(JExpr.lit(
+        String.format("Failure while loading vector %s with id: %s.", vv.name(), fieldId.toString()))));
     //b.assign(vv, JExpr.cast(retClass, ((JExpression) JExpr.cast(wrapperClass, obj)).invoke(vectorAccess)));
     b.assign(vv, JExpr.cast(retClass, obj));
     vvDeclaration.put(setup, vv);
@@ -414,7 +421,6 @@
    * @param mode the {@link BlkCreateMode block create mode}
    * for the new block.
    */
-
   private void rotateBlock(BlkCreateMode mode) {
     boolean blockRotated = false;
     for (LinkedList<SizedJBlock> b : blocks) {
@@ -433,7 +439,8 @@
   }
 
   /**
-   * Creates methods from the signature {@code sig} with body from the appropriate {@code blocks}.
+   * Creates methods from the signature {@code sig} with body from the
+   * appropriate {@code blocks}.
    */
   void flushCode() {
     if (innerClassGenerator != null) {
@@ -454,9 +461,9 @@
       int methodIndex = 0;
       int exprsInMethod = 0;
       boolean isVoidMethod = method.getReturnType() == void.class;
-      for(SizedJBlock sb : blocks[i++]) {
+      for (SizedJBlock sb : blocks[i++]) {
         JBlock b = sb.getBlock();
-        if(!b.isEmpty()) {
+        if (!b.isEmpty()) {
           if (optionManager != null &&
               exprsInMethod > optionManager.getOption(ExecConstants.CODE_GEN_EXP_IN_METHOD_SIZE_VALIDATOR)) {
             JMethod inner = clazz.method(JMod.PRIVATE, model._ref(method.getReturnType()), method.getMethodName() + methodIndex);
@@ -542,7 +549,7 @@
     if (innerClassGenerator != null && hasMaxIndexValue()) {
       return innerClassGenerator.declareClassField(prefix, t, init);
     }
-    return clazz.field(JMod.NONE, t, prefix + index++, init);
+    return clazz.field(JMod.NONE, t, getNextVar(prefix), init);
   }
 
   public Pair<Integer, JVar> declareClassConstField(String prefix, JType t,
@@ -551,15 +558,14 @@
   }
 
   /**
-   * declare a constant field for the class.
-   * argument {@code function} holds the constant value which
-   * returns a value holder must be set to the class field when the class instance created.
-   * the class field innerClassField will be created if innerClassGenerator exists.
+   * Declare a constant field for the class.
+   * The class field innerClassField will be created if innerClassGenerator exists.
    *
    * @param prefix the prefix name of class field
    * @param t the type of class field
    * @param init init expression
-   * @param function the function holds the constant value
+   * @param function holds the constant value which
+   * returns a value holder must be set to the class field when the class instance created.
    * @return the depth of nested class, class field
    */
   public Pair<Integer, JVar> declareClassConstField(String prefix, JType t, JExpression init,
@@ -571,7 +577,7 @@
       depth = nested.getKey() + 1;
       var = nested.getValue();
     } else {
-      var = clazz.field(JMod.NONE, t, prefix + index++, init);
+      var = clazz.field(JMod.NONE, t, getNextVar(prefix), init);
     }
     Pair<Integer, JVar> depthVar = Pair.of(depth, var);
     constantVars.put(depthVar, function);
@@ -591,7 +597,7 @@
   }
 
   /**
-   * Adds local variable declaration based on given name and type.
+   * Adds a local variable declaration based on given name and type.
    *
    * @param t major type
    * @param name variable name
@@ -602,24 +608,23 @@
     JType holderType = getHolderType(t);
     JVar var;
     if (includeNewInstance) {
-      var = getEvalBlock().decl(holderType, name + index, JExpr._new(holderType));
+      var = getEvalBlock().decl(holderType, getNextVar(name), JExpr._new(holderType));
     } else {
-      var = getEvalBlock().decl(holderType, name + index);
+      var = getEvalBlock().decl(holderType, getNextVar(name));
     }
     JFieldRef outputSet = null;
     if (t.getMode() == DataMode.OPTIONAL) {
       outputSet = var.ref("isSet");
     }
-    index++;
     return new HoldingContainer(t, var, var.ref("value"), outputSet);
   }
 
   public List<TypedFieldId> getWorkspaceTypes() {
-    return this.workspaceTypes;
+    return workspaceTypes;
   }
 
   public Map<WorkspaceReference, JVar> getWorkspaceVectors() {
-    return this.workspaceVectors;
+    return workspaceVectors;
   }
 
   /**
@@ -636,7 +641,6 @@
    * instance of that nested class using a well-defined name. This
    * method overrides the base class method defined for this purpose.</li>
    */
-
   public void preparePlainJava() {
 
     // If this generated class uses the "straight Java" technique
@@ -719,7 +723,6 @@
    * introspection package. But, Drill prefers Java 7 which only provides
    * parameter types.
    */
-
   private void addCtor(Class<?>[] parameters) {
     JMethod ctor = clazz.constructor(JMod.PUBLIC);
     JBlock body = ctor.body();
@@ -791,13 +794,17 @@
       }
       return true;
     }
-
   }
 
   /**
-   * Represents a (Nullable)?(Type)Holder instance.
+   * Represents a (Nullable)?(Type)Holder instance. Allows code
+   * gen to declare, set, work with and retrieve values from a holder.
+   * Holders exploit scalar replacement in Drill (or in Java): that
+   * the holder can, via code rewrites, be replaced by scalars that
+   * do the same job.
    */
-
+  // TODO: Might be better modeled as a set of classes for the special
+  // kinds of holders rather than a generic class and many if-statements.
   public static class HoldingContainer {
     private final JVar holder;
     private final JFieldRef value;
@@ -821,8 +828,18 @@
       this.isReader = isReader;
     }
 
+    public HoldingContainer(HoldingContainer from) {
+      this.holder = from.holder;
+      this.value =from. value;
+      this.isSet = from.isSet;
+      this.type = from.type;
+      this.isConstant = from.isConstant;
+      this.singularRepeated = from.singularRepeated;
+      this.isReader = from.isReader;
+    }
+
     public boolean isReader() {
-      return this.isReader;
+      return isReader;
     }
 
     public boolean isSingularRepeated() {
@@ -839,7 +856,7 @@
     }
 
     public boolean isConstant() {
-      return this.isConstant;
+      return isConstant;
     }
 
     public JVar getHolder() {
@@ -855,7 +872,8 @@
     }
 
     public JFieldRef getIsSet() {
-      Preconditions.checkNotNull(isSet, "You cannot access the isSet variable when operating on a non-nullable output value.");
+      Preconditions.checkNotNull(isSet,
+          "You cannot access the isSet variable when operating on a non-nullable output value.");
       return isSet;
     }
 
@@ -874,14 +892,13 @@
     /**
      * Convert holder to a string for debugging use.
      */
-
     @Override
     public String toString() {
       DebugStringBuilder buf = new DebugStringBuilder(this);
       if (isConstant()) {
         buf.append("const ");
       }
-      buf.append(holder.type().fullName())
+      buf.append(holder.type().name())
         .append(" ")
         .append(holder.name())
         .append(", ")
@@ -890,8 +907,10 @@
         .append(type.getMinorType().name())
         .append(", ");
       holder.generate(buf.formatter());
-      buf.append(", ");
-      value.generate(buf.formatter());
+      if (value != null) {
+        buf.append(", ");
+        value.generate(buf.formatter());
+      }
       return buf.toString();
     }
   }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java
index 1b0a6eb..78ffb5d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java
@@ -26,13 +26,20 @@
 import org.apache.drill.common.types.TypeProtos.MajorType;
 import org.apache.drill.exec.expr.fn.DrillFuncHolder;
 
-public class DrillFuncHolderExpr extends FunctionHolderExpression implements Iterable<LogicalExpression>{
-  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillFuncHolderExpr.class);
+/**
+ * Represents the call of a function within a query and includes
+ * the actual arguments and a reference to the function declaration (as a
+ * "function holder.")
+ */
+public class DrillFuncHolderExpr extends FunctionHolderExpression
+        implements Iterable<LogicalExpression> {
+
   private final DrillFuncHolder holder;
   private final MajorType majorType;
   private DrillSimpleFunc interpreter;
 
-  public DrillFuncHolderExpr(String nameUsed, DrillFuncHolder holder, List<LogicalExpression> args, ExpressionPosition pos) {
+  public DrillFuncHolderExpr(String nameUsed, DrillFuncHolder holder,
+      List<LogicalExpression> args, ExpressionPosition pos) {
     super(nameUsed, pos, args);
     this.holder = holder;
     // since function return type can not be changed, cache it for better performance
@@ -76,9 +83,9 @@
 
   @Override
   public int getCumulativeCost() {
-    int cost = this.getSelfCost();
+    int cost = getSelfCost();
 
-    for (LogicalExpression arg : this.args) {
+    for (LogicalExpression arg : args) {
       cost += arg.getCumulativeCost();
     }
 
@@ -87,7 +94,7 @@
 
   @Override
   public DrillFuncHolderExpr copy(List<LogicalExpression> args) {
-    return new DrillFuncHolderExpr(this.nameUsed, this.holder, args, this.getPosition());
+    return new DrillFuncHolderExpr(nameUsed, holder, args, getPosition());
   }
 
   public void setInterpreter(DrillSimpleFunc interpreter) {
@@ -95,8 +102,7 @@
   }
 
   public DrillSimpleFunc getInterpreter() {
-    return this.interpreter;
+    return interpreter;
   }
-
 }
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
index d8742ff..1c5dc53 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
@@ -78,6 +78,9 @@
 import org.apache.drill.exec.vector.complex.reader.FieldReader;
 
 import org.apache.drill.shaded.guava.com.google.common.base.Function;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.sun.codemodel.JBlock;
 import com.sun.codemodel.JClass;
 import com.sun.codemodel.JConditional;
@@ -93,21 +96,7 @@
  * Visitor that generates code for eval
  */
 public class EvaluationVisitor {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(EvaluationVisitor.class);
-
-  public EvaluationVisitor() {
-  }
-
-  public HoldingContainer addExpr(LogicalExpression e, ClassGenerator<?> generator) {
-
-    Set<LogicalExpression> constantBoundaries;
-    if (generator.getMappingSet().hasEmbeddedConstant()) {
-      constantBoundaries = Collections.emptySet();
-    } else {
-      constantBoundaries = ConstantExpressionIdentifier.getConstantExpressionSet(e);
-    }
-    return e.accept(new CSEFilter(constantBoundaries), generator);
-  }
+  private static final Logger logger = LoggerFactory.getLogger(EvaluationVisitor.class);
 
   /**
    * Callback function for the expression visitor
@@ -116,10 +105,10 @@
     HoldingContainer getHolder();
   }
 
-  private class ExpressionHolder {
-    private LogicalExpression expression;
-    private GeneratorMapping mapping;
-    private MappingSet mappingSet;
+  private static class ExpressionHolder {
+    private final LogicalExpression expression;
+    private final GeneratorMapping mapping;
+    private final MappingSet mappingSet;
 
     ExpressionHolder(LogicalExpression expression, MappingSet mappingSet) {
       this.expression = expression;
@@ -142,9 +131,88 @@
     }
   }
 
-  Map<ExpressionHolder,HoldingContainer> previousExpressions = new HashMap<>();
+  /**
+   * Extended variable descriptor ("holding container") for the variable
+   * which references the value holder ("FooHolder") that stores the value
+   * from a value vector. Used to pass along the original value vector
+   * variable along with the value holder. In particular, this class allows
+   * "time travel": to retroactively generate a reader for a union once
+   * we realize we need the reader.
+   * <p>
+   * TODO: This is not especially elegant. But the code that declares the
+   * holder/reader does not know about the parameter(s) that will use it,
+   * and the normal <code>HoldingContainer</code> can hold only one variable,
+   * not a broader context. This version holds more context. Perhaps this
+   * idea should be generalized.
+   */
+  public static class VectorVariableHolder extends HoldingContainer {
 
-  Stack<Map<ExpressionHolder,HoldingContainer>> mapStack = new Stack<>();
+    private final ClassGenerator<?> classGen;
+    private final JBlock vvSetupBlock;
+    private final int insertPosn;
+    private final JExpression vectorExpr;
+    private final JExpression recordIndex;
+    private JVar readerVar;
+
+    public VectorVariableHolder(HoldingContainer base, ClassGenerator<?> classGen,
+        JBlock vvSetupBlock, JExpression vectorExpr, JExpression recordIndex) {
+      super(base);
+      this.classGen = classGen;
+      this.vvSetupBlock = vvSetupBlock;
+      this.vectorExpr = vectorExpr;
+      this.recordIndex = recordIndex;
+      insertPosn = vvSetupBlock.pos();
+    }
+
+    /**
+     * Specialized hack for the UNION type to obtain a <code>FieldReader</code>
+     * directly from the value vector, bypassing the <code>UnionHolder</code>
+     * created from that vector. Handles setting the reader position only once
+     * in an eval block rather than on each reference. There may be multiple
+     * functions that need the reader. To ensure we create only one common
+     * reader, we "go back in time" to add the reader at the point after
+     * we declared the <code>UnionHolder</code>.
+     */
+    public JVar generateUnionReader() {
+      if (readerVar == null) {
+        createReaderVar();
+      }
+      return readerVar;
+    }
+
+    private void createReaderVar() {
+      // Rewind to where we added the UnionHolder
+      int curPosn = vvSetupBlock.pos();
+      vvSetupBlock.pos(insertPosn);
+
+      // UnionReader readerx = vvy.getReader();
+      JType readerClass = classGen.getModel()._ref(FieldReader.class);
+      JExpression expr = vectorExpr.invoke("getReader");
+      readerVar = vvSetupBlock.decl(readerClass, classGen.getNextVar("reader"), expr);
+
+      // readerx.setPosition(indexExpr);
+      JInvocation setPosnStmt = readerVar.invoke("setPosition").arg(recordIndex);
+      vvSetupBlock.add(setPosnStmt);
+
+      // Put the insert position back to where it was; adjusting
+      // for the statements just added.
+      int offset = vvSetupBlock.pos() - insertPosn;
+      vvSetupBlock.pos(curPosn + offset);
+    }
+  }
+
+  public Map<ExpressionHolder, HoldingContainer> previousExpressions = new HashMap<>();
+  private final Stack<Map<ExpressionHolder, HoldingContainer>> mapStack = new Stack<>();
+
+  public HoldingContainer addExpr(LogicalExpression e, ClassGenerator<?> generator) {
+    Set<LogicalExpression> constantBoundaries;
+    if (generator.getMappingSet().hasEmbeddedConstant()) {
+      constantBoundaries = Collections.emptySet();
+    } else {
+      constantBoundaries = ConstantExpressionIdentifier.getConstantExpressionSet(e);
+    }
+    return e.accept(new CSEFilter(constantBoundaries), generator);
+  }
 
   void newScope() {
     mapStack.push(previousExpressions);
@@ -171,7 +239,7 @@
     previousExpressions.put(new ExpressionHolder(expression, mappingSet), hc);
   }
 
-  private class EvalVisitor extends AbstractExprVisitor<HoldingContainer, ClassGenerator<?>, RuntimeException> {
+  private static class EvalVisitor extends AbstractExprVisitor<HoldingContainer, ClassGenerator<?>, RuntimeException> {
 
     @Override
     public HoldingContainer visitFunctionCall(FunctionCall call, ClassGenerator<?> generator) throws RuntimeException {
@@ -187,34 +255,47 @@
       } else if(op.getName().equals("booleanOr")) {
         return visitBooleanOr(op, generator);
       } else {
-        throw new UnsupportedOperationException("BooleanOperator can only be booleanAnd, booleanOr. You are using " + op.getName());
+        throw new UnsupportedOperationException(
+            "BooleanOperator can only be booleanAnd, booleanOr. You are using " + op.getName());
       }
     }
 
+    /**
+     * Generate a function call (which actually in-lines the function within
+     * the generated code.)
+     */
     @Override
     public HoldingContainer visitFunctionHolderExpression(FunctionHolderExpression holderExpr,
         ClassGenerator<?> generator) throws RuntimeException {
 
-      AbstractFuncHolder holder = (AbstractFuncHolder) holderExpr.getHolder();
+      // Reference to the function definition.
+      AbstractFuncHolder fnHolder = (AbstractFuncHolder) holderExpr.getHolder();
 
-      JVar[] workspaceVars = holder.renderStart(generator, null, holderExpr.getFieldReference());
+      // Define (internal) workspace variables
+      JVar[] workspaceVars = fnHolder.renderStart(generator, null, holderExpr.getFieldReference());
 
-      if (holder.isNested()) {
+      if (fnHolder.isNested()) {
         generator.getMappingSet().enterChild();
       }
 
+      // Create (or obtain) value holders for each of the actual function
+      // arguments. In many cases (all? some?), the function reference
+      // (holder expr) represents arguments as value holders.
       HoldingContainer[] args = new HoldingContainer[holderExpr.args.size()];
       for (int i = 0; i < holderExpr.args.size(); i++) {
         args[i] = holderExpr.args.get(i).accept(this, generator);
       }
 
-      holder.renderMiddle(generator, args, workspaceVars);
+      // Aggregate functions generate the per-value code here.
+      fnHolder.renderMiddle(generator, args, workspaceVars);
 
-      if (holder.isNested()) {
+      if (fnHolder.isNested()) {
         generator.getMappingSet().exitChild();
       }
 
-      return holder.renderEnd(generator, args, workspaceVars, holderExpr);
+      // Simple functions generate per-value code here.
+      // Aggregate functions generate the per-group aggregate here.
+      return fnHolder.renderEnd(generator, args, workspaceVars, holderExpr);
     }
 
     @Override
@@ -369,6 +450,11 @@
         buffer -> ValueHolderHelper.getBitHolder(e.getBoolean() ? 1 : 0));
     }
 
+    /**
+     * Handles implementation-specific expressions not known to the visitor
+     * mechanism.
+     */
+    // TODO: Consider adding these "unknown" expressions to the visitor class.
     @Override
     public HoldingContainer visitUnknown(LogicalExpression e, ClassGenerator<?> generator) throws RuntimeException {
       if (e instanceof ValueVectorReadExpression) {
@@ -378,6 +464,7 @@
       } else if (e instanceof ReturnValueExpression) {
         return visitReturnValueExpression((ReturnValueExpression) e, generator);
       } else if (e instanceof HoldingContainerExpression) {
+        // Expression contains a "holder", just return it.
         return ((HoldingContainerExpression) e).getContainer();
       } else if (e instanceof NullExpression) {
         return generator.declare(e.getMajorType());
@@ -386,12 +473,12 @@
       } else {
         return super.visitUnknown(e, generator);
       }
-
     }
 
     /**
      * <p>
-     * Creates local variable based on given parameter type and name and assigns parameter to this local instance.
+     * Creates local variable based on given parameter type and name and assigns
+     * parameter to this local instance.
      * </p>
      *
      * <p>
@@ -438,19 +525,33 @@
           return outputContainer;
         }
       } else {
-
         final JInvocation setMeth = GetSetVectorHelper.write(e.getChild().getMajorType(), vv, inputContainer, outIndex, e.isSafe() ? "setSafe" : "set");
           if (inputContainer.isOptional()) {
             JConditional jc = block._if(inputContainer.getIsSet().eq(JExpr.lit(0)).not());
             block = jc._then();
           }
           block.add(setMeth);
-
       }
-
       return null;
     }
 
+    /**
+     * Generate code to extract a value from a value vector into a local
+     * variable which can be passed to a function. The local variable is either
+     * a value holder (<code>FooHolder</code>) or a reader. In most cases, the
+     * decision is made based on the type of the vector. Primitives use value
+     * holders, complex variables use readers.
+     * <p>
+     * Code in {@link DrillFuncHolder#declareInputVariable} converts the local
+     * variable into the form requested by the </code>{@literal @}Param</code>
+     * annotation which itself can be a value holder or a reader.
+     * <p>
+     * A special case occurs for the UNION type. Drill supports conversion from
+     * a reader to a holder, but not the other way around. We prefer to use a
+     * holder. But, if the function wants a reader, we must generate a reader.
+     * If some other function wants a holder, we can convert from the reader to
+     * a holder.
+     */
     private HoldingContainer visitValueVectorReadExpression(ValueVectorReadExpression e, ClassGenerator<?> generator)
         throws RuntimeException {
       // declare value vector
@@ -458,9 +559,10 @@
       JExpression batchIndex;
       JExpression recordIndex;
 
-      // if value vector read expression has batch reference, use its values in generated code,
-      // otherwise use values provided by mapping set (which point to only one batch)
-      // primary used for non-equi joins where expression conditions may refer to more than one batch
+      // If value vector read expression has batch reference, use its values in
+      // generated code, otherwise use values provided by mapping set (which
+      // point to only one batch) primarily used for non-equi joins where
+      // expression conditions may refer to more than one batch
       BatchReference batchRef = e.getBatchRef();
       if (batchRef != null) {
         batchName = DirectExpression.direct(batchRef.getBatchName());
@@ -480,169 +582,190 @@
       }
 
       // evaluation work.
-      HoldingContainer out = generator.declare(e.getMajorType());
-
+      MajorType type = e.getMajorType();
       final boolean hasReadPath = e.hasReadPath();
-      final boolean complex = Types.isComplex(e.getMajorType());
-      final boolean repeated = Types.isRepeated(e.getMajorType());
-      final boolean listVector = e.getTypedFieldId().isListVector();
+      final boolean complex = Types.isComplex(type);
 
       if (!hasReadPath && !complex) {
-
-        JBlock eval = new JBlock();
-
-        if (repeated) {
-          JExpression expr = vv1.invoke("getReader");
-          // Set correct position to the reader
-          eval.add(expr.invoke("reset"));
-          eval.add(expr.invoke("setPosition").arg(recordIndex));
-        }
-
-        GetSetVectorHelper.read(e.getMajorType(),  vv1, eval, out, generator.getModel(), recordIndex);
-        generator.getEvalBlock().add(eval);
-
+        return createHolderForVector(generator, recordIndex, vv1, type);
       } else {
+        return createReaderForVector(e, generator, recordIndex, vv1, type);
+      }
+    }
+
+    private HoldingContainer createHolderForVector(ClassGenerator<?> generator,
+        JExpression recordIndex, JExpression vv1, MajorType type) {
+      JBlock eval = new JBlock();
+      HoldingContainer out = generator.declare(type);
+      if (Types.isRepeated(type)) {
         JExpression expr = vv1.invoke("getReader");
-        PathSegment seg = e.getReadPath();
-
-        JVar isNull = null;
-        boolean isNullReaderLikely = isNullReaderLikely(seg, complex || repeated || listVector);
-        if (isNullReaderLikely) {
-          isNull = generator.getEvalBlock().decl(generator.getModel().INT, generator.getNextVar("isNull"), JExpr.lit(0));
-        }
-
-        JLabel label = generator.getEvalBlock().label("complex");
-        JBlock eval = generator.getEvalBlock().block();
-
-        // position to the correct value.
+        // Set correct position to the reader
         eval.add(expr.invoke("reset"));
         eval.add(expr.invoke("setPosition").arg(recordIndex));
-        int listNum = 0;
-
-        // variable used to store index of key in dict (if there is one)
-        // if entry for the key is not found, will be assigned a value of SingleDictReaderImpl#NOT_FOUND.
-        JVar valueIndex = eval.decl(generator.getModel().INT, "valueIndex", JExpr.lit(-2));
-
-        int depth = 0;
-
-        while (seg != null) {
-          if (seg.isArray()) {
-
-            // stop once we get to the last segment and the final type is neither complex nor repeated (map, dict, list, repeated list).
-            // In case of non-complex and non-repeated type, we return Holder, in stead of FieldReader.
-            if (seg.isLastPath() && !complex && !repeated && !listVector) {
-              break;
-            }
-
-            if (e.getFieldId().isDict(depth)) {
-              expr = getDictReaderReadByKeyExpression(generator, eval, expr, seg, valueIndex, isNull);
-              seg = seg.getChild();
-              depth++;
-              continue;
-            }
-
-            JVar list = generator.declareClassField("list", generator.getModel()._ref(FieldReader.class));
-            eval.assign(list, expr);
-
-            // if this is an array, set a single position for the expression to
-            // allow us to read the right data lower down.
-            JVar desiredIndex = eval.decl(generator.getModel().INT, "desiredIndex" + listNum,
-                JExpr.lit(seg.getArraySegment().getIndex()));
-            // start with negative one so that we are at zero after first call
-            // to next.
-            JVar currentIndex = eval.decl(generator.getModel().INT, "currentIndex" + listNum, JExpr.lit(-1));
-
-            eval._while( //
-                currentIndex.lt(desiredIndex) //
-                    .cand(list.invoke("next"))).body().assign(currentIndex, currentIndex.plus(JExpr.lit(1)));
-
-
-            JBlock ifNoVal = eval._if(desiredIndex.ne(currentIndex))._then().block();
-            if (out.isOptional()) {
-              ifNoVal.assign(out.getIsSet(), JExpr.lit(0));
-            }
-            ifNoVal.assign(isNull, JExpr.lit(1));
-            ifNoVal._break(label);
-
-            expr = list.invoke("reader");
-            listNum++;
-          } else {
-
-            if (e.getFieldId().isDict(depth)) {
-              MajorType finalType = e.getFieldId().getFinalType();
-              if (seg.getChild() == null && !(Types.isComplex(finalType) || Types.isRepeated(finalType))) {
-                // This is the last segment:
-                eval.add(expr.invoke("read").arg(getKeyExpression(seg, generator)).arg(out.getHolder()));
-                return out;
-              }
-
-              expr = getDictReaderReadByKeyExpression(generator, eval, expr, seg, valueIndex, isNull);
-
-              seg = seg.getChild();
-              depth++;
-              continue;
-            }
-
-            JExpression fieldName = JExpr.lit(seg.getNameSegment().getPath());
-            expr = expr.invoke("reader").arg(fieldName);
-          }
-          seg = seg.getChild();
-          depth++;
-        }
-
-        // expected that after loop depth at least equal to last id index
-        depth = Math.max(depth, e.getFieldId().getFieldIds().length - 1);
-
-        if (complex || repeated) {
-
-          JVar complexReader = generator.declareClassField("reader", generator.getModel()._ref(FieldReader.class));
-
-          if (isNullReaderLikely) {
-            JConditional jc = generator.getEvalBlock()._if(isNull.eq(JExpr.lit(0)));
-
-            JClass nrClass = generator.getModel().ref(org.apache.drill.exec.vector.complex.impl.NullReader.class);
-            JExpression nullReader;
-            if (complex) {
-              nullReader = nrClass.staticRef("EMPTY_MAP_INSTANCE");
-            } else {
-              nullReader = nrClass.staticRef("EMPTY_LIST_INSTANCE");
-            }
-
-            jc._then().assign(complexReader, expr);
-            jc._else().assign(complexReader, nullReader);
-          } else {
-            eval.assign(complexReader, expr);
-          }
-
-          HoldingContainer hc = new HoldingContainer(e.getMajorType(), complexReader, null, null, false, true);
-          return hc;
-        } else {
-          if (seg != null) {
-            JExpression holderExpr = out.getHolder();
-            JExpression argExpr;
-            if (e.getFieldId().isDict(depth)) {
-              holderExpr = JExpr.cast(generator.getModel()._ref(ValueHolder.class), holderExpr);
-              argExpr = getKeyExpression(seg, generator);
-            } else {
-              argExpr = JExpr.lit(seg.getArraySegment().getIndex());
-            }
-            JClass dictReaderClass = generator.getModel().ref(org.apache.drill.exec.vector.complex.impl.SingleDictReaderImpl.class);
-            JConditional jc = eval._if(valueIndex.ne(dictReaderClass.staticRef("NOT_FOUND")));
-            jc._then().add(expr.invoke("read").arg(argExpr).arg(holderExpr));
-          } else {
-            eval.add(expr.invoke("read").arg(out.getHolder()));
-          }
-        }
-
       }
 
+      // Generate code to read the vector into a holder
+      GetSetVectorHelper.read(type, vv1, eval, out, generator.getModel(), recordIndex);
+      JBlock evalBlock = generator.getEvalBlock();
+      evalBlock.add(eval);
+      return new VectorVariableHolder(out, generator, evalBlock, vv1, recordIndex);
+    }
+
+    private HoldingContainer createReaderForVector(ValueVectorReadExpression e,
+        ClassGenerator<?> generator, JExpression recordIndex, JExpression vv1,
+        MajorType type) {
+
+      HoldingContainer out = generator.declare(type);
+
+      final boolean repeated = Types.isRepeated(type);
+      final boolean complex = Types.isComplex(type);
+      final boolean listVector = e.getTypedFieldId().isListVector();
+      final boolean isUnion = Types.isUnion(type);
+
+      // Create a reader for the vector.
+
+      JExpression expr = vv1.invoke("getReader");
+      PathSegment seg = e.getReadPath();
+
+      JVar isNull = null;
+      boolean isNullReaderLikely = isNullReaderLikely(seg, complex || repeated || listVector || isUnion);
+      if (isNullReaderLikely) {
+        isNull = generator.getEvalBlock().decl(generator.getModel().INT, generator.getNextVar("isNull"), JExpr.lit(0));
+      }
+
+      JLabel label = generator.getEvalBlock().label("complex");
+      JBlock eval = generator.getEvalBlock().block();
+
+      // Position to the correct value.
+      eval.add(expr.invoke("reset"));
+      eval.add(expr.invoke("setPosition").arg(recordIndex));
+      int listNum = 0;
+
+      // Variable to store index of key in dict (if there is one)
+      // if entry for the key is not found, will be assigned a value of SingleDictReaderImpl#NOT_FOUND.
+      JVar valueIndex = eval.decl(generator.getModel().INT, "valueIndex", JExpr.lit(-2));
+
+      int depth = 0;
+
+      while (seg != null) {
+        if (seg.isArray()) {
+
+          // stop once we get to the last segment and the final type is
+          // neither complex nor repeated (map, dict, list, repeated list).
+          // In case of non-complex and non-repeated type, we return Holder,
+          // in stead of FieldReader.
+          if (seg.isLastPath() && !complex && !repeated && !listVector) {
+            break;
+          }
+
+          if (e.getFieldId().isDict(depth)) {
+            expr = getDictReaderReadByKeyExpression(generator, eval, expr, seg, valueIndex, isNull);
+            seg = seg.getChild();
+            depth++;
+            continue;
+          }
+
+          JVar list = generator.declareClassField("list", generator.getModel()._ref(FieldReader.class));
+          eval.assign(list, expr);
+
+          // If this is an array, set a single position for the expression to
+          // allow us to read the right data lower down.
+          JVar desiredIndex = eval.decl(generator.getModel().INT, "desiredIndex" + listNum,
+              JExpr.lit(seg.getArraySegment().getIndex()));
+          // start with negative one so that we are at zero after first call
+          // to next.
+          JVar currentIndex = eval.decl(generator.getModel().INT, "currentIndex" + listNum, JExpr.lit(-1));
+
+          eval._while(
+              currentIndex.lt(desiredIndex)
+                  .cand(list.invoke("next"))).body().assign(currentIndex, currentIndex.plus(JExpr.lit(1)));
+
+          JBlock ifNoVal = eval._if(desiredIndex.ne(currentIndex))._then().block();
+          if (out.isOptional()) {
+            ifNoVal.assign(out.getIsSet(), JExpr.lit(0));
+          }
+          ifNoVal.assign(isNull, JExpr.lit(1));
+          ifNoVal._break(label);
+
+          expr = list.invoke("reader");
+          listNum++;
+        } else {
+
+          if (e.getFieldId().isDict(depth)) {
+            MajorType finalType = e.getFieldId().getFinalType();
+            if (seg.getChild() == null && !(Types.isComplex(finalType) || Types.isRepeated(finalType))) {
+              // This is the last segment:
+              eval.add(expr.invoke("read").arg(getKeyExpression(seg, generator)).arg(out.getHolder()));
+              return out;
+            }
+
+            expr = getDictReaderReadByKeyExpression(generator, eval, expr, seg, valueIndex, isNull);
+            seg = seg.getChild();
+            depth++;
+            continue;
+          }
+
+          JExpression fieldName = JExpr.lit(seg.getNameSegment().getPath());
+          expr = expr.invoke("reader").arg(fieldName);
+        }
+        seg = seg.getChild();
+        depth++;
+      }
+
+      // expected that after loop depth at least equal to last id index
+      depth = Math.max(depth, e.getFieldId().getFieldIds().length - 1);
+
+      if (complex || repeated) {
+
+        // Declare the reader for this vector
+        JVar complexReader = generator.declareClassField("reader", generator.getModel()._ref(FieldReader.class));
+
+        if (isNullReaderLikely) {
+          JConditional jc = generator.getEvalBlock()._if(isNull.eq(JExpr.lit(0)));
+
+          JClass nrClass = generator.getModel().ref(org.apache.drill.exec.vector.complex.impl.NullReader.class);
+          JExpression nullReader;
+          if (complex) {
+            nullReader = nrClass.staticRef("EMPTY_MAP_INSTANCE");
+          } else {
+            nullReader = nrClass.staticRef("EMPTY_LIST_INSTANCE");
+          }
+
+          jc._then().assign(complexReader, expr);
+          jc._else().assign(complexReader, nullReader);
+        } else {
+          eval.assign(complexReader, expr);
+        }
+
+        HoldingContainer hc = new HoldingContainer(type, complexReader, null, null, false, true);
+        return hc;
+      } else {
+
+        // For a DICT, create a holder, then obtain the reader.
+
+        if (seg != null) {
+          JExpression holderExpr = out.getHolder();
+          JExpression argExpr;
+          if (e.getFieldId().isDict(depth)) {
+            holderExpr = JExpr.cast(generator.getModel()._ref(ValueHolder.class), holderExpr);
+            argExpr = getKeyExpression(seg, generator);
+          } else {
+            argExpr = JExpr.lit(seg.getArraySegment().getIndex());
+          }
+          JClass dictReaderClass = generator.getModel().ref(org.apache.drill.exec.vector.complex.impl.SingleDictReaderImpl.class);
+          JConditional jc = eval._if(valueIndex.ne(dictReaderClass.staticRef("NOT_FOUND")));
+          jc._then().add(expr.invoke("read").arg(argExpr).arg(holderExpr));
+        } else {
+          eval.add(expr.invoke("read").arg(out.getHolder()));
+        }
+      }
       return out;
     }
 
     /*  Check if a Path expression could produce a NullReader. A path expression will produce a null reader, when:
      *   1) It contains an array segment as non-leaf segment :  a.b[2].c.  segment [2] might produce null reader.
      *   2) It contains an array segment as leaf segment, AND the final output is complex or repeated : a.b[2], when
-     *     the final type of this expression is a map, or releated list, or repeated map.
+     *     the final type of this expression is a map, or repeated list, or repeated map.
      */
     private boolean isNullReaderLikely(PathSegment seg, boolean complexOrRepeated) {
       while (seg != null) {
@@ -674,7 +797,7 @@
      * @return expression corresponding to {@link org.apache.drill.exec.vector.complex.DictVector#FIELD_VALUE_NAME}'s
      *         reader with its position set to index corresponding to the key
      */
-    private JExpression getDictReaderReadByKeyExpression(ClassGenerator generator, JBlock eval, JExpression expr,
+    private JExpression getDictReaderReadByKeyExpression(ClassGenerator<?> generator, JBlock eval, JExpression expr,
                                                          PathSegment segment, JVar valueIndex, JVar isNull) {
       JVar dictReader = generator.declareClassField("dictReader", generator.getModel()._ref(FieldReader.class));
       eval.assign(dictReader, expr);
@@ -712,7 +835,7 @@
      * @param generator current class generator
      * @return Java Object representation of key wrapped into {@link JVar}
      */
-    private JExpression getKeyExpression(PathSegment segment, ClassGenerator generator) {
+    private JExpression getKeyExpression(PathSegment segment, ClassGenerator<?> generator) {
       MajorType valueType = segment.getOriginalValueType();
       JType keyType;
       JExpression newKeyObject;
@@ -774,7 +897,7 @@
       }
     }
 
-    private JVar getDateTimeKey(PathSegment segment, ClassGenerator generator, Class<?> javaClass, String methodName) {
+    private JVar getDateTimeKey(PathSegment segment, ClassGenerator<?> generator, Class<?> javaClass, String methodName) {
       String strValue = (String) segment.getOriginalValue();
 
       JClass dateUtilityClass = generator.getModel().ref(org.apache.drill.exec.expr.fn.impl.DateUtility.class);
@@ -1014,9 +1137,14 @@
 
       return out;
     }
-
   }
 
+  /**
+   * Creates a representation of the "Holder" for a constant
+   * value. For optimization, constant holders are cached. This
+   * visitor retrieves an existing cached holder, if available, else
+   * creates a new one.
+   */
   private class CSEFilter extends ConstantFilter {
 
     public CSEFilter(Set<LogicalExpression> constantBoundaries) {
@@ -1282,14 +1410,16 @@
     }
   }
 
+  /**
+   * An evaluation visitor which special cases constant (sub-) expressions
+   * of any form, passing non-constant expressions to the parent
+   * visitor.
+   */
+  private static class ConstantFilter extends EvalVisitor {
 
-
-  private class ConstantFilter extends EvalVisitor {
-
-    private Set<LogicalExpression> constantBoundaries;
+    private final Set<LogicalExpression> constantBoundaries;
 
     public ConstantFilter(Set<LogicalExpression> constantBoundaries) {
-      super();
       this.constantBoundaries = constantBoundaries;
     }
 
@@ -1431,7 +1561,7 @@
       return visitExpression(e, generator, () -> super.visitIntervalDayConstant(e, generator));
     }
 
-    /*
+    /**
      * Get a HoldingContainer for a constant expression. The returned
      * HoldingContainer will indicate it's for a constant expression.
      */
@@ -1458,5 +1588,4 @@
           .setConstant(true);
     }
   }
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
index 946f3d8..17a2e0b 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
@@ -18,6 +18,7 @@
 package org.apache.drill.exec.expr;
 
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Deque;
@@ -92,7 +93,6 @@
 import org.slf4j.LoggerFactory;
 
 public class ExpressionTreeMaterializer {
-
   static final Logger logger = LoggerFactory.getLogger(ExpressionTreeMaterializer.class);
 
   private ExpressionTreeMaterializer() { }
@@ -195,7 +195,7 @@
   public static LogicalExpression convertToNullableType(LogicalExpression fromExpr,
       MinorType toType, FunctionLookupContext functionLookupContext, ErrorCollector errorCollector) {
     String funcName = "convertToNullable" + toType.toString();
-    List<LogicalExpression> args = Lists.newArrayList();
+    List<LogicalExpression> args = new ArrayList<>();
     args.add(fromExpr);
     FunctionCall funcCall = new FunctionCall(funcName, args, ExpressionPosition.UNKNOWN);
     FunctionResolver resolver = FunctionResolverFactory.getResolver(funcCall);
@@ -217,7 +217,7 @@
   public static LogicalExpression addCastExpression(LogicalExpression fromExpr, MajorType toType,
       FunctionLookupContext functionLookupContext, ErrorCollector errorCollector, boolean exactResolver) {
     String castFuncName = FunctionReplacementUtils.getCastFunc(toType.getMinorType());
-    List<LogicalExpression> castArgs = Lists.newArrayList();
+    List<LogicalExpression> castArgs = new ArrayList<>();
     castArgs.add(fromExpr);  //input_expr
 
     if (fromExpr.getMajorType().getMinorType() == MinorType.UNION && toType.getMinorType() == MinorType.UNION) {
@@ -380,7 +380,7 @@
     @Override
     public LogicalExpression visitFunctionHolderExpression(FunctionHolderExpression holder,
         FunctionLookupContext functionLookupContext) {
-      // a function holder is already materialized, no need to rematerialize.
+      // A function holder is already materialized, no need to rematerialize.
       // generally this won't be used unless we materialize a partial tree and
       // rematerialize the whole tree.
       return holder;
@@ -388,14 +388,14 @@
 
     @Override
     public LogicalExpression visitBooleanOperator(BooleanOperator op, FunctionLookupContext functionLookupContext) {
-      List<LogicalExpression> args = Lists.newArrayList();
+      List<LogicalExpression> args = new ArrayList<>();
       for (int i = 0; i < op.args.size(); ++i) {
         LogicalExpression newExpr = op.args.get(i).accept(this, functionLookupContext);
         assert newExpr != null : String.format("Materialization of %s return a null expression.", op.args.get(i));
         args.add(newExpr);
       }
 
-      //replace with a new function call, since its argument could be changed.
+      // Replace with a new function call, since its argument could be changed.
       return new BooleanOperator(op.getName(), args, op.getPosition());
     }
 
@@ -406,16 +406,19 @@
 
     @Override
     public LogicalExpression visitFunctionCall(FunctionCall call, FunctionLookupContext functionLookupContext) {
-      List<LogicalExpression> args = Lists.newArrayList();
+
+      // Possibly convert input expressions with a rewritten expression.
+      List<LogicalExpression> args = new ArrayList<>();
       for (int i = 0; i < call.args.size(); ++i) {
         LogicalExpression newExpr = call.args.get(i).accept(this, functionLookupContext);
         assert newExpr != null : String.format("Materialization of %s returned a null expression.", call.args.get(i));
         args.add(newExpr);
       }
 
-      //replace with a new function call, since its argument could be changed.
+      // Replace with a new function call, since its argument could be changed.
       call = new FunctionCall(call.getName(), args, call.getPosition());
 
+      // Resolve the function
       FunctionResolver resolver = FunctionResolverFactory.getResolver(call);
       DrillFuncHolder matchedFuncHolder = functionLookupContext.findDrillFunction(resolver, call);
 
@@ -425,91 +428,121 @@
                 + call.getName() + " in a non-project operation!");
       }
 
-      //new arg lists, possible with implicit cast inserted.
-      List<LogicalExpression> argsWithCast = Lists.newArrayList();
-
       if (matchedFuncHolder != null) {
-        // Compare param type against arg type. Insert cast on top of arg, whenever necessary.
-        for (int i = 0; i < call.args.size(); ++i) {
-
-          LogicalExpression currentArg = call.args.get(i);
-
-          TypeProtos.MajorType parmType = matchedFuncHolder.getParamMajorType(i);
-
-          // Case 1: If  1) the argument is NullExpression
-          //             2) the minor type of parameter of matchedFuncHolder is not LATE (the type of null expression is still unknown)
-          //             3) the parameter of matchedFuncHolder allows null input, or func's null_handling
-          //                is NULL_IF_NULL (means null and non-null are exchangeable).
-          //         then replace NullExpression with a TypedNullConstant
-          if (currentArg.equals(NullExpression.INSTANCE) && !MinorType.LATE.equals(parmType.getMinorType()) &&
-              (TypeProtos.DataMode.OPTIONAL.equals(parmType.getMode()) ||
-              matchedFuncHolder.getNullHandling() == FunctionTemplate.NullHandling.NULL_IF_NULL)) {
-            argsWithCast.add(new TypedNullConstant(parmType));
-          } else if (Types.softEquals(parmType, currentArg.getMajorType(),
-              matchedFuncHolder.getNullHandling() == FunctionTemplate.NullHandling.NULL_IF_NULL) ||
-                     matchedFuncHolder.isFieldReader(i)) {
-            // Case 2: argument and parameter matches, or parameter is FieldReader.  Do nothing.
-            argsWithCast.add(currentArg);
-          } else {
-            // Case 3: insert cast if param type is different from arg type.
-            if (Types.isDecimalType(parmType)) {
-              // We are implicitly promoting a decimal type, set the required scale and precision
-              parmType = MajorType.newBuilder().setMinorType(parmType.getMinorType()).setMode(parmType.getMode()).
-                  setScale(currentArg.getMajorType().getScale()).setPrecision(computePrecision(currentArg)).build();
-            }
-            argsWithCast.add(addCastExpression(currentArg, parmType, functionLookupContext, errorCollector));
-          }
-        }
-
-        FunctionHolderExpression funcExpr = matchedFuncHolder.getExpr(call.getName(), argsWithCast, call.getPosition());
-        MajorType funcExprMajorType = funcExpr.getMajorType();
-        if (DecimalUtility.isObsoleteDecimalType(funcExprMajorType.getMinorType())) {
-          MajorType majorType =
-              MajorType.newBuilder()
-                  .setMinorType(MinorType.VARDECIMAL)
-                  .setMode(funcExprMajorType.getMode())
-                  .setScale(funcExprMajorType.getScale())
-                  .setPrecision(funcExprMajorType.getPrecision())
-                  .build();
-          return addCastExpression(funcExpr, majorType, functionLookupContext, errorCollector);
-        }
-        return funcExpr;
+        return bindDrillFunc(call, functionLookupContext, matchedFuncHolder);
       }
 
       // as no drill func is found, search for a non-Drill function.
       AbstractFuncHolder matchedNonDrillFuncHolder = functionLookupContext.findNonDrillFunction(call);
       if (matchedNonDrillFuncHolder != null) {
-        // Insert implicit cast function holder expressions if required
-        List<LogicalExpression> extArgsWithCast = Lists.newArrayList();
-
-        for (int i = 0; i < call.args.size(); ++i) {
-          LogicalExpression currentArg = call.args.get(i);
-          TypeProtos.MajorType paramType = matchedNonDrillFuncHolder.getParamMajorType(i);
-
-          if (Types.softEquals(paramType, currentArg.getMajorType(), true)) {
-            extArgsWithCast.add(currentArg);
-          } else {
-            // Insert cast if param type is different from arg type.
-            if (Types.isDecimalType(paramType)) {
-              // We are implicitly promoting a decimal type, set the required scale and precision
-              paramType = MajorType.newBuilder().setMinorType(paramType.getMinorType()).setMode(paramType.getMode()).
-                  setScale(currentArg.getMajorType().getScale()).setPrecision(computePrecision(currentArg)).build();
-            }
-            extArgsWithCast.add(addCastExpression(call.args.get(i), paramType, functionLookupContext, errorCollector));
-          }
-        }
-
-        return matchedNonDrillFuncHolder.getExpr(call.getName(), extArgsWithCast, call.getPosition());
+        return bindNonDrillFunc(call, functionLookupContext,
+            matchedNonDrillFuncHolder);
       }
 
       if (hasUnionInput(call)) {
         return rewriteUnionFunction(call, functionLookupContext);
       }
 
+      // No match found. Add an error (which will fail the query), but also
+      // return a NULL instance so that analysis can continue.
       logFunctionResolutionError(errorCollector, call);
       return NullExpression.INSTANCE;
     }
 
+    /**
+     * Bind a call to a Drill function, casting input and output arguments
+     * as needed. Casts done here are of SQL types, not internal Drill types
+     * (FieldReader vs. Holder).
+     *
+     * @param call the logical call expression
+     * @param functionLookupContext function registry
+     * @param matchedFuncHolder the matched Drill function declaration
+     * @return a new expression that represents the actual function call
+     */
+    private LogicalExpression bindDrillFunc(FunctionCall call,
+        FunctionLookupContext functionLookupContext,
+        DrillFuncHolder matchedFuncHolder) {
+
+      // New arg lists, possible with implicit cast inserted.
+      List<LogicalExpression> argsWithCast = new ArrayList<>();
+
+      // Compare param type against arg type. Insert cast on top of arg, whenever necessary.
+      for (int i = 0; i < call.args.size(); ++i) {
+
+        LogicalExpression currentArg = call.args.get(i);
+
+        TypeProtos.MajorType parmType = matchedFuncHolder.getParamMajorType(i);
+
+        // Case 1: If  1) the argument is NullExpression
+        //             2) the minor type of parameter of matchedFuncHolder is not LATE
+        //                (the type of null expression is still unknown)
+        //             3) the parameter of matchedFuncHolder allows null input, or func's null_handling
+        //                is NULL_IF_NULL (means null and non-null are exchangeable).
+        //         then replace NullExpression with a TypedNullConstant
+        if (currentArg.equals(NullExpression.INSTANCE) && !MinorType.LATE.equals(parmType.getMinorType()) &&
+            (TypeProtos.DataMode.OPTIONAL.equals(parmType.getMode()) ||
+            matchedFuncHolder.getNullHandling() == FunctionTemplate.NullHandling.NULL_IF_NULL)) {
+          // Case 1: argument is a null expression, convert it to a typed null
+          argsWithCast.add(new TypedNullConstant(parmType));
+        } else if (Types.softEquals(parmType, currentArg.getMajorType(),
+            matchedFuncHolder.getNullHandling() == FunctionTemplate.NullHandling.NULL_IF_NULL) ||
+                   matchedFuncHolder.isFieldReader(i)) {
+          // Case 2: argument and parameter matches, or parameter is FieldReader.  Do nothing.
+          argsWithCast.add(currentArg);
+        } else {
+          // Case 3: insert cast if param type is different from arg type.
+          if (Types.isDecimalType(parmType)) {
+            // We are implicitly promoting a decimal type, set the required scale and precision
+            parmType = MajorType.newBuilder().setMinorType(parmType.getMinorType()).setMode(parmType.getMode()).
+                setScale(currentArg.getMajorType().getScale()).setPrecision(computePrecision(currentArg)).build();
+          }
+          argsWithCast.add(addCastExpression(currentArg, parmType, functionLookupContext, errorCollector));
+        }
+      }
+
+      FunctionHolderExpression funcExpr = matchedFuncHolder.getExpr(call.getName(), argsWithCast, call.getPosition());
+
+      // Convert old-style Decimal return type to VarDecimal
+      MajorType funcExprMajorType = funcExpr.getMajorType();
+      if (DecimalUtility.isObsoleteDecimalType(funcExprMajorType.getMinorType())) {
+        MajorType majorType =
+            MajorType.newBuilder()
+                .setMinorType(MinorType.VARDECIMAL)
+                .setMode(funcExprMajorType.getMode())
+                .setScale(funcExprMajorType.getScale())
+                .setPrecision(funcExprMajorType.getPrecision())
+                .build();
+        return addCastExpression(funcExpr, majorType, functionLookupContext, errorCollector);
+      }
+      return funcExpr;
+    }
+
+    private LogicalExpression bindNonDrillFunc(FunctionCall call,
+        FunctionLookupContext functionLookupContext,
+        AbstractFuncHolder matchedNonDrillFuncHolder) {
+      // Insert implicit cast function holder expressions if required
+      List<LogicalExpression> extArgsWithCast = new ArrayList<>();
+
+      for (int i = 0; i < call.args.size(); ++i) {
+        LogicalExpression currentArg = call.args.get(i);
+        TypeProtos.MajorType paramType = matchedNonDrillFuncHolder.getParamMajorType(i);
+
+        if (Types.softEquals(paramType, currentArg.getMajorType(), true)) {
+          extArgsWithCast.add(currentArg);
+        } else {
+          // Insert cast if param type is different from arg type.
+          if (Types.isDecimalType(paramType)) {
+            // We are implicitly promoting a decimal type, set the required scale and precision
+            paramType = MajorType.newBuilder().setMinorType(paramType.getMinorType()).setMode(paramType.getMode()).
+                setScale(currentArg.getMajorType().getScale()).setPrecision(computePrecision(currentArg)).build();
+          }
+          extArgsWithCast.add(addCastExpression(call.args.get(i), paramType, functionLookupContext, errorCollector));
+        }
+      }
+
+      return matchedNonDrillFuncHolder.getExpr(call.getName(), extArgsWithCast, call.getPosition());
+    }
+
     private boolean hasUnionInput(FunctionCall call) {
       for (LogicalExpression arg : call.args) {
         if (arg.getMajorType().getMinorType() == MinorType.UNION) {
@@ -547,7 +580,7 @@
           LogicalExpression ifCondition = getIsTypeExpressionForType(minorType, arg.accept(new CloneVisitor(), null));
           args[i] = getUnionAssertFunctionForType(minorType, arg.accept(new CloneVisitor(), null));
 
-          List<LogicalExpression> newArgs = Lists.newArrayList();
+          List<LogicalExpression> newArgs = new ArrayList<>();
           for (LogicalExpression e : args) {
             newArgs.add(e.accept(new CloneVisitor(), null));
           }
@@ -555,10 +588,9 @@
           // When expanding the expression tree to handle the different
           // subtypes, we will not throw an exception if one
           // of the branches fails to find a function match, since it is
-          // possible that code path will never occur in execution
+          // possible that code path will never occur in execution.
           // So instead of failing to materialize, we generate code to throw the
-          // exception during execution if that code
-          // path is hit.
+          // exception during execution if that code path is hit.
 
           errorCollectors.push(errorCollector);
           errorCollector = new ErrorCollectorImpl();
@@ -594,7 +626,7 @@
      */
     private LogicalExpression getExceptionFunction(String message) {
       QuotedString msg = new QuotedString(message, message.length(), ExpressionPosition.UNKNOWN);
-      List<LogicalExpression> args = Lists.newArrayList();
+      List<LogicalExpression> args = new ArrayList<>();
       args.add(msg);
       return new FunctionCall(ExceptionFunction.EXCEPTION_FUNCTION_NAME, args, ExpressionPosition.UNKNOWN);
     }
@@ -625,7 +657,7 @@
      */
     private LogicalExpression getIsTypeExpressionForType(MinorType type, LogicalExpression arg) {
       String isFuncName = String.format("is_%s", type.toString());
-      List<LogicalExpression> args = Lists.newArrayList();
+      List<LogicalExpression> args = new ArrayList<>();
       args.add(arg);
       return new FunctionCall(isFuncName, args, ExpressionPosition.UNKNOWN);
     }
@@ -690,7 +722,7 @@
 
       // Resolve NullExpression into TypedNullConstant by visiting all conditions
       // We need to do this because we want to give the correct MajorType to the Null constant
-      List<LogicalExpression> allExpressions = Lists.newArrayList();
+      List<LogicalExpression> allExpressions = new ArrayList<>();
       allExpressions.add(conditions.expression);
       allExpressions.add(newElseExpr);
 
@@ -860,7 +892,7 @@
     public LogicalExpression visitConvertExpression(ConvertExpression e, FunctionLookupContext functionLookupContext) {
       String convertFunctionName = e.getConvertFunction() + e.getEncodingType();
 
-      List<LogicalExpression> newArgs = Lists.newArrayList();
+      List<LogicalExpression> newArgs = new ArrayList<>();
       newArgs.add(e.getInput());  //input_expr
 
       FunctionCall fc = new FunctionCall(convertFunctionName, newArgs, e.getPosition());
@@ -894,7 +926,7 @@
         // Get the cast function name from the map
         String castFuncWithType = FunctionReplacementUtils.getCastFunc(type.getMinorType());
 
-        List<LogicalExpression> newArgs = Lists.newArrayList();
+        List<LogicalExpression> newArgs = new ArrayList<>();
         newArgs.add(input);  //input_expr
 
         if (Types.isDecimalType(type)) {
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/GetSetVectorHelper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/GetSetVectorHelper.java
index 3b7e2b4..4cf02be 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/GetSetVectorHelper.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/GetSetVectorHelper.java
@@ -31,89 +31,98 @@
 import com.sun.codemodel.JVar;
 
 public class GetSetVectorHelper {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(GetSetVectorHelper.class);
 
-  // eval.add(getValueAccessor.arg(indexVariable).arg(out.getHolder()));
-
+  /**
+   * Generates the code to read a vector into a holder. Example: <pre><code>
+   *   NullableBigIntHolder out3 = new NullableBigIntHolder();
+   *   {
+   *    out3 .isSet = vv0 [((leftIndex)>>> 16)].getAccessor().isSet(((leftIndex)& 65535));
+   *    if (out3 .isSet == 1) {
+   *      out3 .value = vv0 [((leftIndex)>>> 16)].getAccessor().get(((leftIndex)& 65535));
+   *    }
+   *  }</code></pre>
+   */
   public static void read(MajorType type, JExpression vector, JBlock eval, HoldingContainer out, JCodeModel model,
       JExpression indexVariable) {
 
     JInvocation getValueAccessor = vector.invoke("getAccessor");
 
     switch(type.getMode()){
-    case OPTIONAL:
-      eval.assign(out.getIsSet(), getValueAccessor.invoke("isSet").arg(indexVariable));
-      eval = eval._if(out.getIsSet().eq(JExpr.lit(1)))._then();
+      case OPTIONAL:
+        eval.assign(out.getIsSet(), getValueAccessor.invoke("isSet").arg(indexVariable));
+        eval = eval._if(out.getIsSet().eq(JExpr.lit(1)))._then();
+        // fall through
 
-    // fall through
-    case REQUIRED:
-      switch (type.getMinorType()) {
-      case BIGINT:
-      case FLOAT4:
-      case FLOAT8:
-      case INT:
-      case MONEY:
-      case SMALLINT:
-      case TINYINT:
-      case UINT1:
-      case UINT2:
-      case UINT4:
-      case UINT8:
-      case INTERVALYEAR:
-      case DATE:
-      case TIME:
-      case TIMESTAMP:
-      case BIT:
-        eval.assign(out.getValue(), getValueAccessor.invoke("get").arg(indexVariable));
-        return;
-      case DECIMAL9:
-      case DECIMAL18:
-        eval.assign(out.getHolder().ref("scale"), vector.invoke("getField").invoke("getScale"));
-        eval.assign(out.getHolder().ref("precision"), vector.invoke("getField").invoke("getPrecision"));
-        eval.assign(out.getValue(), getValueAccessor.invoke("get").arg(indexVariable));
-        return;
-      case DECIMAL28DENSE:
-      case DECIMAL28SPARSE:
-      case DECIMAL38DENSE:
-      case DECIMAL38SPARSE:
-        eval.assign(out.getHolder().ref("scale"), vector.invoke("getField").invoke("getScale"));
-        eval.assign(out.getHolder().ref("precision"), vector.invoke("getField").invoke("getPrecision"));
-        eval.assign(out.getHolder().ref("start"), JExpr.lit(TypeHelper.getSize(type)).mul(indexVariable));
-        eval.assign(out.getHolder().ref("buffer"), vector.invoke("getBuffer"));
-        return;
-      case VARDECIMAL: {
-        eval.assign(out.getHolder().ref("buffer"), vector.invoke("getBuffer"));
-        JVar se = eval.decl(model.LONG, "startEnd", getValueAccessor.invoke("getStartEnd").arg(indexVariable));
-        eval.assign(out.getHolder().ref("start"), JExpr.cast(model._ref(int.class), se));
-        eval.assign(out.getHolder().ref("end"), JExpr.cast(model._ref(int.class), se.shr(JExpr.lit(32))));
-        eval.assign(out.getHolder().ref("scale"), vector.invoke("getField").invoke("getScale"));
-        eval.assign(out.getHolder().ref("precision"), vector.invoke("getField").invoke("getPrecision"));
-        return;
-      }
-      case INTERVAL: {
-        JVar start = eval.decl(model.INT, "start", JExpr.lit(TypeHelper.getSize(type)).mul(indexVariable));
-        JVar data = eval.decl(model.ref(DrillBuf.class), "data", vector.invoke("getBuffer"));
-        eval.assign(out.getHolder().ref("months"), data.invoke("getInt").arg(start));
-        eval.assign(out.getHolder().ref("days"), data.invoke("getInt").arg(start.plus(JExpr.lit(4))));
-        eval.assign(out.getHolder().ref("milliseconds"), data.invoke("getInt").arg(start.plus(JExpr.lit(8))));
-        return;
-      }
-      case INTERVALDAY: {
-        JVar start = eval.decl(model.INT, "start", JExpr.lit(TypeHelper.getSize(type)).mul(indexVariable));
-        eval.assign(out.getHolder().ref("days"), vector.invoke("getBuffer").invoke("getInt").arg(start));
-        eval.assign(out.getHolder().ref("milliseconds"), vector.invoke("getBuffer").invoke("getInt").arg(start.plus(JExpr.lit(4))));
-        return;
-      }
-      case VAR16CHAR:
-      case VARBINARY:
-      case VARCHAR: {
-         eval.assign(out.getHolder().ref("buffer"), vector.invoke("getBuffer"));
-         JVar se = eval.decl(model.LONG, "startEnd", getValueAccessor.invoke("getStartEnd").arg(indexVariable));
-         eval.assign(out.getHolder().ref("start"), JExpr.cast(model._ref(int.class), se));
-         eval.assign(out.getHolder().ref("end"), JExpr.cast(model._ref(int.class), se.shr(JExpr.lit(32))));
-        return;
-      }
-      }
+      case REQUIRED:
+        switch (type.getMinorType()) {
+          case BIGINT:
+          case FLOAT4:
+          case FLOAT8:
+          case INT:
+          case MONEY:
+          case SMALLINT:
+          case TINYINT:
+          case UINT1:
+          case UINT2:
+          case UINT4:
+          case UINT8:
+          case INTERVALYEAR:
+          case DATE:
+          case TIME:
+          case TIMESTAMP:
+          case BIT:
+            eval.assign(out.getValue(), getValueAccessor.invoke("get").arg(indexVariable));
+            return;
+          case DECIMAL9:
+          case DECIMAL18:
+            eval.assign(out.getHolder().ref("scale"), vector.invoke("getField").invoke("getScale"));
+            eval.assign(out.getHolder().ref("precision"), vector.invoke("getField").invoke("getPrecision"));
+            eval.assign(out.getValue(), getValueAccessor.invoke("get").arg(indexVariable));
+            return;
+          case DECIMAL28DENSE:
+          case DECIMAL28SPARSE:
+          case DECIMAL38DENSE:
+          case DECIMAL38SPARSE:
+            eval.assign(out.getHolder().ref("scale"), vector.invoke("getField").invoke("getScale"));
+            eval.assign(out.getHolder().ref("precision"), vector.invoke("getField").invoke("getPrecision"));
+            eval.assign(out.getHolder().ref("start"), JExpr.lit(TypeHelper.getSize(type)).mul(indexVariable));
+            eval.assign(out.getHolder().ref("buffer"), vector.invoke("getBuffer"));
+            return;
+          case VARDECIMAL: {
+            eval.assign(out.getHolder().ref("buffer"), vector.invoke("getBuffer"));
+            JVar se = eval.decl(model.LONG, "startEnd", getValueAccessor.invoke("getStartEnd").arg(indexVariable));
+            eval.assign(out.getHolder().ref("start"), JExpr.cast(model._ref(int.class), se));
+            eval.assign(out.getHolder().ref("end"), JExpr.cast(model._ref(int.class), se.shr(JExpr.lit(32))));
+            eval.assign(out.getHolder().ref("scale"), vector.invoke("getField").invoke("getScale"));
+            eval.assign(out.getHolder().ref("precision"), vector.invoke("getField").invoke("getPrecision"));
+            return;
+          }
+          case INTERVAL: {
+            JVar start = eval.decl(model.INT, "start", JExpr.lit(TypeHelper.getSize(type)).mul(indexVariable));
+            JVar data = eval.decl(model.ref(DrillBuf.class), "data", vector.invoke("getBuffer"));
+            eval.assign(out.getHolder().ref("months"), data.invoke("getInt").arg(start));
+            eval.assign(out.getHolder().ref("days"), data.invoke("getInt").arg(start.plus(JExpr.lit(4))));
+            eval.assign(out.getHolder().ref("milliseconds"), data.invoke("getInt").arg(start.plus(JExpr.lit(8))));
+            return;
+          }
+          case INTERVALDAY: {
+            JVar start = eval.decl(model.INT, "start", JExpr.lit(TypeHelper.getSize(type)).mul(indexVariable));
+            eval.assign(out.getHolder().ref("days"), vector.invoke("getBuffer").invoke("getInt").arg(start));
+            eval.assign(out.getHolder().ref("milliseconds"), vector.invoke("getBuffer").invoke("getInt").arg(start.plus(JExpr.lit(4))));
+            return;
+          }
+          case VAR16CHAR:
+          case VARBINARY:
+          case VARCHAR: {
+             eval.assign(out.getHolder().ref("buffer"), vector.invoke("getBuffer"));
+             JVar se = eval.decl(model.LONG, "startEnd", getValueAccessor.invoke("getStartEnd").arg(indexVariable));
+             eval.assign(out.getHolder().ref("start"), JExpr.cast(model._ref(int.class), se));
+             eval.assign(out.getHolder().ref("end"), JExpr.cast(model._ref(int.class), se.shr(JExpr.lit(32))));
+            return;
+          }
+          default:
+        }
+      default:
     }
 
     // fallback.
@@ -128,61 +137,63 @@
       return setMethod.arg(in.getHolder());
     }
     switch(type.getMode()){
-    case OPTIONAL:
-      setMethod = setMethod.arg(in.f("isSet"));
-    case REQUIRED:
-      switch (type.getMinorType()) {
-      case BIGINT:
-      case FLOAT4:
-      case FLOAT8:
-      case INT:
-      case MONEY:
-      case SMALLINT:
-      case TINYINT:
-      case UINT1:
-      case UINT2:
-      case UINT4:
-      case UINT8:
-      case INTERVALYEAR:
-      case DATE:
-      case TIME:
-      case TIMESTAMP:
-      case BIT:
-      case DECIMAL9:
-      case DECIMAL18:
-        return setMethod
-            .arg(in.getValue());
-      case DECIMAL28DENSE:
-      case DECIMAL28SPARSE:
-      case DECIMAL38DENSE:
-      case DECIMAL38SPARSE:
-        return setMethod
-            .arg(in.f("start"))
-            .arg(in.f("buffer"));
-      case INTERVAL:{
-        return setMethod
-            .arg(in.f("months"))
-            .arg(in.f("days"))
-            .arg(in.f("milliseconds"));
-      }
-      case INTERVALDAY: {
-        return setMethod
-            .arg(in.f("days"))
-            .arg(in.f("milliseconds"));
-      }
-      case VAR16CHAR:
-      case VARBINARY:
-      case VARCHAR:
-      case VARDECIMAL:
-        return setMethod
-            .arg(in.f("start"))
-            .arg(in.f("end"))
-            .arg(in.f("buffer"));
-      }
+      case OPTIONAL:
+        setMethod = setMethod.arg(in.f("isSet"));
+        // Fall through
+
+      case REQUIRED:
+        switch (type.getMinorType()) {
+          case BIGINT:
+          case FLOAT4:
+          case FLOAT8:
+          case INT:
+          case MONEY:
+          case SMALLINT:
+          case TINYINT:
+          case UINT1:
+          case UINT2:
+          case UINT4:
+          case UINT8:
+          case INTERVALYEAR:
+          case DATE:
+          case TIME:
+          case TIMESTAMP:
+          case BIT:
+          case DECIMAL9:
+          case DECIMAL18:
+            return setMethod
+                .arg(in.getValue());
+          case DECIMAL28DENSE:
+          case DECIMAL28SPARSE:
+          case DECIMAL38DENSE:
+          case DECIMAL38SPARSE:
+            return setMethod
+                .arg(in.f("start"))
+                .arg(in.f("buffer"));
+          case INTERVAL:{
+            return setMethod
+                .arg(in.f("months"))
+                .arg(in.f("days"))
+                .arg(in.f("milliseconds"));
+          }
+          case INTERVALDAY: {
+            return setMethod
+                .arg(in.f("days"))
+                .arg(in.f("milliseconds"));
+          }
+          case VAR16CHAR:
+          case VARBINARY:
+          case VARCHAR:
+          case VARDECIMAL:
+            return setMethod
+                .arg(in.f("start"))
+                .arg(in.f("end"))
+                .arg(in.f("buffer"));
+          default:
+        }
+      default:
     }
 
     return setMethod.arg(in.getHolder());
-
   }
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/HoldingContainerExpression.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/HoldingContainerExpression.java
index ccabb8a..e18fb3e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/HoldingContainerExpression.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/HoldingContainerExpression.java
@@ -25,9 +25,21 @@
 import org.apache.drill.common.expression.visitors.ExprVisitor;
 import org.apache.drill.common.types.TypeProtos.MajorType;
 import org.apache.drill.exec.expr.ClassGenerator.HoldingContainer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+/**
+ * Wrapper around a representation of a "Holder" to represent that
+ * Holder as an expression. Whether this expression represents a
+ * declaration, initialization, or reference (use) of the holder
+ * depends on the visitor applied to this expression.
+ * <p>
+ * This expression class is a run-time implementation detail; it
+ * shows up in the "unknown" bucket within the visitor, where it
+ * must be parsed out using <code>instanceof</code> (or assumed.)
+ */
 public class HoldingContainerExpression implements LogicalExpression{
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(HoldingContainerExpression.class);
+  static final Logger logger = LoggerFactory.getLogger(HoldingContainerExpression.class);
 
   final HoldingContainer container;
 
@@ -50,7 +62,6 @@
     return visitor.visitUnknown(this, value);
   }
 
-
   public HoldingContainer getContainer() {
     return container;
   }
@@ -70,4 +81,8 @@
     return 0; // TODO
   }
 
+  @Override
+  public String toString() {
+    return container.toString();
+  }
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/IsPredicate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/IsPredicate.java
index def1d3c..1f2369e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/IsPredicate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/IsPredicate.java
@@ -27,6 +27,8 @@
 import org.apache.drill.metastore.statistics.ColumnStatisticsKind;
 import org.apache.drill.metastore.statistics.Statistic;
 import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Iterator;
@@ -34,6 +36,7 @@
 import java.util.function.BiFunction;
 
 public class IsPredicate<C extends Comparable<C>> extends LogicalExpressionBase implements FilterPredicate<C> {
+  private static final Logger logger = LoggerFactory.getLogger(IsPredicate.class);
 
   private final LogicalExpression expr;
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ValueVectorReadExpression.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ValueVectorReadExpression.java
index 4f6c341..0c18b02 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ValueVectorReadExpression.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ValueVectorReadExpression.java
@@ -18,6 +18,8 @@
 package org.apache.drill.exec.expr;
 
 import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.apache.drill.common.expression.ExpressionPosition;
 import org.apache.drill.common.expression.LogicalExpression;
 import org.apache.drill.common.expression.PathSegment;
@@ -29,11 +31,12 @@
 
 /**
  * Wraps a value vector field to be read, providing metadata about the field.
- * Also may contain batch naming information to which this field belongs.
- * If such information is absent default namings will be used from mapping set during materialization.
+ * Also may contain batch naming information to which this field belongs. If
+ * such information is absent default naming will be used from mapping set
+ * during materialization.
  */
 public class ValueVectorReadExpression implements LogicalExpression {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ValueVectorReadExpression.class);
+  static final Logger logger = LoggerFactory.getLogger(ValueVectorReadExpression.class);
 
   private final TypedFieldId fieldId;
   private final BatchReference batchRef;
@@ -108,5 +111,4 @@
   public String toString() {
     return "ValueVectorReadExpression [fieldId=" + fieldId + "]";
   }
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ValueVectorWriteExpression.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ValueVectorWriteExpression.java
index 3658bf2..5dbbdc0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ValueVectorWriteExpression.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ValueVectorWriteExpression.java
@@ -28,8 +28,6 @@
 import java.util.Iterator;
 
 public class ValueVectorWriteExpression implements LogicalExpression {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ValueVectorWriteExpression.class);
-
   private final TypedFieldId fieldId;
   private final LogicalExpression child;
   private final boolean safe;
@@ -53,7 +51,6 @@
     return Types.NULL;
   }
 
-
   public boolean isSafe() {
     return safe;
   }
@@ -90,6 +87,4 @@
   public int getCumulativeCost() {
     return 0; // TODO
   }
-
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java
index def41b7..9df9b8e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java
@@ -23,7 +23,8 @@
 import java.lang.annotation.Target;
 
 /**
- * Marker annotation to determine which fields should be included as parameters for the function.
+ * Marker annotation to determine which fields should be included as parameters
+ * for the function.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.FIELD})
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
index 6203cd8..9c11c89 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
@@ -213,7 +213,7 @@
         declareVarArgArray(g.getModel(), sub, inputVariables);
       }
       for (int i = 0; i < inputVariables.length; i++) {
-        declare(g.getModel(), sub, inputVariables[i], i);
+        declareInputVariable(g.getModel(), sub, inputVariables[i], i);
       }
     }
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
index 91f7cea..76e1e16 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
@@ -35,9 +35,11 @@
 import org.apache.drill.exec.expr.ClassGenerator.BlockType;
 import org.apache.drill.exec.expr.ClassGenerator.HoldingContainer;
 import org.apache.drill.exec.expr.DrillFuncHolderExpr;
+import org.apache.drill.exec.expr.EvaluationVisitor.VectorVariableHolder;
 import org.apache.drill.exec.expr.TypeHelper;
 import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
 import org.apache.drill.exec.expr.fn.output.OutputWidthCalculator;
+import org.apache.drill.exec.expr.holders.UnionHolder;
 import org.apache.drill.exec.expr.holders.ValueHolder;
 import org.apache.drill.exec.ops.UdfUtilities;
 import org.apache.drill.exec.vector.complex.reader.FieldReader;
@@ -56,8 +58,7 @@
 import com.sun.codemodel.JVar;
 
 public abstract class DrillFuncHolder extends AbstractFuncHolder {
-
-  static final Logger logger = LoggerFactory.getLogger(DrillFuncHolder.class);
+  private static final Logger logger = LoggerFactory.getLogger(DrillFuncHolder.class);
 
   private final FunctionAttributes attributes;
   private final FunctionInitializer initializer;
@@ -73,11 +74,17 @@
 
   /**
    * Check if function type supports provided null handling strategy.
-   * <p>Keep in mind that this method is invoked in {@link #DrillFuncHolder(FunctionAttributes, FunctionInitializer)}
-   * constructor so make sure not to use any state fields when overriding the method to avoid uninitialized state.</p>
+   * <p>
+   * Keep in mind that this method is invoked in
+   * {@link #DrillFuncHolder(FunctionAttributes, FunctionInitializer)}
+   * constructor so make sure not to use any state fields when overriding the
+   * method to avoid uninitialized state.
+   * </p>
    *
-   * @param nullHandling null handling strategy defined for a function
-   * @throws IllegalArgumentException if provided {@code nullHandling} is not supported
+   * @param nullHandling
+   *          null handling strategy defined for a function
+   * @throws IllegalArgumentException
+   *           if provided {@code nullHandling} is not supported
    */
   protected void checkNullHandling(NullHandling nullHandling) {
   }
@@ -184,57 +191,88 @@
               ));
         } else {
           // Invalid injectable type provided, this should have been caught in FunctionConverter
-          throw new DrillRuntimeException("Invalid injectable type requested in UDF: " + ref.getType().getSimpleName());
+          throw new DrillRuntimeException("Invalid injectable type requested in UDF: " +
+                ref.getType().getSimpleName());
         }
-      } else {
-        //g.getBlock(BlockType.SETUP).assign(workspaceJVars[i], JExpr._new(jtype));
       }
     }
     return workspaceJVars;
   }
 
+  /**
+   * Generate the body of a Drill function by copying the source code of the
+   * corresponding function method into the generated output. For this to work,
+   * all symbol references must be absolute (they cannot refer to imports),
+   * or they must refer to local variables or class fields marked with an
+   * annotation.
+   * <p>
+   * To make this work, the function body is wrapped in a code block that
+   * simulates the class fields by declaring local variables of the same
+   * name, and assigning those variables based on the input and workspace
+   * variables provided.
+   * <p>
+   * This version is used for blocks other than the main eval block.
+   *
+   * @param g code generator
+   * @param bt type of the block to be added
+   * @param body source code of the block. Optional. Block will be omitted
+   * if the method body is null or empty
+   * @param inputVariables list of input variable bindings which match up to declared
+   * <code>@Param</code> member variables in order. The input variables have the
+   * same name as the parameters that they represent
+   * @param workspaceJVars list of workspace variables, structures the same as
+   * input variables
+   * @param workspaceOnly <code>true</code> if this is a setup block and
+   * we should declare only constant workspace variables, <code>false</code> to
+   * declare all variables
+   */
   protected void generateBody(ClassGenerator<?> g, BlockType bt, String body, HoldingContainer[] inputVariables,
-      JVar[] workspaceJVars, boolean decConstantInputOnly) {
-    if (!Strings.isNullOrEmpty(body) && !body.trim().isEmpty()) {
-      JBlock sub = new JBlock(true, true);
-      if (decConstantInputOnly) {
-        addProtectedBlock(g, sub, body, inputVariables, workspaceJVars, true);
-      } else {
-        addProtectedBlock(g, sub, body, null, workspaceJVars, false);
-      }
-      g.getBlock(bt).directStatement(String.format("/** start %s for function %s **/ ", bt.name(), attributes.getRegisteredNames()[0]));
-      g.getBlock(bt).add(sub);
-      g.getBlock(bt).directStatement(String.format("/** end %s for function %s **/ ", bt.name(), attributes.getRegisteredNames()[0]));
+      JVar[] workspaceJVars, boolean workspaceOnly) {
+    if (Strings.isNullOrEmpty(body) || body.trim().isEmpty()) {
+      return;
     }
+    g.getBlock(bt).directStatement(String.format(
+        "/** start %s for function %s **/ ", bt.name(), attributes.getRegisteredNames()[0]));
+
+    JBlock sub = new JBlock(true, true);
+    addProtectedBlock(g, sub, body, inputVariables, workspaceJVars, workspaceOnly);
+    g.getBlock(bt).add(sub);
+
+    g.getBlock(bt).directStatement(String.format(
+        "/** end %s for function %s **/ ", bt.name(), attributes.getRegisteredNames()[0]));
   }
 
-  protected void addProtectedBlock(ClassGenerator<?> g, JBlock sub, String body, HoldingContainer[] inputVariables,
-      JVar[] workspaceJVars, boolean decConstInputOnly) {
+  /**
+   * Generate the function block itself, without surrounding comments, and
+   * whether or not the method is empty.
+   */
+  protected void addProtectedBlock(ClassGenerator<?> g, JBlock sub, String body,
+      HoldingContainer[] inputVariables,
+      JVar[] workspaceJVars, boolean workspaceOnly) {
+
+    // Create the binding between simulated function fields and their values,
     if (inputVariables != null) {
+      // If VarArgs, all input variables go into a single array.
       if (isVarArg()) {
         declareVarArgArray(g.getModel(), sub, inputVariables);
       }
       for (int i = 0; i < inputVariables.length; i++) {
-        if (decConstInputOnly && !inputVariables[i].isConstant()) {
+        if (workspaceOnly && !inputVariables[i].isConstant()) {
           continue;
         }
-        declare(g.getModel(), sub, inputVariables[i], i);
+        declareInputVariable(g.getModel(), sub, inputVariables[i], i);
       }
     }
 
+    // Declare workspace variables
     JVar[] internalVars = new JVar[workspaceJVars.length];
     for (int i = 0; i < workspaceJVars.length; i++) {
-      if (decConstInputOnly) {
-        internalVars[i] = sub.decl(
-            g.getModel()._ref(attributes.getWorkspaceVars()[i].getType()),
-            attributes.getWorkspaceVars()[i].getName(), workspaceJVars[i]);
-      } else {
-        internalVars[i] = sub.decl(
-            g.getModel()._ref(attributes.getWorkspaceVars()[i].getType()),
-            attributes.getWorkspaceVars()[i].getName(), workspaceJVars[i]);
-      }
+      internalVars[i] = sub.decl(
+          g.getModel()._ref(attributes.getWorkspaceVars()[i].getType()),
+          attributes.getWorkspaceVars()[i].getName(), workspaceJVars[i]);
     }
 
+    // Add the source code for the block.
     Preconditions.checkNotNull(body);
     sub.directStatement(body);
 
@@ -245,48 +283,6 @@
   }
 
   /**
-   * Declares attribute parameter which corresponds to specified {@code currentIndex}
-   * in specified {@code jBlock} considering its type.
-   *
-   * @param model         code model to generate the code
-   * @param jBlock        block of code to be populated
-   * @param inputVariable input variable for current function
-   * @param currentIndex  index of current parameter
-   */
-  protected void declare(JCodeModel model, JBlock jBlock,
-      HoldingContainer inputVariable, int currentIndex) {
-    ValueReference parameter = getAttributeParameter(currentIndex);
-    if (parameter.isFieldReader()
-        && !inputVariable.isReader()
-        && !Types.isComplex(inputVariable.getMajorType())
-        && inputVariable.getMinorType() != MinorType.UNION) {
-      JType singularReaderClass = model._ref(
-          TypeHelper.getHolderReaderImpl(inputVariable.getMajorType().getMinorType(),
-          inputVariable.getMajorType().getMode()));
-      JType fieldReadClass = getParamClass(model, parameter, inputVariable.getHolder().type());
-      JInvocation reader = JExpr._new(singularReaderClass).arg(inputVariable.getHolder());
-      declare(jBlock, parameter, fieldReadClass, reader, currentIndex);
-    } else if (!parameter.isFieldReader()
-        && inputVariable.isReader()
-        && Types.isComplex(parameter.getType())) {
-      // For complex data-types (repeated maps/lists/dicts) the input to the aggregate will be a FieldReader. However, aggregate
-      // functions like ANY_VALUE, will assume the input to be a RepeatedMapHolder etc. Generate boilerplate code, to map
-      // from FieldReader to respective Holder.
-      if (Types.isComplex(parameter.getType())) {
-        JType holderClass = getParamClass(model, parameter, inputVariable.getHolder().type());
-        JAssignmentTarget holderVar = declare(jBlock, parameter, holderClass, JExpr._new(holderClass), currentIndex);
-        jBlock.assign(holderVar.ref("reader"), inputVariable.getHolder());
-      }
-    } else {
-      JExpression exprToAssign = inputVariable.getHolder();
-      if (parameter.isVarArg() && parameter.isFieldReader() && Types.isUnion(inputVariable.getMajorType())) {
-        exprToAssign = exprToAssign.ref("reader");
-      }
-      declare(jBlock, parameter, inputVariable.getHolder().type(), exprToAssign, currentIndex);
-    }
-  }
-
-  /**
    * Declares array for storing vararg function arguments.
    *
    * @param model          code model to generate the code
@@ -308,6 +304,161 @@
   }
 
   /**
+   * Generate the top part of a function call which simulates passing parameters
+   * into the function. Given the following declaration: <code><pre>
+   * public static class UnionIsBigInt implements DrillSimpleFunc {
+   *   @Param UnionHolder in;
+   * </pre></code>, we generate code like the following: <code><pre>
+   *  final BitHolder out = new BitHolder();
+   *  FieldReader in = reader4;
+   * </pre></code>
+   * <p>
+   * Declares attribute parameter which corresponds to specified {@code currentIndex}
+   * in specified {@code jBlock}. Parameters are those fields
+   * in the function declaration annotated with a <tt>@Param</tt> tag. Parameters
+   * or expressions can both be represented by either a <code>FieldReader</code>
+   * or a value holder. Perform conversion between the two as needed.
+   *
+   * @param model         code model to generate the code
+   * @param jBlock        block of code to be populated
+   * @param inputVariable input variable for current function
+   * @param currentIndex  index of current parameter
+   */
+  protected void declareInputVariable(JCodeModel model, JBlock jBlock,
+      HoldingContainer inputVariable, int currentIndex) {
+    ValueReference parameter = getAttributeParameter(currentIndex);
+    if (!inputVariable.isReader() && parameter.isFieldReader()) {
+      convertHolderToReader(model, jBlock, inputVariable, currentIndex, parameter);
+    } else if (inputVariable.isReader() && !parameter.isFieldReader()) {
+      convertReaderToHolder(model, jBlock, inputVariable, currentIndex, parameter);
+    } else {
+      assignParamDirectly(jBlock, inputVariable, currentIndex, parameter);
+    }
+  }
+
+  /**
+   * Convert an input variable (in the generated code) to a reader as declared
+   * on the input parameter.
+   */
+  private void convertHolderToReader(JCodeModel model, JBlock jBlock,
+      HoldingContainer inputVariable, int currentIndex,
+      ValueReference parameter) {
+    JVar inputHolder = inputVariable.getHolder();
+    MajorType inputSqlType = inputVariable.getMajorType();
+
+    // The parameter is a reader in this case.
+    JType paramClass = model._ref(FieldReader.class);
+    if (Types.isComplex(inputSqlType)) {
+        throw new UnsupportedOperationException(String.format(
+            "Cannot convert values of type %s from a holder to a reader",
+            inputSqlType.getMinorType().name()));
+    } else if (Types.isUnion(inputSqlType)) {
+      // For the UNION type, there is no simple conversion from
+      // a value holder to a reader.
+      //
+      // Prior to the fix for DRILL-7502, the parameter is redefined
+      // from type FieldReader to type UnionHolder. This is clearly
+      // wrong, but it worked in most cases. It DOES NOT work for the
+      // typeof() function, which needs a reader. Old code:
+
+      // assignParamDirectly(jBlock, inputVariable, currentIndex, parameter);
+
+      // A large amount of code depends on the UNION being represented
+      // as a UnionHolder, especially that for handling varying types.
+      // ExpressionTreeMaterializer.rewriteUnionFunction(), for example
+      // relies heavily on UnionHolder. As a result, we cannot simply
+      // change the "holder" for the UNION to be a reader, as is done
+      // for complex types.
+      //
+      // Run TestTopNSchemaChanges.testNumericTypes with saving of code
+      // enabled, and look at the second "PriorityQueueGen" for an example.
+
+      // One would think that the following should work: the UnionHolder
+      // has a reader field. However, that field is not set in the code
+      // gen path; only when creating a holder from a reader. Code which
+      // does NOT work:
+
+      // jBlock.decl(paramClass, parameter.getName(), inputHolder.ref("reader"));
+
+      // One solution that works (but is probably slow and redundant) is to
+      // obtain the reader directly from the underling value vector. We saved
+      // this information when defining the holder. We retrieve it here
+      // and insert the vector --> reader conversion.
+      VectorVariableHolder vvHolder = (VectorVariableHolder) inputVariable;
+      JVar readerVar = vvHolder.generateUnionReader();
+      declare(jBlock, parameter, paramClass, readerVar, currentIndex);
+
+      // TODO: This probably needs a more elegant solution, but this does
+      // work for now. Run TestTypeFns.testUnionType to verify.
+    } else {
+      // FooHolderReader param = new FooHolderReader(inputVar);
+      JType singularReaderClass = model._ref(
+          TypeHelper.getHolderReaderImpl(inputSqlType.getMinorType(),
+              inputSqlType.getMode()));
+      JInvocation reader = JExpr._new(singularReaderClass).arg(inputHolder);
+      declare(jBlock, parameter, paramClass, reader, currentIndex);
+    }
+  }
+
+  /**
+   * Convert an input value holder (in the generated code) into a value
+   * holder as declared on the input parameter.
+   */
+  private void convertReaderToHolder(JCodeModel model, JBlock jBlock,
+      HoldingContainer inputVariable, int currentIndex,
+      ValueReference parameter) {
+    JVar inputHolder = inputVariable.getHolder();
+    MajorType inputSqlType = inputVariable.getMajorType();
+    if (Types.isComplex(parameter.getType())) {
+      // For complex data-types (repeated maps/lists/dicts) the input to the
+      // aggregate will be a FieldReader. However, aggregate
+      // functions like ANY_VALUE, will assume the input to be a
+      // RepeatedMapHolder etc. Generate boilerplate code, to map
+      // from FieldReader to respective Holder.
+
+      // FooHolder param = new FooHolder();
+      // param.reader = inputVar;
+      JType holderClass = getParamClass(model, parameter, inputHolder.type());
+      JAssignmentTarget holderVar = declare(jBlock, parameter, holderClass, JExpr._new(holderClass), currentIndex);
+      jBlock.assign(holderVar.ref("reader"), inputHolder);
+    } else if (Types.isUnion(inputSqlType)) {
+      // Normally unions are generated as a UnionHolder. However, if a parameter
+      // is a FieldReader, then we must generate the union as a UnionReader.
+      // Then, if there is another function that use a holder, we can convert
+      // from the UnionReader to a UnionHolder.
+      //
+      // UnionHolder param = new UnionHolder();
+      // inputVar.read(param);
+      JType holderClass = model._ref(UnionHolder.class);
+      JAssignmentTarget paramVar = jBlock.decl(holderClass, parameter.getName(), JExpr._new(holderClass));
+      JInvocation readCall = inputHolder.invoke("read");
+      readCall.arg(paramVar);
+      jBlock.add(readCall);
+    } else {
+      throw new UnsupportedOperationException(String.format(
+          "Cannot convert values of type %s from a reader to a holder",
+          inputSqlType.getMinorType().name()));
+    }
+  }
+
+  /**
+   * The input variable and parameter are both either a holder or a
+   * field reader.
+   */
+  private void assignParamDirectly(JBlock jBlock,
+      HoldingContainer inputVariable, int currentIndex,
+      ValueReference parameter) {
+
+    // Declare parameter as the type of the input variable because
+    // if we get here, they must be the same type.
+    //
+    // InputType param = inputVar;
+    JVar inputHolder = inputVariable.getHolder();
+    declare(jBlock, parameter, inputHolder.type(),
+        inputHolder, currentIndex);
+  }
+
+  /**
    * Returns {@link JType} instance which corresponds to the parameter of the function.
    *
    * @param model       code model to generate the code
@@ -342,10 +493,20 @@
   protected JAssignmentTarget declare(JBlock jBlock, ValueReference parameter,
       JType paramClass, JExpression paramExpression, int currentIndex) {
     if (parameter.isVarArg()) {
+      // For VarArg, an array has already been generated:
+
+      // FieldReader[] in = new FieldReader[ 4 ] ;
+
+      // Here we assign the input value to the array:
+
+      // in[ 0 ] = new VarCharHolderReaderImpl(constant1);
       JAssignmentTarget arrayComponent = JExpr.ref(parameter.getName()).component(JExpr.lit(currentIndex - getParamCount() + 1));
       jBlock.assign(arrayComponent, paramExpression);
       return arrayComponent;
     } else {
+      // Actual define the input variable with an expression.
+      // Foo in = bar12;
+
       return jBlock.decl(paramClass, parameter.getName(), paramExpression);
     }
   }
@@ -353,21 +514,15 @@
   public boolean matches(MajorType returnType, List<MajorType> argTypes) {
 
     if (!softCompare(returnType, attributes.getReturnValue().getType())) {
-      // logger.debug(String.format("Call [%s] didn't match as return type [%s] was different than expected [%s]. ",
-      // call.getDefinition().getName(), returnValue.type, call.getMajorType()));
       return false;
     }
 
     if (argTypes.size() != attributes.getParameters().length) {
-      // logger.debug(String.format("Call [%s] didn't match as the number of arguments provided [%d] were different than expected [%d]. ",
-      // call.getDefinition().getName(), parameters.length, call.args.size()));
       return false;
     }
 
     for (int i = 0; i < attributes.getParameters().length; i++) {
       if (!softCompare(getAttributeParameter(i).getType(), argTypes.get(i))) {
-        // logger.debug(String.format("Call [%s] didn't match as the argument [%s] didn't match the expected type [%s]. ",
-        // call.getDefinition().getName(), arg.getMajorType(), param.type));
         return false;
       }
     }
@@ -392,7 +547,7 @@
   /**
    * Returns i-th function attribute parameter.
    * For the case when current function is vararg and specified index
-   * is greater than or equals to the attributes count, the last function attribute parameter  is returnedd.
+   * is greater than or equals to the attributes count, the last function attribute parameter  is returned.
    *
    * @param i index of function attribute parameter to  be returned
    * @return i-th function attribute parameter
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java
index 54e176c..580c3d2 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java
@@ -40,6 +40,13 @@
 import com.sun.codemodel.JOp;
 import com.sun.codemodel.JVar;
 
+/**
+ * Definition of a Drill function defined using the
+ * <code>@FunctionTemplate</code> annotation of the class which
+ * implements the function. Simple functions have
+ * input parameters as defined by member variables annotated
+ * with the <code>@Param</code> annotation.
+ */
 public class DrillSimpleFuncHolder extends DrillFuncHolder {
 
   private final String drillFuncClass;
@@ -78,28 +85,56 @@
     return (DrillSimpleFunc)Class.forName(drillFuncClass, true, classLoader).newInstance();
   }
 
+  /**
+   * Render the various code blocks for a simple function.
+   *
+   * @param classGenerator code generator
+   * @param inputVariables value holder references for each input value. Variables
+   * are in the same order as the declared function parameters
+   * @param workspaceJVars internal working variables as declared with the
+   * <code>@Workspece</code> annotations. These are simple variables, not
+   * Drill value holders
+   * @param holderExpr the function call expression
+   */
   @Override
   public HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingContainer[] inputVariables,
                                     JVar[] workspaceJVars, FunctionHolderExpression holderExpr) {
 
-    //If the function's annotation specifies a parameter has to be constant expression, but the HoldingContainer
-    //for the argument is not, then raise exception.
+    // If the function's annotation specifies a parameter has to be constant
+    // expression, but the HoldingContainer for the argument is not, then raise
+    // exception.
     for (int i = 0; i < inputVariables.length; i++) {
       if (getAttributeParameter(i).isConstant() && !inputVariables[i].isConstant()) {
-        throw new DrillRuntimeException(String.format("The argument '%s' of Function '%s' has to be constant!", getAttributeParameter(i).getName(), this.getRegisteredNames()[0]));
+        throw new DrillRuntimeException(String.format(
+            "The argument '%s' of Function '%s' has to be constant!",
+            getAttributeParameter(i).getName(), this.getRegisteredNames()[0]));
       }
     }
+    // Inline code from the function's setup block
     generateBody(classGenerator, BlockType.SETUP, setupBody(), inputVariables, workspaceJVars, true);
+
+    // Generate the wrapper, and inline code for, the function's eval block
     HoldingContainer c = generateEvalBody(classGenerator, inputVariables, evalBody(), workspaceJVars, holderExpr);
+
+    // Generate code for an aggregate functions reset block
     generateBody(classGenerator, BlockType.RESET, resetBody(), null, workspaceJVars, false);
+
+    // Inline code from the function's cleanup() method
     generateBody(classGenerator, BlockType.CLEANUP, cleanupBody(), null, workspaceJVars, false);
     return c;
   }
 
+  /**
+   * Generate the eval block for a simple function, including the null-handling wrapper,
+   * if requested.
+   *
+   * @see {@link #generateBody()}
+   */
   protected HoldingContainer generateEvalBody(ClassGenerator<?> g, HoldingContainer[] inputVariables, String body,
                                               JVar[] workspaceJVars, FunctionHolderExpression holderExpr) {
 
-    g.getEvalBlock().directStatement(String.format("//---- start of eval portion of %s function. ----//", getRegisteredNames()[0]));
+    g.getEvalBlock().directStatement(String.format(
+        "//---- start of eval portion of %s function. ----//", getRegisteredNames()[0]));
 
     JBlock sub = new JBlock(true, true);
     JBlock topSub = sub;
@@ -107,13 +142,15 @@
     MajorType returnValueType = getReturnType();
 
     // add outside null handling if it is defined.
+    // If defined, the actual eval is in the "else" block of the
+    // null checks.
     if (getNullHandling() == NullHandling.NULL_IF_NULL) {
       JExpression e = null;
       for (HoldingContainer v : inputVariables) {
         if (v.isOptional()) {
           JExpression isNullExpr;
           if (v.isReader()) {
-           isNullExpr = JOp.cond(v.getHolder().invoke("isSet"), JExpr.lit(1), JExpr.lit(0));
+            isNullExpr = JOp.cond(v.getHolder().invoke("isSet"), JExpr.lit(1), JExpr.lit(0));
           } else {
             isNullExpr = v.getIsSet();
           }
@@ -145,20 +182,26 @@
     // add the subblock after the out declaration.
     g.getEvalBlock().add(topSub);
 
+    JVar internalOutput = sub.decl(JMod.FINAL, g.getHolderType(returnValueType),
+        getReturnValue().getName(), JExpr._new(g.getHolderType(returnValueType)));
 
-    JVar internalOutput = sub.decl(JMod.FINAL, g.getHolderType(returnValueType), getReturnValue().getName(), JExpr._new(g.getHolderType(returnValueType)));
+    // Generate function body from source, including fields rendered as
+    // block local vars.
     addProtectedBlock(g, sub, body, inputVariables, workspaceJVars, false);
 
+    // Copy the output from the internal output parameter to the
+    // one known in the outer scope.
     List<String> holderFields = ValueHolderHelper.getHolderParams(returnValueType);
     for (String holderField : holderFields) {
       sub.assign(out.f(holderField), internalOutput.ref(holderField));
     }
 
     if (sub != topSub) {
-      sub.assign(out.f("isSet"),JExpr.lit(1));  // Assign null if NULL_IF_NULL mode
+      sub.assign(out.f("isSet"), JExpr.lit(1));  // Assign null if NULL_IF_NULL mode
     }
 
-    g.getEvalBlock().directStatement(String.format("//---- end of eval portion of %s function. ----//", getRegisteredNames()[0]));
+    g.getEvalBlock().directStatement(String.format(
+        "//---- end of eval portion of %s function. ----//", getRegisteredNames()[0]));
 
     return out;
   }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionAttributes.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionAttributes.java
index 0d11a51..c9b2c6e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionAttributes.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionAttributes.java
@@ -23,17 +23,41 @@
 import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
 
 /**
- * Attributes of a function
- * Those are used in code generation and optimization.
+ * Attributes of a function used in code generation and optimization.
+ * Represents the values contained in the annotations of a Drill
+ * function.
  */
 public class FunctionAttributes {
 
+  /**
+   * Reference to the <code>@FunctionTemplate</code> annotation
+   * instance for the function.
+   */
   private final FunctionTemplate template;
-  private final String[] registeredNames;
-  private final ValueReference[] parameters;
-  private final ValueReference returnValue;
-  private final WorkspaceReference[] workspaceVars;
 
+  /**
+   * Function names (aliases). Some functions are known by multiple
+   * names.
+   */
+  private final String[] registeredNames;
+
+  /**
+   * Input parameters to the function. Indicated by the
+   * <code>@Param</code> annotation for Drill functions.
+   */
+  private final ValueReference[] parameters;
+
+  /**
+   * Single return value of the function. Indicated by the
+   * <code>@Output</code> annotation for Drill functions.
+   */
+  private final ValueReference returnValue;
+
+  /**
+   * Drill functions allow extra "workspace" variables indicated
+   * by the @Workspace annotation.
+   */
+  private final WorkspaceReference[] workspaceVars;
 
   public FunctionAttributes (FunctionTemplate template,
                              ValueReference[] parameters,
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionGenerationHelper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionGenerationHelper.java
index 2871173..52b89db 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionGenerationHelper.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionGenerationHelper.java
@@ -113,7 +113,7 @@
   private static LogicalExpression getFunctionExpression(String name, HoldingContainer... args) {
     List<MajorType> argTypes = new ArrayList<MajorType>(args.length);
     List<LogicalExpression> argExpressions = new ArrayList<LogicalExpression>(args.length);
-    for(HoldingContainer c : args) {
+    for (HoldingContainer c : args) {
       argTypes.add(c.getMajorType());
       argExpressions.add(new HoldingContainerExpression(c));
     }
@@ -132,7 +132,7 @@
   private static LogicalExpression getTypeComparisonFunction(LogicalExpression comparisonFunction, HoldingContainer... args) {
     List<LogicalExpression> argExpressions = Lists.newArrayList();
     List<MajorType> argTypes = Lists.newArrayList();
-    for(HoldingContainer c : args) {
+    for (HoldingContainer c : args) {
       argTypes.add(c.getMajorType());
       argExpressions.add(new HoldingContainerExpression(c));
     }
@@ -175,5 +175,4 @@
 
     return sb.toString();
   }
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionInitializer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionInitializer.java
index 5fa1774..2b9d223 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionInitializer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionInitializer.java
@@ -31,13 +31,15 @@
 import org.codehaus.janino.Parser;
 import org.codehaus.janino.Scanner;
 import org.mortbay.util.IO;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * To avoid the cost of initializing all functions up front,
- * this class contains all information required to initializing a function when it is used.
+ * To avoid the cost of initializing all functions up front, this class contains
+ * all information required to initializing a function when it is used.
  */
 public class FunctionInitializer {
-  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FunctionInitializer.class);
+  private static final Logger logger = LoggerFactory.getLogger(FunctionInitializer.class);
 
   private final String className;
   private final ClassLoader classLoader;
@@ -46,9 +48,12 @@
   private volatile boolean isLoaded;
 
   /**
-   * @param className the fully qualified name of the class implementing the function
-   * @param classLoader class loader associated with the function, is unique for each jar that holds function
-   *                    to prevent classpath collisions during loading an unloading jars
+   * @param className
+   *          the fully qualified name of the class implementing the function
+   * @param classLoader
+   *          class loader associated with the function, is unique for each jar
+   *          that holds function to prevent classpath collisions during loading
+   *          an unloading jars
    */
   public FunctionInitializer(String className, ClassLoader classLoader) {
     this.className = className;
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ValueReference.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ValueReference.java
index 94c6800..32761ad 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ValueReference.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ValueReference.java
@@ -22,14 +22,19 @@
 import org.apache.drill.common.types.TypeProtos.MinorType;
 import org.apache.drill.common.types.Types;
 
+/**
+ * Represents a declared variable (parameter) in a Drill function.
+ * This is a field declared with the <code>@Param</code> or
+ * <code>@Output</code> tags.
+ */
 public class ValueReference {
   private final MajorType type;
   private final String name;
-  private boolean isConstant = false;
-  private boolean isFieldReader = false;
-  private boolean isComplexWriter = false;
-  private boolean isInternal = false;
-  private boolean isVarArg = false;
+  private boolean isConstant;
+  private boolean isFieldReader;
+  private boolean isComplexWriter;
+  private boolean isInternal;
+  private boolean isVarArg;
 
   public ValueReference(MajorType type, String name) {
     Preconditions.checkNotNull(type);
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
index 70db585..aff9f27 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
@@ -40,7 +40,6 @@
 import java.nio.charset.Charset;
 
 public class StringFunctions{
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(StringFunctions.class);
 
   private StringFunctions() {}
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/TypeFunctions.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/TypeFunctions.java
new file mode 100644
index 0000000..3a1df8f
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/TypeFunctions.java
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.expr.fn.impl;
+
+import javax.inject.Inject;
+
+import org.apache.drill.exec.expr.DrillSimpleFunc;
+import org.apache.drill.exec.expr.annotations.FunctionTemplate;
+import org.apache.drill.exec.expr.annotations.Output;
+import org.apache.drill.exec.expr.annotations.Param;
+import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
+import org.apache.drill.exec.expr.holders.VarCharHolder;
+import org.apache.drill.exec.vector.complex.reader.FieldReader;
+
+import io.netty.buffer.DrillBuf;
+
+/**
+ * Type functions for all types. See UnionFunctions for type functions
+ * specifically for the UNION type.
+ */
+
+public class TypeFunctions {
+
+  @FunctionTemplate(name = "sqlTypeOf",
+          scope = FunctionTemplate.FunctionScope.SIMPLE,
+          nulls = NullHandling.INTERNAL)
+  public static class GetSqlType implements DrillSimpleFunc {
+
+    @Param
+    FieldReader input;
+    @Output
+    VarCharHolder out;
+    @Inject
+    DrillBuf buf;
+
+    @Override
+    public void setup() {}
+
+    @Override
+    public void eval() {
+
+      String typeName = org.apache.drill.common.types.Types.getExtendedSqlTypeName(input.getType());
+      byte[] type = typeName.getBytes();
+      buf = buf.reallocIfNeeded(type.length);
+      buf.setBytes(0, type);
+      out.buffer = buf;
+      out.start = 0;
+      out.end = type.length;
+    }
+  }
+
+  @FunctionTemplate(name = "drillTypeOf",
+          scope = FunctionTemplate.FunctionScope.SIMPLE,
+          nulls = NullHandling.INTERNAL)
+  public static class GetDrillType implements DrillSimpleFunc {
+
+    @Param
+    FieldReader input;
+    @Output
+    VarCharHolder out;
+    @Inject
+    DrillBuf buf;
+
+    @Override
+    public void setup() {}
+
+    @Override
+    public void eval() {
+      String typeName = input.getVectorType().name();
+      byte[] type = typeName.getBytes();
+      buf = buf.reallocIfNeeded(type.length);
+      buf.setBytes(0, type);
+      out.buffer = buf;
+      out.start = 0;
+      out.end = type.length;
+    }
+  }
+
+  @FunctionTemplate(name = "modeOf",
+          scope = FunctionTemplate.FunctionScope.SIMPLE,
+          nulls = NullHandling.INTERNAL)
+  public static class GetMode implements DrillSimpleFunc {
+
+    @Param
+    FieldReader input;
+    @Output
+    VarCharHolder out;
+    @Inject
+    DrillBuf buf;
+
+    @Override
+    public void setup() {}
+
+    @Override
+    public void eval() {
+      String typeName = org.apache.drill.common.types.Types.getSqlModeName(
+          input.getType());
+      byte[] type = typeName.getBytes();
+      buf = buf.reallocIfNeeded(type.length);
+      buf.setBytes(0, type);
+      out.buffer = buf;
+      out.start = 0;
+      out.end = type.length;
+    }
+  }
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/UnionFunctions.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/UnionFunctions.java
index 8c3d662..a5fb217 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/UnionFunctions.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/UnionFunctions.java
@@ -35,13 +35,16 @@
 import io.netty.buffer.DrillBuf;
 
 /**
- * The class contains additional functions for union types in addition to those in GUnionFunctions
+ * Contains additional functions for union types in addition to those in
+ * GUnionFunctions
  */
 public class UnionFunctions {
 
   /**
-   * Returns zero if the inputs have equivalent types. Two numeric types are considered equivalent, as are a combination
-   * of date/timestamp. If not equivalent, returns a value determined by the numeric value of the MinorType enum
+   * Returns zero if the inputs have equivalent types. Two numeric types are
+   * considered equivalent, as are a combination of date/timestamp. If not
+   * equivalent, returns a value determined by the numeric value of the
+   * MinorType enum
    */
   @FunctionTemplate(names = {"compareType"},
           scope = FunctionTemplate.FunctionScope.SIMPLE,
@@ -84,11 +87,9 @@
   }
 
   /**
-   * Gives a type ordering modeled after the behavior of MongoDB
-   * Numeric types are first, folowed by string types, followed by binary, then boolean, then date, then timestamp
-   * Any other times will be sorted after that
-   * @param type
-   * @return
+   * Gives a type ordering modeled after the behavior of MongoDB Numeric types
+   * are first, followed by string types, followed by binary, then boolean, then
+   * date, then timestamp Any other times will be sorted after that
    */
   private static int getTypeValue(MinorType type) {
     if (TypeCastRules.isNumericType(type)) {
@@ -144,68 +145,6 @@
 
     @Override
     public void eval() {
-
-      String typeName;
-      if (input.isSet()) {
-        typeName = input.getTypeString();
-      } else {
-        typeName = org.apache.drill.common.types.TypeProtos.MinorType.NULL.name();
-      }
-      byte[] type = typeName.getBytes();
-      buf = buf.reallocIfNeeded(type.length);
-      buf.setBytes(0, type);
-      out.buffer = buf;
-      out.start = 0;
-      out.end = type.length;
-    }
-  }
-
-  @FunctionTemplate(name = "sqlTypeOf",
-          scope = FunctionTemplate.FunctionScope.SIMPLE,
-          nulls = NullHandling.INTERNAL)
-  public static class GetSqlType implements DrillSimpleFunc {
-
-    @Param
-    FieldReader input;
-    @Output
-    VarCharHolder out;
-    @Inject
-    DrillBuf buf;
-
-    @Override
-    public void setup() {}
-
-    @Override
-    public void eval() {
-
-      String typeName = org.apache.drill.common.types.Types.getExtendedSqlTypeName(input.getType());
-      byte[] type = typeName.getBytes();
-      buf = buf.reallocIfNeeded(type.length);
-      buf.setBytes(0, type);
-      out.buffer = buf;
-      out.start = 0;
-      out.end = type.length;
-    }
-  }
-
-  @FunctionTemplate(name = "drillTypeOf",
-          scope = FunctionTemplate.FunctionScope.SIMPLE,
-          nulls = NullHandling.INTERNAL)
-  public static class GetDrillType implements DrillSimpleFunc {
-
-    @Param
-    FieldReader input;
-    @Output
-    VarCharHolder out;
-    @Inject
-    DrillBuf buf;
-
-    @Override
-    public void setup() {}
-
-    @Override
-    public void eval() {
-
       String typeName = input.getTypeString();
       byte[] type = typeName.getBytes();
       buf = buf.reallocIfNeeded(type.length);
@@ -216,35 +155,6 @@
     }
   }
 
-  @FunctionTemplate(name = "modeOf",
-          scope = FunctionTemplate.FunctionScope.SIMPLE,
-          nulls = NullHandling.INTERNAL)
-  public static class GetMode implements DrillSimpleFunc {
-
-    @Param
-    FieldReader input;
-    @Output
-    VarCharHolder out;
-    @Inject
-    DrillBuf buf;
-
-    @Override
-    public void setup() {}
-
-    @Override
-    public void eval() {
-
-      String typeName = org.apache.drill.common.types.Types.getSqlModeName(
-          input.getType());
-      byte[] type = typeName.getBytes();
-      buf = buf.reallocIfNeeded(type.length);
-      buf.setBytes(0, type);
-      out.buffer = buf;
-      out.start = 0;
-      out.end = type.length;
-    }
-  }
-
   @FunctionTemplate(names = {"castUNION", "castToUnion"},
       scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.NULL_IF_NULL)
   public static class CastUnionToUnion implements DrillSimpleFunc{
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
index 555b78d..33b2196 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
@@ -67,6 +67,9 @@
 import org.apache.drill.exec.vector.complex.AbstractContainerVector;
 
 import org.apache.drill.shaded.guava.com.google.common.base.Stopwatch;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.sun.codemodel.JConditional;
 import com.sun.codemodel.JExpr;
 
@@ -81,7 +84,7 @@
  * internally it maintains a priority queue backed by a heap with the size being same as limit value.
  */
 public class TopNBatch extends AbstractRecordBatch<TopN> {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TopNBatch.class);
+  static final Logger logger = LoggerFactory.getLogger(TopNBatch.class);
 
   private final MappingSet mainMapping = createMainMappingSet();
   private final MappingSet leftMapping = createLeftMappingSet();
@@ -92,7 +95,7 @@
 
   private final RecordBatch incoming;
   private BatchSchema schema;
-  private boolean schemaChanged = false;
+  private boolean schemaChanged;
   private PriorityQueue priorityQueue;
   private final TopN config;
   private SelectionVector4 sv4;
@@ -100,10 +103,10 @@
   private int batchCount;
   private Copier copier;
   private boolean first = true;
-  private int recordCount = 0;
+  private int recordCount;
   private IterOutcome lastKnownOutcome = OK;
   private boolean firstBatchForSchema = true;
-  private boolean hasOutputRecords = false;
+  private boolean hasOutputRecords;
 
   public TopNBatch(TopN popConfig, FragmentContext context, RecordBatch incoming) throws OutOfMemoryException {
     super(popConfig, context);
@@ -395,15 +398,17 @@
     FunctionLookupContext functionLookupContext = context.getFunctionRegistry();
     CodeGenerator<PriorityQueue> cg = CodeGenerator.get(PriorityQueue.TEMPLATE_DEFINITION, optionSet);
     cg.plainJavaCapable(true);
-    // Uncomment out this line to debug the generated code.
     cg.saveCodeForDebugging(codegenDump);
+    // Uncomment out this line to debug the generated code.
+    // cg.saveCodeForDebugging(true);
     ClassGenerator<PriorityQueue> g = cg.getRoot();
     g.setMappingSet(mainMapping);
 
     for (Ordering od : orderings) {
       // first, we rewrite the evaluation stack for each side of the comparison.
       ErrorCollector collector = new ErrorCollectorImpl();
-      final LogicalExpression expr = ExpressionTreeMaterializer.materialize(od.getExpr(), batch, collector, functionLookupContext, unionTypeEnabled);
+      final LogicalExpression expr = ExpressionTreeMaterializer.materialize(od.getExpr(),
+          batch, collector, functionLookupContext, unionTypeEnabled);
       if (collector.hasErrors()) {
         throw new SchemaChangeException("Failure while materializing expression. " + collector.toErrorString());
       }
@@ -591,7 +596,7 @@
       // across multiple record boundary unless a new schema is observed
       int index = 0;
       for (VectorWrapper<?> w : dataContainer) {
-        HyperVectorWrapper wrapper = (HyperVectorWrapper<?>) container.getValueVector(index++);
+        HyperVectorWrapper<?> wrapper = (HyperVectorWrapper<?>) container.getValueVector(index++);
         wrapper.updateVectorList(w.getValueVectors());
       }
       // Since the reference of SV4 is held by downstream operator and there is no schema change, so just copy the
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java
index 97afac0..9857102 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java
@@ -379,8 +379,6 @@
 
     ClassGenerator<Projector> cg = CodeGenerator.getRoot(Projector.TEMPLATE_DEFINITION, context.getOptions());
     cg.getCodeGenerator().plainJavaCapable(true);
-    // Uncomment out this line to debug the generated code.
-    // cg.getCodeGenerator().saveCodeForDebugging(true);
 
     IntHashSet transferFieldIds = new IntHashSet();
 
@@ -550,8 +548,8 @@
     try {
       CodeGenerator<Projector> codeGen = cg.getCodeGenerator();
       codeGen.plainJavaCapable(true);
-      // Uncomment out this line to debug the generated code.
-      //codeGen.saveCodeForDebugging(true);
+      // Uncomment this line to debug the generated code.
+      // codeGen.saveCodeForDebugging(true);
       this.projector = context.getImplementationClass(codeGen);
       projector.setup(context, incomingBatch, this, transfers);
     } catch (ClassTransformationException | IOException e) {
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
index 742abe1..4cc7a4f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
@@ -105,15 +105,14 @@
 import org.apache.drill.exec.work.foreman.ForemanSetupException;
 import org.apache.drill.exec.work.foreman.SqlUnsupportedException;
 import org.slf4j.Logger;
-
+import org.slf4j.LoggerFactory;
 import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
 import org.apache.drill.shaded.guava.com.google.common.base.Stopwatch;
 import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
 
 public class DefaultSqlHandler extends AbstractSqlHandler {
-  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DefaultSqlHandler.class);
+  private static final Logger logger = LoggerFactory.getLogger(DefaultSqlHandler.class);
 
-  // protected final QueryContext context;
   private final Pointer<String> textPlan;
   private final long targetSliceSize;
   protected final SqlHandlerConfig config;
@@ -129,7 +128,6 @@
     this.context = config.getContext();
     this.textPlan = textPlan;
     this.targetSliceSize = config.getContext().getOptions().getOption(ExecConstants.SLICE_TARGET_OPTION);
-
   }
 
   protected void log(final PlannerType plannerType, final PlannerPhase phase, final RelNode node, final Logger logger,
@@ -182,7 +180,6 @@
     return plan;
   }
 
-
   /**
    * Rewrite the parse tree. Used before validating the parse tree. Useful if a particular statement needs to converted
    * into another statement.
@@ -334,18 +331,14 @@
         return super.visit(other);
       }
     }
-
   }
 
   /**
    * Transform RelNode to a new RelNode without changing any traits. Also will log the outcome.
    *
-   * @param plannerType
-   *          The type of Planner to use.
-   * @param phase
-   *          The transformation phase we're running.
-   * @param input
-   *          The origianl RelNode
+   * @param plannerType The type of Planner to use.
+   * @param phase The transformation phase we're running.
+   * @param input The original RelNode
    * @return The transformed relnode.
    */
   private RelNode transform(PlannerType plannerType, PlannerPhase phase, RelNode input) {
@@ -355,14 +348,10 @@
   /**
    * Transform RelNode to a new RelNode, targeting the provided set of traits. Also will log the outcome.
    *
-   * @param plannerType
-   *          The type of Planner to use.
-   * @param phase
-   *          The transformation phase we're running.
-   * @param input
-   *          The origianl RelNode
-   * @param targetTraits
-   *          The traits we are targeting for output.
+   * @param plannerType The type of Planner to use.
+   * @param phase The transformation phase we're running.
+   * @param input The original RelNode
+   * @param targetTraits The traits we are targeting for output.
    * @return The transformed relnode.
    */
   protected RelNode transform(PlannerType plannerType, PlannerPhase phase, RelNode input, RelTraitSet targetTraits) {
@@ -372,16 +361,11 @@
   /**
    * Transform RelNode to a new RelNode, targeting the provided set of traits. Also will log the outcome if asked.
    *
-   * @param plannerType
-   *          The type of Planner to use.
-   * @param phase
-   *          The transformation phase we're running.
-   * @param input
-   *          The origianl RelNode
-   * @param targetTraits
-   *          The traits we are targeting for output.
-   * @param log
-   *          Whether to log the planning phase.
+   * @param plannerType The type of Planner to use.
+   * @param phase The transformation phase we're running.
+   * @param input The original RelNode
+   * @param targetTraits The traits we are targeting for output.
+   * @param log Whether to log the planning phase.
    * @return The transformed relnode.
    */
   protected RelNode transform(PlannerType plannerType, PlannerPhase phase, RelNode input, RelTraitSet targetTraits,
@@ -658,7 +642,6 @@
       }
       return null;
     }
-
   }
 
   protected Pair<SqlNode, RelDataType> validateNode(SqlNode sqlNode) throws ValidationException, RelConversionException, ForemanSetupException {
@@ -795,6 +778,4 @@
       return this.validatedRowType;
     }
   }
-
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/TypedFieldId.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/TypedFieldId.java
index 178ddfa..e5a22ff 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/TypedFieldId.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/TypedFieldId.java
@@ -241,7 +241,7 @@
         secondaryFinal = finalType;
       }
 
-      MajorType actualFinalType = finalType;
+      //MajorType actualFinalType = finalType;
       //MajorType secondaryFinal = finalType;
 
       // if this has an index, switch to required type for output
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
index 99e4ee0..8603b20 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java
@@ -757,9 +757,9 @@
   private static final int DATAMODE_CAST_COST = 1;
   private static final int VARARG_COST = Integer.MAX_VALUE / 2;
 
-  /*
-   * code decide whether it's legal to do implicit cast. -1 : not allowed for
-   * implicit cast > 0: cost associated with implicit cast. ==0: parms are
+  /**
+   * Decide whether it's legal to do implicit cast. -1 : not allowed for
+   * implicit cast > 0: cost associated with implicit cast. ==0: params are
    * exactly same type of arg. No need of implicit.
    */
   public static int getCost(List<MajorType> argumentTypes, DrillFuncHolder holder) {
@@ -772,12 +772,14 @@
     // Indicates whether we used secondary cast rules
     boolean secondaryCast = false;
 
-    // number of arguments that could implicitly casts using precedence map or didn't require casting at all
+    // number of arguments that could implicitly casts using precedence map or
+    // didn't require casting at all
     int nCasts = 0;
 
     /*
-     * If we are determining function holder for decimal data type, we need to make sure the output type of
-     * the function can fit the precision that we need based on the input types.
+     * If we are determining function holder for decimal data type, we need to
+     * make sure the output type of the function can fit the precision that we
+     * need based on the input types.
      */
     if (holder.checkPrecisionRange()) {
       List<LogicalExpression> logicalExpressions = Lists.newArrayList();
@@ -800,12 +802,10 @@
       //@Param FieldReader will match any type
       if (holder.isFieldReader(i)) {
 //        if (Types.isComplex(call.args.get(i).getMajorType()) ||Types.isRepeated(call.args.get(i).getMajorType()) )
-        // add the max cost when encountered with a field reader considering that it is the most expensive factor
-        // contributing to the cost.
+        // add the max cost when encountered with a field reader considering
+        // that it is the most expensive factor contributing to the cost.
         cost += ResolverTypePrecedence.MAX_IMPLICIT_CAST_COST;
         continue;
-//        else
-//          return -1;
       }
 
       if (!TypeCastRules.isCastableWithNullHandling(argType, paramType, holder.getNullHandling())) {
@@ -829,8 +829,8 @@
 
       if (paramVal - argVal < 0) {
 
-        /* Precedence rules does not allow to implicitly cast, however check
-         * if the seconday rules allow us to cast
+        /* Precedence rules do not allow to implicit cast, however check
+         * if the secondary rules allow us to cast
          */
         Set<MinorType> rules;
         if ((rules = (ResolverTypePrecedence.secondaryImplicitCastRules.get(paramType.getMinorType()))) != null
@@ -841,7 +841,7 @@
         }
       }
       // Check null vs non-null, using same logic as that in Types.softEqual()
-      // Only when the function uses NULL_IF_NULL, nullable and non-nullable are inter-changable.
+      // Only when the function uses NULL_IF_NULL, nullable and non-nullable are interchangeable.
       // Otherwise, the function implementation is not a match.
       if (argType.getMode() != paramType.getMode()) {
         // TODO - this does not seem to do what it is intended to
@@ -883,7 +883,7 @@
         }
       }
 
-      // increase cost vor var arg functions to prioritize regular ones
+      // increase cost for var arg functions to prioritize regular ones
       Integer additionalCost = ResolverTypePrecedence.precedenceMap.get(holder.getParamMajorType(varArgIndex).getMinorType());
       cost += additionalCost != null ? additionalCost : VARARG_COST;
       cost += holder.getParamMajorType(varArgIndex).getMode() == DataMode.REQUIRED ? 0 : 1;
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/ServerMetaProvider.java b/exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/ServerMetaProvider.java
index 97df542..64af4ad 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/ServerMetaProvider.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/ServerMetaProvider.java
@@ -88,8 +88,8 @@
     // duplicates, and an iterable is all we need.
     ImmutableList.Builder<ConvertSupport> supportedConvertedOps = ImmutableList.builder();
 
-    for(MinorType from: MinorType.values()) {
-      for(MinorType to: MinorType.values()) {
+    for (MinorType from: MinorType.values()) {
+      for (MinorType to: MinorType.values()) {
         if (TypeCastRules.isCastable(from, to)) {
           supportedConvertedOps.add(ConvertSupport.newBuilder().setFrom(from).setTo(to).build());
         }
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestTypeFns.java b/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestTypeFns.java
index 46e3216..83cc81f 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestTypeFns.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestTypeFns.java
@@ -19,6 +19,7 @@
 
 import static org.junit.Assert.assertEquals;
 
+import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.planner.physical.PlannerSettings;
 import org.apache.drill.exec.rpc.RpcException;
 import org.apache.drill.test.ClusterFixture;
@@ -82,7 +83,7 @@
 
     sql = "SELECT typeof(CAST(a AS " + castType + ")) FROM cp.`functions/null.json`";
     result = queryBuilder().sql(sql).singletonString();
-    assertEquals("NULL", result);
+    assertEquals(resultType, result);
   }
 
   private void doTypeOfTestSpecial(String expr, String value, String resultType) throws RpcException {
@@ -222,7 +223,7 @@
         .sqlQuery(sql)
         .unOrdered()
         .baselineColumns("c1", "c2", "c3", "c4", "c5", "c6", "c7", "c8", "c9")
-        .baselineValues("INT", "VARCHAR", "DATE", "TIME", "TIMESTAMP", "BIT", "VARDECIMAL", "BIT", "NULL")
+        .baselineValues("INT", "VARCHAR", "DATE", "TIME", "TIMESTAMP", "BIT", "VARDECIMAL", "BIT", "INT")
         .go();
   }
 
@@ -305,4 +306,43 @@
         .baselineValues(0, 1, -1, -1, 0, 0)
         .go();
   }
+
+  @Test
+  public void testTypeOfWithFile() throws Exception {
+    // Column `x` does not actually appear in the file.
+    String sql ="SELECT typeof(bi) AS bi_t, typeof(fl) AS fl_t, typeof(st) AS st_t,\n" +
+                "       typeof(mp) AS mp_t, typeof(ar) AS ar_t, typeof(nu) AS nu_t,\n" +
+                "       typeof(x) AS x_t\n" +
+                "FROM cp.`jsoninput/allTypes.json`";
+     testBuilder()
+      .sqlQuery(sql)
+      .ordered()
+      .baselineColumns("bi_t",   "fl_t",   "st_t",    "mp_t", "ar_t",   "nu_t", "x_t")
+      .baselineValues( "BIGINT", "FLOAT8", "VARCHAR", "MAP",  "BIGINT", "NULL", "NULL")
+      .go();
+  }
+
+  @Test
+  public void testUnionType() throws Exception {
+    String sql ="SELECT typeof(a) AS t, modeof(a) AS m, drilltypeof(a) AS dt\n" +
+                "FROM cp.`jsoninput/union/c.json`";
+    try {
+      testBuilder()
+        .optionSettingQueriesForTestQuery("alter session set `exec.enable_union_type` = true")
+        .sqlQuery(sql)
+        .ordered()
+        .baselineColumns("t",       "m",        "dt")
+        .baselineValues( "VARCHAR", "NULLABLE", "UNION")
+        .baselineValues( "BIGINT",  "NULLABLE", "UNION")
+        .baselineValues( "FLOAT8",  "NULLABLE", "UNION")
+        // The following should probably provide the type of the list,
+        // and report cardinality as ARRAY.
+        .baselineValues( "LIST",    "NULLABLE", "UNION")
+        .baselineValues( "NULL",    "NULLABLE", "UNION")
+        .go();
+    }
+    finally {
+      client.resetSession(ExecConstants.ENABLE_UNION_TYPE_KEY);
+    }
+  }
 }
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestTopNSchemaChanges.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestTopNSchemaChanges.java
index 6b16aab..4aab0a4 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestTopNSchemaChanges.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestTopNSchemaChanges.java
@@ -54,15 +54,15 @@
 
   @Test
   public void testNumericTypes() throws Exception {
-    // left side int and strings
+    // left side int and floats
     BufferedWriter writer = new BufferedWriter(new FileWriter(new File(tableDir, "d1.json")));
     for (int i = 0; i < 10000; i+=2) {
-      writer.write(String.format("{ \"kl\" : %d , \"vl\": %d }\n", i, i));
+      writer.write(String.format("{ \"kl\": %d, \"vl\": %d }\n", i, i));
     }
     writer.close();
     writer = new BufferedWriter(new FileWriter(new File(tableDir, "d2.json")));
     for (int i = 1; i < 10000; i+=2) {
-      writer.write(String.format("{ \"kl\" : %f , \"vl\": %f }\n", (float)i, (float)i));
+      writer.write(String.format("{ \"kl\": %f, \"vl\": %f }\n", (float)i, (float)i));
     }
     writer.close();
 
@@ -87,12 +87,12 @@
     // left side int and strings
     BufferedWriter writer = new BufferedWriter(new FileWriter(new File(tableDir, "d1.json")));
     for (int i = 0; i < 1000; i+=2) {
-      writer.write(String.format("{ \"kl\" : %d , \"vl\": %d }\n", i, i));
+      writer.write(String.format("{ \"kl\": %d, \"vl\": %d }\n", i, i));
     }
     writer.close();
     writer = new BufferedWriter(new FileWriter(new File(tableDir, "d2.json")));
     for (int i = 1; i < 1000; i+=2) {
-      writer.write(String.format("{ \"kl\" : \"%s\" , \"vl\": \"%s\" }\n", i, i));
+      writer.write(String.format("{ \"kl\": \"%s\", \"vl\": \"%s\" }\n", i, i));
     }
     writer.close();
 
@@ -133,13 +133,13 @@
     for (int i = 0; i <= 9; ++i) {
       switch (i%3) {
         case 0: // 0, 3, 6, 9
-          writer.write(String.format("{ \"kl\" : %d , \"vl\": %d }\n", i, i));
+          writer.write(String.format("{ \"kl\": %d, \"vl\": %d }\n", i, i));
           break;
         case 1: // 1, 4, 7
-          writer.write(String.format("{ \"kl\" : %f , \"vl\": %f }\n", (float)i, (float)i));
+          writer.write(String.format("{ \"kl\": %f, \"vl\": %f }\n", (float)i, (float)i));
           break;
         case 2: // 2, 5, 8
-          writer.write(String.format("{ \"kl\" : \"%s\" , \"vl\": \"%s\" }\n", i, i));
+          writer.write(String.format("{ \"kl\": \"%s\", \"vl\": \"%s\" }\n", i, i));
           break;
       }
     }
@@ -153,12 +153,12 @@
 
     builder.baselineValues(0l, 0l);
     builder.baselineValues(1.0d, 1.0d);
+    builder.baselineValues("2", "2");
     builder.baselineValues(3l, 3l);
     builder.baselineValues(4.0d, 4.0d);
+    builder.baselineValues("5", "5");
     builder.baselineValues(6l, 6l);
     builder.baselineValues(7.0d, 7.0d);
-    builder.baselineValues(9l, 9l);
-    builder.baselineValues("2", "2");
     builder.go();
   }
 
@@ -166,18 +166,18 @@
   public void testMissingColumn() throws Exception {
     BufferedWriter writer = new BufferedWriter(new FileWriter(new File(tableDir, "d1.json")));
     for (int i = 0; i < 100; i++) {
-      writer.write(String.format("{ \"kl1\" : %d , \"vl1\": %d }\n", i, i));
+      writer.write(String.format("{ \"kl1\": %d, \"vl1\": %d }\n", i, i));
     }
     writer.close();
     writer = new BufferedWriter(new FileWriter(new File(tableDir, "d2.json")));
     for (int i = 100; i < 200; i++) {
-      writer.write(String.format("{ \"kl\" : %f , \"vl\": %f }\n", (float)i, (float)i));
+      writer.write(String.format("{ \"kl\": %f, \"vl\": %f }\n", (float)i, (float)i));
     }
     writer.close();
 
     writer = new BufferedWriter(new FileWriter(new File(tableDir, "d3.json")));
     for (int i = 200; i < 300; i++) {
-      writer.write(String.format("{ \"kl2\" : \"%s\" , \"vl2\": \"%s\" }\n", i, i));
+      writer.write(String.format("{ \"kl2\": \"%s\", \"vl2\": \"%s\" }\n", i, i));
     }
     writer.close();
 
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestExternalSort.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestExternalSort.java
index 31dad53..5aeea22 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestExternalSort.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestExternalSort.java
@@ -49,7 +49,6 @@
    *          and later) sort
    * @throws Exception
    */
-
   @Test
   public void testNumericTypes() throws Exception {
     final int record_count = 10000;
diff --git a/exec/java-exec/src/test/java/org/apache/drill/vector/TestFillEmpties.java b/exec/java-exec/src/test/java/org/apache/drill/vector/TestFillEmpties.java
index cc05e1e..89fcdb4 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/vector/TestFillEmpties.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/vector/TestFillEmpties.java
@@ -34,10 +34,12 @@
 
 import io.netty.buffer.DrillBuf;
 import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Category(VectorTest.class)
 public class TestFillEmpties extends SubOperatorTest {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestFillEmpties.class);
+  private static final Logger logger = LoggerFactory.getLogger(TestFillEmpties.class);
 
   @Test
   public void testNullableVarChar() {
@@ -208,5 +210,4 @@
     }
     return value;
   }
-
 }
diff --git a/exec/java-exec/src/test/resources/jsoninput/allTypes.json b/exec/java-exec/src/test/resources/jsoninput/allTypes.json
new file mode 100644
index 0000000..761e061
--- /dev/null
+++ b/exec/java-exec/src/test/resources/jsoninput/allTypes.json
@@ -0,0 +1,8 @@
+{
+  bi: 123,
+  fl: 123.4,
+  st: "foo",
+  mp: { a: 10, b: "bar" },
+  ar: [ 10, 20 ],
+  nu: null
+}
diff --git a/exec/java-exec/src/test/resources/jsoninput/union/c.json b/exec/java-exec/src/test/resources/jsoninput/union/c.json
new file mode 100644
index 0000000..7833aed
--- /dev/null
+++ b/exec/java-exec/src/test/resources/jsoninput/union/c.json
@@ -0,0 +1,5 @@
+{a: "string"}
+{a: 10}
+{a: 10.1}
+{a: [10, 20]}
+{a: null}
diff --git a/exec/vector/src/main/codegen/templates/AbstractFieldReader.java b/exec/vector/src/main/codegen/templates/AbstractFieldReader.java
index b75fa62..5ca7a88 100644
--- a/exec/vector/src/main/codegen/templates/AbstractFieldReader.java
+++ b/exec/vector/src/main/codegen/templates/AbstractFieldReader.java
@@ -28,11 +28,9 @@
 /*
  * This class is generated using freemarker and the ${.template_name} template.
  */
-@SuppressWarnings("unused")
 public abstract class AbstractFieldReader extends AbstractBaseReader implements FieldReader {
 
-  public AbstractFieldReader() {
-  }
+  public MinorType getVectorType() { return getType().getMinorType(); }
 
   /**
    * Returns true if the current value of the reader is not null
@@ -51,7 +49,6 @@
           "Text", "String", "Byte", "Short", "byte[]"] as friendlyType>
   <#assign safeType=friendlyType />
   <#if safeType=="byte[]"><#assign safeType="ByteArray" /></#if>
-
   public ${friendlyType} read${safeType}(int arrayIndex) {
     fail("read${safeType}(int arrayIndex)");
     return null;
@@ -63,7 +60,6 @@
   }
 
   </#list>
-
   public void copyAsValue(MapWriter writer) {
     fail("copyAsValue(MapWriter writer)");
   }
@@ -78,7 +74,6 @@
 
   <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
   <#assign boxedType = (minor.boxedType!type.boxedType) />
-
   public void read(${name}Holder holder) {
     fail("${name}");
   }
diff --git a/exec/vector/src/main/codegen/templates/BaseReader.java b/exec/vector/src/main/codegen/templates/BaseReader.java
index ef3a72a..df04166 100644
--- a/exec/vector/src/main/codegen/templates/BaseReader.java
+++ b/exec/vector/src/main/codegen/templates/BaseReader.java
@@ -17,8 +17,6 @@
  */
 <@pp.dropOutputFile />
 <@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/reader/BaseReader.java" />
-
-
 <#include "/@includes/license.ftl" />
 
 package org.apache.drill.exec.vector.complex.reader;
@@ -29,8 +27,7 @@
 /*
  * This class is generated using freemarker and the ${.template_name} template.
  */
-@SuppressWarnings("unused")
-public interface BaseReader extends Positionable{
+public interface BaseReader extends Positionable {
   MajorType getType();
   MaterializedField getField();
   void reset();
@@ -40,10 +37,31 @@
   boolean isSet();
   void read(ValueHolder holder);
 
+  /**
+   * Returns {@code String} representation of the reader's type. In case if
+   * {@link #getType()} is primitive, the method is equivalent to
+   * {@link #getType().getMinorType().name()}. If the reader has minor type
+   * equal to {@link org.apache.drill.common.types.TypeProtos.MinorType#DICT},
+   * {@code DICT&lt;keyMinorType,valueMinorType&gt;}, with {@code keyMinorType}
+   * and {@code valueMinorType} being key's and value's minor types
+   * respectively, will be returned. Used in {@code typeOf} UDF.
+   *
+   * @return {@code String} representation of reader's type.
+   */
+  String getTypeString();
+
+  /**
+   * Returns the type of the vector, not value. For all vectors, this is
+   * the same as the vector's <tt>getField().getType().getMinorType()</tt>.
+   * It is used to report the actual vector type in the getDrillType()
+   * function.
+   */
+  MinorType getVectorType();
+
   public interface MapReader extends BaseReader, Iterable<String>{
     FieldReader reader(String name);
   }
-  
+
   public interface RepeatedMapReader extends MapReader{
     boolean next();
     int size();
@@ -147,22 +165,22 @@
      */
     void read(Object key, ValueHolder holder);
   }
-  
+
   public interface ListReader extends BaseReader{
-    FieldReader reader(); 
+    FieldReader reader();
   }
-  
+
   public interface RepeatedListReader extends ListReader{
     boolean next();
     int size();
     void copyAsValue(ListWriter writer);
   }
-  
+
   public interface ScalarReader extends
-  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first /> ${name}Reader, </#list></#list> 
+  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first /> ${name}Reader, </#list></#list>
   <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first /> Repeated${name}Reader, </#list></#list>
   BaseReader {}
-  
+
   interface ComplexReader{
     MapReader rootAsMap();
     ListReader rootAsList();
diff --git a/exec/vector/src/main/codegen/templates/HolderReaderImpl.java b/exec/vector/src/main/codegen/templates/HolderReaderImpl.java
index 5905b19..dd0c3f8 100644
--- a/exec/vector/src/main/codegen/templates/HolderReaderImpl.java
+++ b/exec/vector/src/main/codegen/templates/HolderReaderImpl.java
@@ -87,7 +87,6 @@
 <#else>
     throw new UnsupportedOperationException("You can't call next on a single value reader.");
 </#if>
-
   }
 
   @Override
@@ -101,9 +100,9 @@
     return BasicTypeHelper.getType(holder);
 <#else>
   <#if holderMode == "Repeated">
-    return this.repeatedHolder.TYPE;
+    return repeatedHolder.TYPE;
   <#else>
-    return this.holder.TYPE;
+    return holder.TYPE;
   </#if>
 </#if>
   }
@@ -111,13 +110,12 @@
   @Override
   public boolean isSet() {
     <#if holderMode == "Repeated">
-    return this.repeatedHolder.end!=this.repeatedHolder.start;
+    return repeatedHolder.end!=this.repeatedHolder.start;
     <#elseif nullMode == "Nullable">
-    return this.holder.isSet == 1;
+    return holder.isSet == 1;
     <#else>
     return true;
     </#if>
-    
   }
 
 <#if holderMode != "Repeated">
@@ -135,8 +133,8 @@
   </#list>
     h.isSet = isSet() ? 1 : 0;
   }
-</#if>
 
+</#if>
 <#if holderMode == "Repeated">
   @Override
   public ${friendlyType} read${safeType}(int index){
@@ -147,20 +145,19 @@
     }
     return value;
   }
-</#if>
 
+</#if>
   @Override
   public ${friendlyType} read${safeType}(){
 <#if nullMode == "Nullable">
     if (!isSet()) {
       return null;
     }
+
 </#if>
-
 <#if type.major == "VarLen">
-
       int length = holder.end - holder.start;
-      byte[] value = new byte [length];
+      byte[] value = new byte[length];
       holder.buffer.getBytes(holder.start, value, 0, length);
 
 <#if minor.class == "VarBinary">
@@ -216,7 +213,6 @@
       ${friendlyType} value = new ${friendlyType}(this.holder.value);
       return value;
 </#if>
-
   }
 
   @Override
@@ -241,7 +237,7 @@
 
 <#if type.major == "VarLen">
       int length = holder.end - holder.start;
-      byte[] value = new byte [length];
+      byte[] value = new byte[length];
       holder.buffer.getBytes(holder.start, value, 0, length);
 
 <#if minor.class == "VarBinary">
@@ -298,8 +294,8 @@
       return value;
 </#if>
   }
-<#if holderMode == "Repeated">
 
+<#if holderMode == "Repeated">
   public void copyAsValue(${minor.class?cap_first}Writer writer) {
     Repeated${minor.class?cap_first}WriterImpl impl = (Repeated${minor.class?cap_first}WriterImpl) writer;
     impl.vector.getMutator().setSafe(impl.idx(), repeatedHolder);
@@ -315,7 +311,6 @@
 </#if>
     impl.vector.getMutator().setSafe(impl.idx(), repeatedHolder);
   }
-
 <#else>
   <#if !(minor.class == "Decimal9" || minor.class == "Decimal18")>
   public void copyAsValue(${minor.class?cap_first}Writer writer) {
@@ -335,11 +330,9 @@
       impl.write${minor.class}(<#list fields as field>holder.${field.name}<#if field_has_next>,</#if></#list>);
     }
   }
-
   </#if>
 </#if>
 }
-
 </#list>
 </#list>
 </#list>
diff --git a/exec/vector/src/main/codegen/templates/NullReader.java b/exec/vector/src/main/codegen/templates/NullReader.java
index 91a7b12..3924e71 100644
--- a/exec/vector/src/main/codegen/templates/NullReader.java
+++ b/exec/vector/src/main/codegen/templates/NullReader.java
@@ -26,18 +26,20 @@
 <#include "/@includes/vv_imports.ftl" />
 
 import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.common.types.TypeProtos.MajorType;
+import org.apache.drill.common.types.TypeProtos.MinorType;
 
 /*
  * This class is generated using freemarker and the ${.template_name} template.
  */
 @SuppressWarnings("unused")
 public class NullReader extends AbstractBaseReader implements FieldReader {
-  
+
   public static final NullReader INSTANCE = new NullReader();
   public static final NullReader EMPTY_LIST_INSTANCE = new NullReader(Types.repeated(TypeProtos.MinorType.NULL));
   public static final NullReader EMPTY_MAP_INSTANCE = new NullReader(Types.required(TypeProtos.MinorType.MAP));
   private MajorType type;
-  
+
   private NullReader() {
     type = Types.NULL;
   }
@@ -54,7 +56,7 @@
   public void read(ValueHolder holder) {
     throw new UnsupportedOperationException("NullReader cannot write into non-nullable holder");
   }
-  
+
   public void copyAsValue(MapWriter writer) {}
 
   public void copyAsValue(ListWriter writer) {}
@@ -73,7 +75,7 @@
   public void read(int arrayIndex, ${name}Holder holder) {
     throw new ArrayIndexOutOfBoundsException();
   }
-  
+
   public void copyAsValue(${minor.class}Writer writer) {}
   <#if minor.class == "VarDecimal">
   public void copyAsField(String name, ${minor.class}Writer writer, int precision, int scale) {}
@@ -85,57 +87,57 @@
     throw new ArrayIndexOutOfBoundsException();
   }
   </#list></#list>
-  
+
   public int size() {
     return 0;
   }
-  
+
   public boolean isSet() {
     return false;
   }
-  
+
   public boolean next() {
     return false;
   }
-  
+
   public RepeatedMapReader map() {
     return this;
   }
-  
+
   public RepeatedListReader list() {
     return this;
   }
-  
+
   public MapReader map(String name) {
     return this;
   }
-  
+
   public ListReader list(String name) {
     return this;
   }
-  
+
   public FieldReader reader(String name) {
     return this;
   }
-  
+
   public FieldReader reader() {
     return this;
   }
-  
+
   private void fail(String name) {
     throw new IllegalArgumentException(String.format("You tried to read a %s type when you are using a ValueReader of type %s.", name, this.getClass().getSimpleName()));
   }
-  
-  <#list ["Object", "BigDecimal", "Integer", "Long", "Boolean", 
+
+  <#list ["Object", "BigDecimal", "Integer", "Long", "Boolean",
           "Character", "LocalDate", "LocalTime", "LocalDateTime", "Period", "Double", "Float",
           "Text", "String", "Byte", "Short", "byte[]"] as friendlyType>
   <#assign safeType=friendlyType />
   <#if safeType=="byte[]"><#assign safeType="ByteArray" /></#if>
-  
+
   public ${friendlyType} read${safeType}(int arrayIndex) {
     return null;
   }
-  
+
   public ${friendlyType} read${safeType}() {
     return null;
   }
diff --git a/exec/vector/src/main/codegen/templates/UnionReader.java b/exec/vector/src/main/codegen/templates/UnionReader.java
index 47957b6..a865aef 100644
--- a/exec/vector/src/main/codegen/templates/UnionReader.java
+++ b/exec/vector/src/main/codegen/templates/UnionReader.java
@@ -15,7 +15,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import org.apache.drill.common.types.Types;
+import org.apache.drill.common.types.TypeProtos.MajorType;
 import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.vector.complex.UnionVector;
 import org.apache.drill.exec.vector.complex.impl.NullReader;
 
 <@pp.dropOutputFile />
@@ -31,26 +34,22 @@
 /*
  * This class is generated using freemarker and the ${.template_name} template.
  */
-@SuppressWarnings("unused")
 public class UnionReader extends AbstractFieldReader {
 
-  private BaseReader[] readers = new BaseReader[45];
-  public UnionVector data;
-  
+  public final UnionVector data;
+  private final BaseReader[] readers = new BaseReader[UnionVector.TYPE_COUNT];
+
   public UnionReader(UnionVector data) {
     this.data = data;
   }
 
-  private static MajorType[] TYPES = new MajorType[45];
-
-  static {
-    for (MinorType minorType : MinorType.values()) {
-      TYPES[minorType.getNumber()] = Types.optional(minorType);
-    }
+  @Override
+  public MajorType getType() {
+    return UnionVector.TYPES[data.getTypeValue(idx())];
   }
 
-  public MajorType getType() {
-    return TYPES[data.getTypeValue(idx())];
+  public MinorType getVectorType() {
+    return data.getField().getType().getMinorType();
   }
 
   public boolean isSet(){
@@ -68,23 +67,27 @@
 
   private FieldReader getReaderForIndex(int index) {
     int typeValue = data.getTypeValue(index);
+    if (typeValue == UnionVector.NULL_MARKER) {
+      return NullReader.INSTANCE;
+    }
     FieldReader reader = (FieldReader) readers[typeValue];
     if (reader != null) {
       return reader;
     }
-    switch (typeValue) {
-    case 0:
-      return NullReader.INSTANCE;
-    case MinorType.MAP_VALUE:
+    // Warning: do not use valueOf as that uses Protobuf
+    // field numbers, not enum ordinals.
+    MinorType type = MinorType.values()[typeValue];
+    switch (type) {
+    case MAP:
       return (FieldReader) getMap();
-    case MinorType.DICT_VALUE:
+    case DICT:
       return (FieldReader) getDict();
-    case MinorType.LIST_VALUE:
+    case LIST:
       return (FieldReader) getList();
     <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
     <#assign uncappedName = name?uncap_first/>
     <#if !minor.class?starts_with("Decimal")>
-    case MinorType.${name?upper_case}_VALUE:
+    case ${name?upper_case}:
       return (FieldReader) get${name}();
     </#if>
     </#list></#list>
@@ -99,7 +102,7 @@
     if (mapReader == null) {
       mapReader = (SingleMapReaderImpl) data.getMap().getReader();
       mapReader.setPosition(idx());
-      readers[MinorType.MAP_VALUE] = mapReader;
+      readers[MinorType.MAP.ordinal()] = mapReader;
     }
     return mapReader;
   }
@@ -110,7 +113,7 @@
     if (dictReader == null) {
       dictReader = (SingleDictReaderImpl) data.getDict().getReader();
       dictReader.setPosition(idx());
-      readers[MinorType.DICT_VALUE] = dictReader;
+      readers[MinorType.DICT.ordinal()] = dictReader;
     }
     return dictReader;
   }
@@ -121,7 +124,7 @@
     if (listReader == null) {
       listReader = new UnionListReader(data.getList());
       listReader.setPosition(idx());
-      readers[MinorType.LIST_VALUE] = listReader;
+      readers[MinorType.LIST.ordinal()] = listReader;
     }
     return listReader;
   }
@@ -146,9 +149,7 @@
   public ${friendlyType} read${safeType}() {
     return getReaderForIndex(idx()).read${safeType}();
   }
-
   </#list>
-
   <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
           <#assign uncappedName = name?uncap_first/>
   <#assign boxedType = (minor.boxedType!type.boxedType) />
@@ -164,7 +165,7 @@
     if (${uncappedName}Reader == null) {
       ${uncappedName}Reader = new Nullable${name}ReaderImpl(data.get${name}Vector());
       ${uncappedName}Reader.setPosition(idx());
-      readers[MinorType.${name?upper_case}_VALUE] = ${uncappedName}Reader;
+      readers[MinorType.${name?upper_case}.ordinal()] = ${uncappedName}Reader;
     }
     return ${uncappedName}Reader;
   }
@@ -193,7 +194,7 @@
       }
     }
   }
-  
+
   public FieldReader reader(String name){
     return getMap().reader(name);
   }
@@ -205,7 +206,13 @@
   public boolean next() {
     return getReaderForIndex(idx()).next();
   }
+
+  @Override
+  public String getTypeString() {
+    if (isSet()) {
+      return getType().getMinorType().name();
+    } else {
+      return MinorType.NULL.name();
+    }
+  }
 }
-
-
-
diff --git a/exec/vector/src/main/codegen/templates/UnionVector.java b/exec/vector/src/main/codegen/templates/UnionVector.java
index ef83f4b..d4581cb 100644
--- a/exec/vector/src/main/codegen/templates/UnionVector.java
+++ b/exec/vector/src/main/codegen/templates/UnionVector.java
@@ -15,6 +15,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import org.apache.drill.common.types.Types;
 import org.apache.drill.common.types.TypeProtos.MinorType;
 import org.apache.drill.exec.vector.ValueVector;
 
@@ -62,23 +63,16 @@
   public static final int NULL_MARKER = 0;
   public static final String TYPE_VECTOR_NAME = "types";
   public static final String INTERNAL_MAP_NAME = "internal";
+  public static final int TYPE_COUNT = MinorType.values().length;
 
-  private static final MajorType MAJOR_TYPES[] = new MajorType[MinorType.values().length];
+  // Types must be indexed by ordinal, not by number. That is,
+  // use type.ordinal(), not type.getNumber().
+  public static final MajorType TYPES[] = new MajorType[TYPE_COUNT];
 
   static {
-    MAJOR_TYPES[MinorType.MAP.ordinal()] = Types.optional(MinorType.MAP);
-    MAJOR_TYPES[MinorType.LIST.ordinal()] = Types.optional(MinorType.LIST);
-    MAJOR_TYPES[MinorType.DICT.ordinal()] = Types.optional(MinorType.DICT);
-    <#list vv.types as type>
-      <#list type.minor as minor>
-        <#assign name = minor.class?cap_first />
-        <#assign fields = minor.fields!type.fields />
-        <#assign uncappedName = name?uncap_first/>
-        <#if !minor.class?starts_with("Decimal")>
-    MAJOR_TYPES[MinorType.${name?upper_case}.ordinal()] = Types.optional(MinorType.${name?upper_case});
-        </#if>
-      </#list>
-    </#list>
+    for (MinorType minorType : MinorType.values()) {
+      TYPES[minorType.ordinal()] = Types.optional(minorType);
+    }
   }
 
   private MaterializedField field;
@@ -194,7 +188,7 @@
   private <T extends ValueVector> T classicAddType(MinorType type, Class<? extends ValueVector> vectorClass) {
     int vectorCount = internalMap.size();
     @SuppressWarnings("unchecked")
-    T vector = (T) internalMap.addOrGet(type.name().toLowerCase(), MAJOR_TYPES[type.ordinal()], vectorClass);
+    T vector = (T) internalMap.addOrGet(type.name().toLowerCase(), TYPES[type.ordinal()], vectorClass);
     cachedSubtypes[type.ordinal()] = vector;
     if (internalMap.size() > vectorCount) {
       vector.allocateNew();
@@ -547,26 +541,30 @@
 
     @Override
     public Object getObject(int index) {
-      int type = getTypeVector().getAccessor().get(index);
-      switch (type) {
-      case NULL_MARKER:
+      int ordinal = getTypeVector().getAccessor().get(index);
+      if (ordinal == NULL_MARKER) {
         return null;
+      }
+      // Warning: do not use valueOf as that uses Protobuf
+      // field numbers, not enum ordinals.
+      MinorType type = MinorType.values()[ordinal];
+      switch (type) {
       <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
       <#assign fields = minor.fields!type.fields />
       <#assign uncappedName = name?uncap_first/>
       <#if !minor.class?starts_with("Decimal")>
-      case MinorType.${name?upper_case}_VALUE:
+      case ${name?upper_case}:
         return get${name}Vector().getAccessor().getObject(index);
       </#if>
       </#list></#list>
-      case MinorType.MAP_VALUE:
+      case MAP:
         return getMap().getAccessor().getObject(index);
-      case MinorType.LIST_VALUE:
+      case LIST:
         return getList().getAccessor().getObject(index);
-      case MinorType.DICT_VALUE:
+      case DICT:
         return getDict().getAccessor().getObject(index);
       default:
-        throw new UnsupportedOperationException("Cannot support type: " + MinorType.forNumber(type));
+        throw new UnsupportedOperationException("Cannot support type: " + type.name());
       }
     }
 
@@ -670,7 +668,7 @@
     </#list></#list>
 
     public void setType(int index, MinorType type) {
-      getTypeVector().getMutator().setSafe(index, type.getNumber());
+      getTypeVector().getMutator().setSafe(index, type.ordinal());
     }
 
     public void setNull(int index) {
diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/AbstractBaseReader.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/AbstractBaseReader.java
index b050005..faa4561 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/AbstractBaseReader.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/AbstractBaseReader.java
@@ -20,6 +20,7 @@
 import java.util.Iterator;
 
 import org.apache.drill.common.types.TypeProtos.MajorType;
+import org.apache.drill.common.types.TypeProtos.MinorType;
 import org.apache.drill.common.types.Types;
 import org.apache.drill.exec.expr.holders.UnionHolder;
 import org.apache.drill.exec.record.MaterializedField;
@@ -29,16 +30,11 @@
 import org.apache.drill.exec.vector.complex.writer.FieldWriter;
 
 
-abstract class AbstractBaseReader implements FieldReader{
-
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(AbstractBaseReader.class);
+abstract class AbstractBaseReader implements FieldReader {
 
   private int index;
 
-  public AbstractBaseReader() {
-    super();
-  }
-
+  @Override
   public void setPosition(int index) { this.index = index; }
 
   int idx() { return index; }
@@ -51,11 +47,15 @@
     throw new IllegalStateException("The current reader doesn't support reading as a map.");
   }
 
-  public MajorType getType(){
+  @Override
+  public MajorType getType() {
     throw new IllegalStateException("The current reader doesn't support getting type information.");
   }
 
   @Override
+  public MinorType getVectorType() { return getType().getMinorType(); }
+
+  @Override
   public MaterializedField getField() {
     return MaterializedField.create("unknown", Types.LATE_BIND_TYPE);
   }
diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleDictReaderImpl.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleDictReaderImpl.java
index 4378bcf..b116aae 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleDictReaderImpl.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleDictReaderImpl.java
@@ -18,6 +18,7 @@
 package org.apache.drill.exec.vector.complex.impl;
 
 import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.common.types.TypeProtos.MinorType;
 import org.apache.drill.exec.expr.fn.impl.DateUtility;
 import org.apache.drill.exec.expr.holders.ValueHolder;
 import org.apache.drill.exec.util.Text;
@@ -199,4 +200,8 @@
     }
     return sb.toString();
   }
+
+  public MinorType getVectorType() {
+    return vector.getField().getType().getMinorType();
+  }
 }
diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java
index 4dacb99..b9e567b 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java
@@ -26,15 +26,4 @@
 import org.apache.drill.exec.vector.complex.reader.BaseReader.ScalarReader;
 
 public interface FieldReader extends MapReader, DictReader, ListReader, ScalarReader, RepeatedMapReader, RepeatedListReader, UntypedReader {
-
-  /**
-   * Returns {@code String} representation of the reader's type. In case if {@link #getType()} is primitive,
-   * the method is equivalent to {@link #getType().getMinorType().name()}. If the reader has minor type equal to
-   * {@link org.apache.drill.common.types.TypeProtos.MinorType#DICT}, {@code DICT&lt;keyMinorType,valueMinorType&gt;},
-   * with {@code keyMinorType} and {@code valueMinorType} being key's and value's minor types respectively,
-   * will be returned. Used in {@code typeOf} UDF.
-   *
-   * @return {@code String} representation of reader's type.
-   */
-  String getTypeString();
 }
diff --git a/logical/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java b/logical/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java
index 30f6975..5d86d82 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java
@@ -21,31 +21,35 @@
 
 import org.apache.drill.common.expression.fn.FuncHolder;
 import org.apache.drill.common.expression.visitors.ExprVisitor;
-
 import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
-import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
 
+/**
+ * Represents an actual call (a reference) to a declared function.
+ * Holds the name used (functions can have multiple aliases), the
+ * function declaration, and the actual argument expressions used
+ * in this call. This might be better named
+ * <code>FunctionCallExpression</code> as it represents a use
+ * of a function. Subclasses hold references to the declaration
+ * depending on the type (Drill, Hive) of the function.
+ */
 public abstract class FunctionHolderExpression extends LogicalExpressionBase {
   public final ImmutableList<LogicalExpression> args;
   public final String nameUsed;
 
   /**
-   * A field reference identifies the output field and
-   * is used to reference that field in the generated classes.
+   * Identifies the output field. References that field in the
+   * generated classes.
    */
   private FieldReference fieldReference;
 
   public FunctionHolderExpression(String nameUsed, ExpressionPosition pos, List<LogicalExpression> args) {
     super(pos);
-    if (args == null) {
-      args = Lists.newArrayList();
-    }
-
-    if (!(args instanceof ImmutableList)) {
-      args = ImmutableList.copyOf(args);
-    }
-    this.args = (ImmutableList<LogicalExpression>) args;
     this.nameUsed = nameUsed;
+    if (args == null) {
+      this.args = ImmutableList.of();
+    } else {
+      this.args = ImmutableList.copyOf(args);
+    }
   }
 
   @Override
@@ -74,16 +78,17 @@
   public abstract boolean isAggregating();
 
   /**
-   * is the function output non-deterministic?
+   * Is the function output non-deterministic?
    */
   public abstract boolean isRandom();
 
   /**
-   * @ return a copy of FunctionHolderExpression, with passed in argument list.
+   * @return a copy of FunctionHolderExpression, with passed in argument list.
    */
   public abstract FunctionHolderExpression copy(List<LogicalExpression> args);
 
-  /** Return the underlying function implementation holder. */
+  /** Return the underlying function implementation holder. That is,
+   * returns the function declaration. */
   public abstract FuncHolder getHolder();
 
   public FieldReference getFieldReference() {
@@ -98,4 +103,21 @@
   public void setFieldReference(FieldReference fieldReference) {
     this.fieldReference = fieldReference;
   }
+
+  @Override
+  public String toString() {
+    StringBuilder buf = new StringBuilder()
+        .append("[").append(getClass().getSimpleName())
+        .append(", ")
+        .append(nameUsed).append("(");
+    boolean first = true;
+    for (LogicalExpression arg : args) {
+      if (!first) {
+        buf.append(", ");
+      }
+      first = false;
+      buf.append(arg.toString());
+    }
+    return buf.append("]").toString();
+  }
 }
diff --git a/logical/src/main/java/org/apache/drill/common/expression/LogicalExpression.java b/logical/src/main/java/org/apache/drill/common/expression/LogicalExpression.java
index 3cfbade..d073ee7 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/LogicalExpression.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/LogicalExpression.java
@@ -23,8 +23,6 @@
 import org.apache.drill.common.expression.visitors.ExprVisitor;
 import org.apache.drill.common.parser.LogicalExpressionParser;
 import org.apache.drill.common.types.TypeProtos.MajorType;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
@@ -35,8 +33,7 @@
 import com.fasterxml.jackson.databind.ser.std.StdSerializer;
 
 @JsonSerialize(using = LogicalExpression.Se.class)
-public interface LogicalExpression extends Iterable<LogicalExpression>{
-  Logger logger = LoggerFactory.getLogger(LogicalExpression.class);
+public interface LogicalExpression extends Iterable<LogicalExpression> {
 
   MajorType getMajorType();
 
@@ -66,7 +63,6 @@
 
       return LogicalExpressionParser.parse(expr);
     }
-
   }
 
   @SuppressWarnings("serial")
diff --git a/logical/src/main/java/org/apache/drill/common/expression/fn/FuncHolder.java b/logical/src/main/java/org/apache/drill/common/expression/fn/FuncHolder.java
index b00c4b4..3d8d0cc 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/fn/FuncHolder.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/fn/FuncHolder.java
@@ -24,22 +24,42 @@
 
 import java.util.List;
 
+/**
+ * Definition of a function as presented to code generation.
+ * Represents the common denominator between Drill and Hive
+ * functions.
+ */
 public interface FuncHolder {
 
   boolean isNested();
 
+  /**
+   * Return a reference to this function given a function alias and a
+   * list of actual arguments.
+   *
+   * @param name alias used in this specific call
+   * @param args expressions of the actual function arguments
+   * @param pos
+   * @return an expression that holds the function definition (this object),
+   * actual parameters and related information
+   */
   FunctionHolderExpression getExpr(String name, List<LogicalExpression> args, ExpressionPosition pos);
 
-  TypeProtos.MajorType getParamMajorType(int i);
-
+  /**
+   * Number of defined input parameters.
+   */
   int getParamCount();
 
   /**
+   * Drill SQL type of an input parameter.
+   */
+  TypeProtos.MajorType getParamMajorType(int i);
+
+  /**
    * Checks that the current function holder stores output value
    * using field writer instead of vector holder.
    *
    * @return true if current function holder uses field writer to store the output value
    */
   boolean isComplexWriterFuncHolder();
-
 }
diff --git a/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java b/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java
index 18483ce..788a8b3 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java
@@ -89,7 +89,6 @@
     return visitUnknown(intExpr, value);
   }
 
-
   @Override
   public T visitDecimal9Constant(Decimal9Expression decExpr, VAL value) throws EXCEP {
     return visitUnknown(decExpr, value);
@@ -180,9 +179,18 @@
     return visitUnknown(e, value);
   }
 
+  /**
+   * Handles implementation-specific expressions not known to the visitor
+   * structure. Since there are no "visitFoo" methods for these "unknown"
+   * expressions, subclassses should use the functionally-equivalent
+   * <code>instanceof</code> approach to parse out these "unknown"
+   * expressions.
+   */
   @Override
   public T visitUnknown(LogicalExpression e, VAL value) throws EXCEP {
-    throw new UnsupportedOperationException(String.format("Expression of type %s not handled by visitor type %s.", e.getClass().getCanonicalName(), this.getClass().getCanonicalName()));
+    throw new UnsupportedOperationException(String.format(
+        "Expression of type %s not handled by visitor type %s.",
+        e.getClass().getCanonicalName(), this.getClass().getCanonicalName()));
   }
 
   @Override
diff --git a/logical/src/main/java/org/apache/drill/common/expression/visitors/ConditionalExprOptimizer.java b/logical/src/main/java/org/apache/drill/common/expression/visitors/ConditionalExprOptimizer.java
index e228e64..330adf4 100644
--- a/logical/src/main/java/org/apache/drill/common/expression/visitors/ConditionalExprOptimizer.java
+++ b/logical/src/main/java/org/apache/drill/common/expression/visitors/ConditionalExprOptimizer.java
@@ -39,17 +39,13 @@
 
   @Override
   public LogicalExpression visitBooleanOperator(BooleanOperator op, Void value) throws RuntimeException {
-
     List<LogicalExpression> newArgs = Lists.newArrayList();
-
     newArgs.addAll(op.args);
-
     Collections.sort(newArgs, costComparator);
 
     return new BooleanOperator(op.getName(), newArgs, op.getPosition());
   }
 
-
   @Override
   public LogicalExpression visitFunctionHolderExpression(FunctionHolderExpression holder, Void value) throws RuntimeException {
     List<LogicalExpression> args = Lists.newArrayList();
@@ -59,8 +55,7 @@
       args.add(newExpr);
     }
 
-    //replace with a new function call, since its argument could be changed.
-
+    // Replace with a new function call, since its argument could be changed.
     return holder.copy(args);
   }
 
@@ -69,7 +64,6 @@
     return e;
   }
 
-
   @Override
   public LogicalExpression visitIfExpression(IfExpression ifExpr, Void value) throws RuntimeException{
     LogicalExpression newElseExpr = ifExpr.elseExpression.accept(this, value);
@@ -94,7 +88,6 @@
         + "It should have been converted to FunctionHolderExpression in materialization");
   }
 
-
   @Override
   public LogicalExpression visitConvertExpression(ConvertExpression cast, Void value) throws RuntimeException {
     throw new UnsupportedOperationException("ConvertExpression is not expected here. "
@@ -108,11 +101,9 @@
   }
 
   private static Comparator<LogicalExpression> costComparator = new Comparator<LogicalExpression> () {
+    @Override
     public int compare(LogicalExpression e1, LogicalExpression e2) {
       return e1.getCumulativeCost() <= e2.getCumulativeCost() ? -1 : 1;
     }
   };
-
-
-
 }
diff --git a/protocol/readme.txt b/protocol/readme.txt
index 8cfd668..cddbe55 100644
--- a/protocol/readme.txt
+++ b/protocol/readme.txt
@@ -8,6 +8,9 @@
 in your PATH (you may need to download and build it first). You can 

 download it from http://code.google.com/p/protobuf/downloads/list.

 

+Note: The Maven file has a dependence on exactly 3.6.1. Find it here:

+https://github.com/protocolbuffers/protobuf/releases/tag/v3.6.1

+

     Note: If generating sources on MAC follow below instructions:

 

               a) Download and install "brew"