drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ve...@apache.org
Subject [1/3] drill git commit: DRILL-1385, along with some cleanup
Date Mon, 16 Mar 2015 23:30:45 GMT
Repository: drill
Updated Branches:
  refs/heads/master 34932e126 -> 7ea212a26


http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderIden.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderIden.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderIden.java
index a0ce390..8b28629 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderIden.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderIden.java
@@ -25,38 +25,59 @@ import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.Type;
 
 import com.carrotsearch.hppc.ObjectIntOpenHashMap;
+import com.carrotsearch.hppc.cursors.ObjectIntCursor;
 import com.google.common.collect.Lists;
 
 class ValueHolderIden {
+//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ValueHolderIden.class);
 
-  final ObjectIntOpenHashMap<String> fieldMap;
-  final Type[] types;
-  final String[] names;
-  final Type type;
+  // the index of a field is the number in which it appears within the holder
+  private final ObjectIntOpenHashMap<String> fieldMap; // field name -> index
+  private final Type[] types; // the type of each field in the holder, by index
+  private final String[] names; // the name of each field in the holder, by index
+  private final int[] offsets; // the offset of each field in the holder, by index
+  private final Type type; // type of the holder itself
 
   public ValueHolderIden(Class<?> c) {
     Field[] fields = c.getFields();
 
+    // Find the non-static member variables
     List<Field> fldList = Lists.newArrayList();
     for (Field f : fields) {
       if (!Modifier.isStatic(f.getModifiers())) {
         fldList.add(f);
       }
     }
+
     this.type = Type.getType(c);
+
     this.types = new Type[fldList.size()];
     this.names = new String[fldList.size()];
-    fieldMap = new ObjectIntOpenHashMap<String>();
-    int i =0;
+    this.offsets = new int[fldList.size()];
+    fieldMap = new ObjectIntOpenHashMap<String>(fldList.size(), 1.0f);
+    int i = 0; // index of the next holder member variable
+    int offset = 0; // offset of the next holder member variable
     for (Field f : fldList) {
       types[i] = Type.getType(f.getType());
       names[i] = f.getName();
+      offsets[i] = offset;
       fieldMap.put(f.getName(), i);
+
+      // the next offset and index
+      offset += types[i].getSize();
       i++;
     }
   }
 
-  private static void initType(int index, Type t, DirectSorter v) {
+  public void dump(final StringBuilder sb) {
+    sb.append("ValueHolderIden: type=" + type + '\n');
+    for (ObjectIntCursor<String> oic : fieldMap) {
+      sb.append("  " + oic.key + ": i=" + oic.value + ", type=" + types[oic.value] +
+          ", offset=" + offsets[oic.value] + '\n');
+    }
+  }
+
+  private static void initType(int offset, Type t, DirectSorter v) {
     switch(t.getSort()) {
     case Type.BOOLEAN:
     case Type.BYTE:
@@ -64,38 +85,51 @@ class ValueHolderIden {
     case Type.SHORT:
     case Type.INT:
       v.visitInsn(Opcodes.ICONST_0);
-      v.directVarInsn(Opcodes.ISTORE, index);
+      v.directVarInsn(Opcodes.ISTORE, offset);
       break;
     case Type.LONG:
       v.visitInsn(Opcodes.LCONST_0);
-      v.directVarInsn(Opcodes.LSTORE, index);
+      v.directVarInsn(Opcodes.LSTORE, offset);
       break;
     case Type.FLOAT:
       v.visitInsn(Opcodes.FCONST_0);
-      v.directVarInsn(Opcodes.FSTORE, index);
+      v.directVarInsn(Opcodes.FSTORE, offset);
       break;
     case Type.DOUBLE:
       v.visitInsn(Opcodes.DCONST_0);
-      v.directVarInsn(Opcodes.DSTORE, index);
+      v.directVarInsn(Opcodes.DSTORE, offset);
       break;
     case Type.OBJECT:
       v.visitInsn(Opcodes.ACONST_NULL);
-      v.directVarInsn(Opcodes.ASTORE, index);
+      v.directVarInsn(Opcodes.ASTORE, offset);
       break;
     default:
       throw new UnsupportedOperationException();
     }
   }
 
-  public ValueHolderSub getHolderSub(DirectSorter adder) {
-    int first = -1;
+  /**
+   * Add the locals necessary to replace the members of a holder of this type.
+   *
+   * @param adder the method visitor to use to add the necessary instructions
+   * @param defaultFirst the default index to return for the first variable
+   *   if we don't find another one
+   * @return the index of the first local variable (standing in for the first holder member)
+   */
+  private int addLocals(final DirectSorter adder, final int defaultFirst) {
+    int first = defaultFirst;
     for (int i = 0; i < types.length; i++) {
       int varIndex = adder.newLocal(types[i]);
       if (i == 0) {
-        first = varIndex;
+        first = varIndex; // first offset
       }
     }
 
+    return first;
+  }
+
+  public ValueHolderSub getHolderSub(DirectSorter adder) {
+    final int first = addLocals(adder, -1);
     return new ValueHolderSub(first);
   }
 
@@ -103,36 +137,56 @@ class ValueHolderIden {
     return new ValueHolderSub(first);
   }
 
-  private int dup(Type t) {
-    return t.getSize() == 1 ? Opcodes.DUP : Opcodes.DUP2;
+  /**
+   * Return the DUP or DUP2 opcode appropriate for the given type.
+   *
+   * @param type the type
+   * @return the DUP/DUP2 opcode to use for type
+   */
+  private static int getDupOpcode(final Type type) {
+    assert type.getSize() >= 1;
+    return type.getSize() == 1 ? Opcodes.DUP : Opcodes.DUP2;
   }
 
-  public void transferToLocal(DirectSorter adder, int localVariable) {
+  /**
+   * Transfer the member variables of this holder to local variables.
+   *
+   * <p>If this is used, the maximum stack size must be increased by one
+   * to accommodate the extra DUP instruction this will generate.
+   *
+   * @param adder a visitor that will be called to add the necessary instructions
+   * @param localVariable the offset of the first local variable to use
+   */
+  public void transferToLocal(final DirectSorter adder, final int localVariable) {
     for (int i = 0; i < types.length; i++) {
-      Type t = types[i];
+      final Type t = types[i];
       if (i + 1 < types.length) {
-        adder.visitInsn(dup(t)); // don't dup for last value.
-      }
+        adder.visitInsn(Opcodes.DUP); // not size dependent: always the objectref from which we'll GETFIELD
+        }
       adder.visitFieldInsn(Opcodes.GETFIELD, type.getInternalName(), names[i], t.getDescriptor());
-      adder.directVarInsn(t.getOpcode(Opcodes.ISTORE), localVariable+i);
+      adder.directVarInsn(t.getOpcode(Opcodes.ISTORE), localVariable + offsets[i]);
     }
   }
 
-  public int createLocalAndTrasfer(DirectSorter adder) {
-    int first = 0;
-    for (int i = 0; i < types.length; i++) {
-      Type t = types[i];
-      int varIndex = adder.newLocal(t);
-      if (i == 0) {
-        first = varIndex;
-      }
-    }
+  /**
+   * Create local variables and transfer the members of a holder to them.
+   *
+   * @param adder the method visitor to use to add the variables
+   * @return the index of the first variable added
+   */
+  public int createLocalAndTransfer(final DirectSorter adder) {
+    final int first = addLocals(adder, 0);
     transferToLocal(adder, first);
     return first;
   }
 
   public class ValueHolderSub {
-    private int first;
+    private int first; // TODO: deal with transfer() so this can be made final
+
+    @Override
+    public String toString() {
+      return "ValueHolderSub(" + first + ")";
+    }
 
     public ValueHolderSub(int first) {
       assert first != -1 : "Create Holder for sub that doesn't have any fields.";
@@ -145,26 +199,20 @@ class ValueHolderIden {
 
     public void init(DirectSorter mv) {
       for (int i = 0; i < types.length; i++) {
-        initType(first+i, types[i], mv);
+        initType(first + offsets[i], types[i], mv);
       }
     }
-    public int size() {
-      return types.length;
-    }
 
     public int first() {
       return first;
     }
 
-    public void updateFirst(int newFirst) {
-      this.first = newFirst;
-    }
-
-    private int field(String name, InstructionModifier mv) {
+    private int getFieldIndex(final String name, final InstructionModifier mv) {
       if (!fieldMap.containsKey(name)) {
-        throw new IllegalArgumentException(String.format("Unknown name '%s' on line %d.", name, mv.lastLineNumber));
+        throw new IllegalArgumentException(String.format(
+            "Unknown name '%s' on line %d.", name, mv.getLastLineNumber()));
       }
-      return fieldMap.lget();
+      return fieldMap.get(name); // using lget() is not thread-safe
     }
 
     public void addInsn(String name, InstructionModifier mv, int opcode) {
@@ -178,44 +226,71 @@ class ValueHolderIden {
       }
     }
 
+    // TODO: do we really need this? Instead of moving the variables, can't we just
+    // use the original locations in the subsequent references?
     public void transfer(InstructionModifier mv, int newStart) {
+      // if the new location is the same as the current position, there's nothing to do
       if (first == newStart) {
         return;
       }
-      for (int i =0; i < types.length; i++) {
-        mv.directVarInsn(types[i].getOpcode(Opcodes.ILOAD), first + i);
-        mv.directVarInsn(types[i].getOpcode(Opcodes.ISTORE), newStart + i);
+
+      for (int i = 0; i < types.length; i++) {
+        mv.directVarInsn(types[i].getOpcode(Opcodes.ILOAD), first + offsets[i]);
+        mv.directVarInsn(types[i].getOpcode(Opcodes.ISTORE), newStart + offsets[i]);
       }
+
       this.first = newStart;
     }
 
     private void addKnownInsn(String name, InstructionModifier mv, int analogOpcode) {
-      int f = field(name, mv);
+      int f = getFieldIndex(name, mv);
       Type t = types[f];
-      mv.directVarInsn(t.getOpcode(analogOpcode), first + f);
+      mv.directVarInsn(t.getOpcode(analogOpcode), first + offsets[f]);
     }
 
-    public void transferToExternal(DirectSorter adder, String owner, String name, String desc) {
-
+    /**
+     *
+     * @param adder
+     * @param owner
+     * @param name
+     * @param desc
+     * @return amount of additional stack space that will be required for this instruction stream
+     */
+    public int transferToExternal(final DirectSorter adder, final String owner,
+        final String name, final String desc) {
       // create a new object and assign it to the desired field.
       adder.visitTypeInsn(Opcodes.NEW, type.getInternalName());
-      adder.visitInsn(dup(type));
-      adder.visitMethodInsn(Opcodes.INVOKESPECIAL, type.getInternalName(), "<init>", "()V");
+      adder.visitInsn(getDupOpcode(type));
+      adder.visitMethodInsn(Opcodes.INVOKESPECIAL, type.getInternalName(), "<init>", "()V", false);
 
       // now we need to set all of the values of this new object.
+      int additionalStack = 0;
       for (int i = 0; i < types.length; i++) {
-        Type t = types[i];
+        final Type t = types[i];
 
         // dup the object where we are putting the field.
-        adder.visitInsn(dup(type)); // dup for every as we want to save in place at end.
-        adder.directVarInsn(t.getOpcode(Opcodes.ILOAD), first+i);
+        adder.visitInsn(getDupOpcode(type)); // dup for every as we want to save in place at end.
+        adder.directVarInsn(t.getOpcode(Opcodes.ILOAD), first + offsets[i]);
         adder.visitFieldInsn(Opcodes.PUTFIELD, type.getInternalName(), names[i], t.getDescriptor());
+
+        /*
+         * The above substitutes a reference to a scalar in a holder with a direct reference to
+         * the scalar.
+         *
+         * In the case of longs or doubles, this requires more stack space than was used before;
+         * if we were moving a reference to a holder with a long, we were previously moving the
+         * reference. But now we're moving the long, which is twice as big. So we may need more
+         * stack space than has currently been allocated.
+         */
+        if (t.getSize() > additionalStack) {
+          additionalStack = t.getSize();
+        }
       }
 
       // lastly we save it to the desired field.
       adder.visitFieldInsn(Opcodes.PUTFIELD, owner, name, desc);
-    }
 
+      return additionalStack;
+    }
   }
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java
index 2cec537..9dee164 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/ValueHolderReplacementVisitor.java
@@ -20,44 +20,65 @@ package org.apache.drill.exec.compile.bytecode;
 import java.io.PrintWriter;
 
 import org.apache.commons.io.output.StringBuilderWriter;
+import org.apache.drill.exec.compile.CheckMethodVisitorFsm;
+import org.apache.drill.exec.compile.CompilationConfig;
 import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.tree.MethodNode;
 import org.objectweb.asm.util.Textifier;
 import org.objectweb.asm.util.TraceMethodVisitor;
 
 public class ValueHolderReplacementVisitor extends ClassVisitor {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ValueHolderReplacementVisitor.class);
+  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ValueHolderReplacementVisitor.class);
+  private final boolean verifyBytecode;
+  private String className; // fully qualified internal class name
 
-  public ValueHolderReplacementVisitor(ClassVisitor cw) {
-    super(Opcodes.ASM4, cw);
+  public ValueHolderReplacementVisitor(ClassVisitor cw, boolean verifyBytecode) {
+    super(CompilationConfig.ASM_API_VERSION, cw);
+    this.verifyBytecode = verifyBytecode;
   }
 
+  @Override
+  public void visit(final int version, final int access, final String name, final String signature,
+      final String superName, final String[] interfaces) {
+    // capture the name of the class for later analysis
+    className = name;
 
+    super.visit(version, access, name, signature, superName, interfaces);
+  }
 
   @Override
   public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
     MethodVisitor innerVisitor = super.visitMethod(access, name, desc, signature, exceptions);
 //     innerVisitor = new Debugger(access, name, desc, signature, exceptions, innerVisitor);
-    return new ScalarReplacementNode(access, name, desc, signature, exceptions, innerVisitor);
+    if (verifyBytecode) {
+      innerVisitor = new CheckMethodVisitorFsm(api, innerVisitor);
+    }
+
+    /*
+     * Before using the ScalarReplacementNode to rewrite method code, use the
+     * AloadPopRemover to eliminate unnecessary ALOAD-POP pairs; see the
+     * AloadPopRemover javadoc for a detailed explanation.
+     */
+    return new AloadPopRemover(api,
+        new ScalarReplacementNode(
+            className, access, name, desc, signature, exceptions, innerVisitor, verifyBytecode));
   }
 
   private static class Debugger extends MethodNode {
-
     MethodVisitor inner;
 
     public Debugger(int access, String name, String desc, String signature, String[] exceptions, MethodVisitor inner) {
       super(access, name, desc, signature, exceptions);
       this.inner = inner;
-
     }
 
     @Override
     public void visitEnd() {
-      try{
+      try {
         accept(inner);
-      }catch(Exception e){
+        super.visitEnd();
+      } catch(Exception e){
         Textifier t = new Textifier();
         accept(new TraceMethodVisitor(t));
         StringBuilderWriter sw = new StringBuilderWriter();
@@ -68,8 +89,6 @@ public class ValueHolderReplacementVisitor extends ClassVisitor {
         logger.error(String.format("Failure while rendering method %s, %s, %s.  ByteCode:\n %s", name, desc, signature, bytecode), e);
         throw new RuntimeException(String.format("Failure while rendering method %s, %s, %s.  ByteCode:\n %s", name, desc, signature, bytecode), e);
       }
-
     }
-
   }
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/GeneratorMapping.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/GeneratorMapping.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/GeneratorMapping.java
index 57551ed..9c12116 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/GeneratorMapping.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/GeneratorMapping.java
@@ -24,13 +24,12 @@ import com.google.common.base.Preconditions;
 public class GeneratorMapping {
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(GeneratorMapping.class);
 
-  private String setup;
-  private String eval;
-  private String reset;
-  private String cleanup;
+  private final String setup;
+  private final String eval;
+  private final String reset;
+  private final String cleanup;
 
-
-  public GeneratorMapping(String setup, String eval, String reset, String cleanup) {
+  public GeneratorMapping(final String setup, final String eval, final String reset, final String cleanup) {
     super();
     this.setup = setup;
     this.eval = eval;
@@ -38,7 +37,7 @@ public class GeneratorMapping {
     this.cleanup = cleanup;
   }
 
-  public GeneratorMapping(GeneratorMapping gm) {
+  public GeneratorMapping(final GeneratorMapping gm) {
     super();
     this.setup = gm.setup;
     this.eval = gm.eval;
@@ -46,20 +45,22 @@ public class GeneratorMapping {
     this.cleanup = gm.cleanup;
   }
 
-  public static GeneratorMapping GM(String setup, String eval){
+  public static GeneratorMapping GM(final String setup, final String eval) {
     return create(setup, eval, null, null);
   }
 
-  public static GeneratorMapping GM(String setup, String eval, String reset, String cleanup){
+  public static GeneratorMapping GM(
+      final String setup, final String eval, final String reset, final String cleanup) {
     return create(setup, eval, reset, cleanup);
   }
 
-  public static GeneratorMapping create(String setup, String eval, String reset, String cleanup){
+  public static GeneratorMapping create(
+      final String setup, final String eval, final String reset, final String cleanup) {
     return new GeneratorMapping(setup, eval, reset, cleanup);
   }
 
-  public String getMethodName(BlockType type){
-    switch(type){
+  public String getMethodName(final BlockType type) {
+    switch(type) {
     case CLEANUP:
       Preconditions.checkNotNull(cleanup, "The current mapping does not have a cleanup method defined.");
       return cleanup;
@@ -76,6 +77,4 @@ public class GeneratorMapping {
       throw new IllegalStateException();
     }
   }
-
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
index 00aaec6..fafc286 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
@@ -87,6 +87,10 @@ public class DrillFunctionRegistry {
     }
   }
 
+  public int size(){
+    return methods.size();
+  }
+
   /** Returns functions with given name. Function name is case insensitive. */
   public List<DrillFuncHolder> getMethods(String name) {
     return this.methods.get(name.toLowerCase());

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
index 55e4121..e96fa60 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
@@ -21,6 +21,7 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.drill.common.config.DrillConfig;
 import org.apache.drill.common.expression.FunctionCall;
@@ -32,6 +33,7 @@ import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.planner.sql.DrillOperatorTable;
 import org.apache.drill.exec.resolver.FunctionResolver;
 
+import com.google.common.base.Stopwatch;
 import com.google.common.collect.Lists;
 import org.apache.drill.exec.server.options.OptionManager;
 
@@ -43,6 +45,9 @@ public class FunctionImplementationRegistry {
   private OptionManager optionManager = null;
 
   public FunctionImplementationRegistry(DrillConfig config){
+    Stopwatch w = new Stopwatch().start();
+
+    logger.debug("Generating Function Registry.");
     drillFuncRegistry = new DrillFunctionRegistry(config);
 
     Set<Class<? extends PluggableFunctionRegistry>> registryClasses = PathScanner.scanForImplementations(
@@ -67,6 +72,7 @@ public class FunctionImplementationRegistry {
         break;
       }
     }
+    logger.debug("Function registry loaded.  {} functions loaded in {}ms.", drillFuncRegistry.size(), w.elapsed(TimeUnit.MILLISECONDS));
   }
 
   public FunctionImplementationRegistry(DrillConfig config, OptionManager optionManager) {

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
index c03d6a0..a23780e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
@@ -73,7 +73,7 @@ import com.sun.codemodel.JConditional;
 import com.sun.codemodel.JExpr;
 
 public class ExternalSortBatch extends AbstractRecordBatch<ExternalSort> {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ExternalSortBatch.class);
+  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ExternalSortBatch.class);
 
   private static final long MAX_SORT_BYTES = 1L * 1024 * 1024 * 1024;
   private static final GeneratorMapping COPIER_MAPPING = new GeneratorMapping("doSetup", "doCopy", null, null);

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java
index 8bf346d..fc08f05 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java
@@ -81,7 +81,7 @@ public class SchemaBuilder {
 //                .format(
 //                    "You attempted to add a field for Id An attempt was made to add a duplicate fieldId to the schema.  The offending fieldId was %d",
 //                    fieldId));
-//      f.checkMaterialization(expectedFields.lget());
+//      f.checkMaterialization(expectedFields.lget()); // TODO: lget is not safe if expectedFields is shared
 //    }
 //    fields.put(f.getFieldId(), f);
 //  }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
index 67342c4..6521303 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
@@ -26,6 +26,9 @@ import org.apache.drill.exec.coord.ClusterCoordinator.RegistrationHandle;
 import org.apache.drill.exec.coord.zk.ZKClusterCoordinator;
 import org.apache.drill.exec.exception.DrillbitStartupException;
 import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.server.options.OptionManager;
+import org.apache.drill.exec.server.options.OptionValue;
+import org.apache.drill.exec.server.options.OptionValue.OptionType;
 import org.apache.drill.exec.server.rest.DrillRestServer;
 import org.apache.drill.exec.service.ServiceEngine;
 import org.apache.drill.exec.store.sys.CachingStoreProvider;
@@ -50,7 +53,7 @@ import com.google.common.io.Closeables;
  * Starts, tracks and stops all the required services for a Drillbit daemon to work.
  */
 public class Drillbit implements Closeable{
-  static final org.slf4j.Logger logger;
+  private static final org.slf4j.Logger logger;
   static {
     logger = org.slf4j.LoggerFactory.getLogger(Drillbit.class);
     Environment.logEnv("Drillbit environment:.", logger);
@@ -78,6 +81,77 @@ public class Drillbit implements Closeable{
     return bit;
   }
 
+  private final static String SYSTEM_OPTIONS_NAME = "org.apache.drill.exec.server.Drillbit.system_options";
+
+  private static void throwInvalidSystemOption(final String systemProp, final String errorMessage) {
+    throw new IllegalStateException("Property \"" + SYSTEM_OPTIONS_NAME + "\" part \"" + systemProp
+        + "\" " + errorMessage + ".");
+  }
+
+  private static String stripQuotes(final String s, final String systemProp) {
+    if (s.isEmpty()) {
+      return s;
+    }
+
+    final char cFirst = s.charAt(0);
+    final char cLast = s.charAt(s.length() - 1);
+    if ((cFirst == '"') || (cFirst == '\'')) {
+      if (cLast != cFirst) {
+        throwInvalidSystemOption(systemProp, "quoted value does not have closing quote");
+      }
+
+      return s.substring(1, s.length() - 2); // strip the quotes
+    }
+
+    if ((cLast == '"') || (cLast == '\'')) {
+        throwInvalidSystemOption(systemProp, "value has unbalanced closing quote");
+    }
+
+    // return as-is
+    return s;
+  }
+
+  private void javaPropertiesToSystemOptions() {
+    // get the system options property
+    final String allSystemProps = System.getProperty(SYSTEM_OPTIONS_NAME);
+    if ((allSystemProps == null) || allSystemProps.isEmpty()) {
+      return;
+    }
+
+    final OptionManager optionManager = getContext().getOptionManager();
+
+    // parse out the properties, validate, and then set them
+    final String systemProps[] = allSystemProps.split(",");
+    for(String systemProp : systemProps) {
+      final String keyValue[] = systemProp.split("=");
+      if (keyValue.length != 2) {
+        throwInvalidSystemOption(systemProp, "does not contain a key=value assignment");
+      }
+
+      final String optionName = keyValue[0].trim();
+      if (optionName.isEmpty()) {
+        throwInvalidSystemOption(systemProp, "does not contain a key before the assignment");
+      }
+
+      final String optionString = stripQuotes(keyValue[1].trim(), systemProp);
+      if (optionString.isEmpty()) {
+        throwInvalidSystemOption(systemProp, "does not contain a value after the assignment");
+      }
+
+      final OptionValue defaultValue = optionManager.getOption(optionName);
+      if (defaultValue == null) {
+        throwInvalidSystemOption(systemProp, "does not specify a valid option name");
+      }
+      if (defaultValue.type != OptionType.SYSTEM) {
+        throwInvalidSystemOption(systemProp, "does not specify a SYSTEM option ");
+      }
+
+      final OptionValue optionValue = OptionValue.createOption(
+          defaultValue.kind, OptionType.SYSTEM, optionName, optionString);
+      optionManager.setOption(optionValue);
+    }
+  }
+
   public static void main(String[] cli) throws DrillbitStartupException {
     StartupOptions options = StartupOptions.parse(cli);
     start(options);
@@ -154,6 +228,7 @@ public class Drillbit implements Closeable{
     manager.start(md, engine.getController(), engine.getDataConnectionCreator(), coord, storeProvider);
     manager.getContext().getStorage().init();
     manager.getContext().getOptionManager().init();
+    javaPropertiesToSystemOptions();
     handle = coord.register(md);
     startJetty();
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java
index 0fb10ff..b0b478e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java
@@ -41,7 +41,7 @@ import com.codahale.metrics.MetricRegistry;
 import com.google.common.base.Preconditions;
 
 public class DrillbitContext {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillbitContext.class);
+//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillbitContext.class);
 
   private final BootStrapContext context;
 
@@ -154,7 +154,4 @@ public class DrillbitContext {
   public CodeCompiler getCompiler() {
     return compiler;
   }
-
-
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/DrillConfigIterator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/DrillConfigIterator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/DrillConfigIterator.java
index e4b03d3..5c2aede 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/DrillConfigIterator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/DrillConfigIterator.java
@@ -21,7 +21,6 @@ import java.util.Iterator;
 import java.util.Map.Entry;
 
 import org.apache.drill.common.config.DrillConfig;
-import org.apache.drill.exec.server.options.OptionValue.Kind;
 import org.apache.drill.exec.server.options.OptionValue.OptionType;
 
 import com.typesafe.config.ConfigValue;
@@ -52,27 +51,34 @@ public class DrillConfigIterator implements Iterable<OptionValue> {
 
     @Override
     public OptionValue next() {
-      Entry<String, ConfigValue> e = entries.next();
-      OptionValue v = new OptionValue();
-      v.name = e.getKey();
-      ConfigValue cv = e.getValue();
-      v.type = OptionType.BOOT;
-      switch(cv.valueType()){
+      final Entry<String, ConfigValue> e = entries.next();
+      final ConfigValue cv = e.getValue();
+      final String name = e.getKey();
+      OptionValue optionValue = null;
+      switch(cv.valueType()) {
       case BOOLEAN:
-        v.kind = Kind.BOOLEAN;
-        v.bool_val = (Boolean) cv.unwrapped();
+        optionValue = OptionValue.createBoolean(OptionType.BOOT, name, (Boolean) cv.unwrapped());
         break;
+
       case LIST:
       case OBJECT:
       case STRING:
-        v.string_val = cv.render();
+        optionValue = OptionValue.createString(OptionType.BOOT, name, cv.render());
         break;
+
       case NUMBER:
-        v.kind = Kind.LONG;
-        v.num_val = ((Number)cv.unwrapped()).longValue();
+        optionValue = OptionValue.createLong(OptionType.BOOT, name, ((Number) cv.unwrapped()).longValue());
         break;
+
+      case NULL:
+        throw new IllegalStateException("Config value \"" + name + "\" has NULL type");
+/* TODO(cwestin)
+        optionValue = OptionValue.createOption(kind, OptionType.BOOT, name, "");
+        break;
+*/
       }
-      return v;
+
+      return optionValue;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
index f515f8e..45d393c 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java
@@ -18,21 +18,18 @@
 package org.apache.drill.exec.server.options;
 
 import java.util.Iterator;
-import java.util.Map;
 
 import org.apache.drill.exec.server.options.OptionValue.OptionType;
 import org.eigenbase.sql.SqlLiteral;
 
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Maps;
 
-public abstract class FallbackOptionManager implements OptionManager{
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FallbackOptionManager.class);
+public abstract class FallbackOptionManager implements OptionManager {
+//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FallbackOptionManager.class);
 
-  protected OptionManager fallback;
+  protected final OptionManager fallback;
 
   public FallbackOptionManager(OptionManager fallback) {
-    super();
     this.fallback = fallback;
   }
 
@@ -43,7 +40,7 @@ public abstract class FallbackOptionManager implements OptionManager{
 
   @Override
   public OptionValue getOption(String name) {
-    OptionValue opt = getLocalOption(name);
+    final OptionValue opt = getLocalOption(name);
     if(opt == null && fallback != null){
       return fallback.getOption(name);
     }else{
@@ -62,9 +59,8 @@ public abstract class FallbackOptionManager implements OptionManager{
   }
 
   @Override
-  public void setOption(String name, SqlLiteral literal, OptionType type) {
-    OptionValue val = getAdmin().validate(name, literal);
-    val.type = type;
+  public void setOption(String name, SqlLiteral literal, OptionType optionType) {
+    final OptionValue val = getAdmin().validate(name, literal, optionType);
     setValidatedOption(val);
   }
 
@@ -89,11 +85,10 @@ public abstract class FallbackOptionManager implements OptionManager{
 
   @Override
   public OptionList getOptionList() {
-    OptionList list = new OptionList();
+    final OptionList list = new OptionList();
     for (OptionValue o : optionIterable()) {
       list.add(o);
     }
     return list;
   }
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java
index 83af7db..4ffe9a3 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java
@@ -17,9 +17,10 @@
  */
 package org.apache.drill.exec.server.options;
 
+import org.apache.drill.exec.server.options.OptionValue.OptionType;
 import org.eigenbase.sql.SqlLiteral;
 
-public interface OptionManager extends Iterable<OptionValue>{
+public interface OptionManager extends Iterable<OptionValue> {
   public OptionValue getOption(String name);
   public void setOption(OptionValue value) throws SetOptionException;
   public void setOption(String name, SqlLiteral literal, OptionValue.OptionType type) throws SetOptionException;
@@ -27,11 +28,9 @@ public interface OptionManager extends Iterable<OptionValue>{
   public OptionManager getSystemManager();
   public OptionList getOptionList();
 
-  public interface OptionAdmin{
+  public interface OptionAdmin {
     public void registerOptionType(OptionValidator validator);
     public void validate(OptionValue v) throws SetOptionException;
-    public OptionValue validate(String name, SqlLiteral value) throws SetOptionException;
+    public OptionValue validate(String name, SqlLiteral value, OptionType optionType) throws SetOptionException;
   }
-
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java
index 5b90ba5..43071e7 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java
@@ -17,8 +17,8 @@
  ******************************************************************************/
 package org.apache.drill.exec.server.options;
 
-
 import org.apache.drill.common.exceptions.ExpressionParsingException;
+import org.apache.drill.exec.server.options.OptionValue.OptionType;
 import org.eigenbase.sql.SqlLiteral;
 
 /**
@@ -27,12 +27,11 @@ import org.eigenbase.sql.SqlLiteral;
  * @param <E>
  */
 public abstract class OptionValidator {
-
   // Stored here as well as in the option static class to allow insertion of option optionName into
   // the error messages produced by the validator
-  private String optionName;
+  private final String optionName;
 
-  public OptionValidator(String optionName){
+  public OptionValidator(String optionName) {
     this.optionName = optionName;
   }
 
@@ -48,17 +47,16 @@ public abstract class OptionValidator {
    *            while allowing some flexibility for users
    * @throws ExpressionParsingException - message to describe error with value, including range or list of expected values
    */
-  public abstract OptionValue validate(SqlLiteral value) throws ExpressionParsingException;
+  public abstract OptionValue validate(SqlLiteral value, OptionType optionType) throws ExpressionParsingException;
 
   public String getOptionName() {
     return optionName;
   }
 
-  public String getDefaultString(){
+  public String getDefaultString() {
     return null;
   }
 
-
   public abstract OptionValue getDefault();
 
   public abstract void validate(OptionValue v) throws ExpressionParsingException;

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java
index bfcbeca..8735fd6 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java
@@ -23,7 +23,7 @@ import com.fasterxml.jackson.annotation.JsonInclude.Include;
 import com.google.common.base.Preconditions;
 
 @JsonInclude(Include.NON_NULL)
-public class OptionValue{
+public class OptionValue {
 
   public static enum OptionType {
     BOOT, SYSTEM, SESSION, QUERY
@@ -41,6 +41,15 @@ public class OptionValue{
   public Boolean bool_val;
   public Double float_val;
 
+  /**
+   * Constructor.
+   *
+   * <p>Unfortunately, we have to have this variant, and are unable to make the members final,
+   * in order to support JSON deserialization into this structure.
+   */
+  public OptionValue() {
+  }
+
   public static OptionValue createLong(OptionType type, String name, long val) {
     return new OptionValue(Kind.LONG, type, name, val, null, null, null);
   }
@@ -57,8 +66,6 @@ public class OptionValue{
     return new OptionValue(Kind.DOUBLE, type, name, null, null, null, val);
   }
 
-  public OptionValue() {}
-
   public static OptionValue createOption(Kind kind, OptionType type, String name, String val) {
     switch (kind) {
       case BOOLEAN:
@@ -73,18 +80,17 @@ public class OptionValue{
     return null;
   }
 
-
-  private OptionValue(Kind kind, OptionType type, String name, Long num_val, String string_val, Boolean bool_val, Double float_val) {
-    super();
+  private OptionValue(Kind kind, OptionType type, String name,
+      Long num_val, String string_val, Boolean bool_val, Double float_val) {
     Preconditions.checkArgument(num_val != null || string_val != null || bool_val != null || float_val != null);
-    this.name = name;
     this.kind = kind;
-    this.float_val = float_val;
     this.type = type;
+    this.name = name;
+
+    this.float_val = float_val;
     this.num_val = num_val;
     this.string_val = string_val;
     this.bool_val = bool_val;
-    this.type = type;
   }
 
   @JsonIgnore
@@ -176,5 +182,4 @@ public class OptionValue{
   public String toString() {
     return "OptionValue [type=" + type + ", name=" + name + ", value=" + getValue() + "]";
   }
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
index d821af8..c5d4656 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
@@ -26,6 +26,7 @@ import java.util.concurrent.ConcurrentMap;
 import org.apache.commons.collections.IteratorUtils;
 import org.apache.drill.common.config.DrillConfig;
 import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.compile.ClassTransformer;
 import org.apache.drill.exec.compile.QueryClassLoader;
 import org.apache.drill.exec.planner.physical.PlannerSettings;
 import org.apache.drill.exec.server.options.OptionValue.OptionType;
@@ -37,10 +38,9 @@ import org.eigenbase.sql.SqlLiteral;
 import com.google.common.collect.Maps;
 
 public class SystemOptionManager implements OptionManager {
+  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(SystemOptionManager.class);
 
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(SystemOptionManager.class);
-
-  private final OptionValidator[] VALIDATORS = {
+  private static final OptionValidator[] VALIDATORS = {
       PlannerSettings.EXCHANGE,
       PlannerSettings.HASHAGG,
       PlannerSettings.STREAMAGG,
@@ -91,79 +91,77 @@ public class SystemOptionManager implements OptionManager {
       QueryClassLoader.JAVA_COMPILER_JANINO_MAXSIZE,
       QueryClassLoader.JAVA_COMPILER_DEBUG,
       ExecConstants.ENABLE_VERBOSE_ERRORS,
-      ExecConstants.ENABLE_WINDOW_FUNCTIONS_VALIDATOR
+      ExecConstants.ENABLE_WINDOW_FUNCTIONS_VALIDATOR,
+      ClassTransformer.SCALAR_REPLACEMENT_VALIDATOR,
   };
 
-  public final PStoreConfig<OptionValue> config;
-
+  private final PStoreConfig<OptionValue> config;
   private PStore<OptionValue> options;
   private SystemOptionAdmin admin;
   private final ConcurrentMap<String, OptionValidator> knownOptions = Maps.newConcurrentMap();
   private final PStoreProvider provider;
 
-  public SystemOptionManager(DrillConfig config, PStoreProvider provider) {
+  public SystemOptionManager(final DrillConfig config, final PStoreProvider provider) {
     this.provider = provider;
-    this.config =  PStoreConfig //
-        .newJacksonBuilder(config.getMapper(), OptionValue.class) //
-        .name("sys.options") //
+    this.config =  PStoreConfig.newJacksonBuilder(config.getMapper(), OptionValue.class)
+        .name("sys.options")
         .build();
   }
 
-  public SystemOptionManager init() throws IOException{
-    this.options = provider.getStore(config);
-    this.admin = new SystemOptionAdmin();
+  public SystemOptionManager init() throws IOException {
+    options = provider.getStore(config);
+    admin = new SystemOptionAdmin();
     return this;
   }
 
   @Override
   public Iterator<OptionValue> iterator() {
-    Map<String, OptionValue> buildList = Maps.newHashMap();
+    final Map<String, OptionValue> buildList = Maps.newHashMap();
     for(OptionValidator v : knownOptions.values()){
       buildList.put(v.getOptionName(), v.getDefault());
     }
     for(Map.Entry<String, OptionValue> v : options){
-      OptionValue value = v.getValue();
+      final OptionValue value = v.getValue();
       buildList.put(value.name, value);
     }
     return buildList.values().iterator();
   }
 
   @Override
-  public OptionValue getOption(String name) {
+  public OptionValue getOption(final String name) {
     // check local space
-    OptionValue v = options.get(name);
+    final OptionValue v = options.get(name);
     if(v != null){
       return v;
     }
 
     // otherwise, return default.
     OptionValidator validator = knownOptions.get(name);
-    if(validator == null){
+    if(validator == null) {
       return null;
-    }else{
+    } else {
       return validator.getDefault();
     }
   }
 
   @Override
-  public void setOption(OptionValue value) {
+  public void setOption(final OptionValue value) {
     assert value.type == OptionType.SYSTEM;
     admin.validate(value);
     setOptionInternal(value);
   }
 
-  private void setOptionInternal(OptionValue value){
-    if(!value.equals(knownOptions.get(value.name))){
+  private void setOptionInternal(final OptionValue value) {
+    if (!value.equals(knownOptions.get(value.name))) {
       options.put(value.name, value);
     }
   }
 
 
   @Override
-  public void setOption(String name, SqlLiteral literal, OptionType type) {
-    assert type == OptionValue.OptionType.SYSTEM;
-    OptionValue v = admin.validate(name, literal);
-    v.type = type;
+  public void setOption(final String name, final SqlLiteral literal, final OptionType type) {
+    assert type == OptionValue.OptionType.SYSTEM || type == OptionValue.OptionType.SESSION;
+    final OptionValue v = admin.validate(name, literal, type);
     setOptionInternal(v);
   }
 
@@ -182,40 +180,34 @@ public class SystemOptionManager implements OptionManager {
     return admin;
   }
 
-  private class SystemOptionAdmin implements OptionAdmin{
-
+  private class SystemOptionAdmin implements OptionAdmin {
     public SystemOptionAdmin() {
       for(OptionValidator v : VALIDATORS) {
         knownOptions.put(v.getOptionName(), v);
       }
 
-      for(Entry<String, OptionValue> v : options){
-        OptionValue value = v.getValue();
-        OptionValidator defaultValidator = knownOptions.get(v.getKey());
-        if(defaultValidator == null){
+      for(Entry<String, OptionValue> v : options) {
+        final OptionValue value = v.getValue();
+        final OptionValidator defaultValidator = knownOptions.get(v.getKey());
+        if (defaultValidator == null) {
           // deprecated option, delete.
           options.delete(value.name);
           logger.warn("Deleting deprecated option `{}`.", value.name);
-        }else if(value.equals(defaultValidator)){
-          // option set with default value, remove storage of record.
-          options.delete(value.name);
-          logger.warn("Deleting option `{}` set to default value.", value.name);
         }
-
       }
-
     }
 
     @Override
-    public void registerOptionType(OptionValidator validator) {
-      if (null != knownOptions.putIfAbsent(validator.getOptionName(), validator) ) {
-        throw new IllegalArgumentException("Only one option is allowed to be registered with name: " + validator.getOptionName());
+    public void registerOptionType(final OptionValidator validator) {
+      if (null != knownOptions.putIfAbsent(validator.getOptionName(), validator)) {
+        throw new IllegalArgumentException("Only one option is allowed to be registered with name: "
+            + validator.getOptionName());
       }
     }
 
     @Override
-    public void validate(OptionValue v) throws SetOptionException {
-      OptionValidator validator = knownOptions.get(v.name);
+    public void validate(final OptionValue v) throws SetOptionException {
+      final OptionValidator validator = knownOptions.get(v.name);
       if (validator == null) {
         throw new SetOptionException("Unknown option " + v.name);
       }
@@ -223,14 +215,13 @@ public class SystemOptionManager implements OptionManager {
     }
 
     @Override
-    public OptionValue validate(String name, SqlLiteral value) throws SetOptionException {
-      OptionValidator validator = knownOptions.get(name);
+    public OptionValue validate(final String name, final SqlLiteral value, final OptionType optionType)
+        throws SetOptionException {
+      final OptionValidator validator = knownOptions.get(name);
       if (validator == null) {
         throw new SetOptionException("Unknown option: " + name);
       }
-      return validator.validate(value);
+      return validator.validate(value, optionType);
     }
-
   }
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
index e53fcfe..b9721cc 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
@@ -25,13 +25,13 @@ import org.apache.drill.common.exceptions.ExpressionParsingException;
 import org.apache.drill.exec.server.options.OptionValue.Kind;
 import org.apache.drill.exec.server.options.OptionValue.OptionType;
 import org.eigenbase.sql.SqlLiteral;
+import org.eigenbase.sql.type.SqlTypeName;
 import org.eigenbase.util.NlsString;
 
 public class TypeValidators {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TypeValidators.class);
+//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TypeValidators.class);
 
   public static class PositiveLongValidator extends LongValidator {
-
     private final long max;
 
     public PositiveLongValidator(String name, long max, long def) {
@@ -82,33 +82,32 @@ public class TypeValidators {
     public void validate(OptionValue v) throws ExpressionParsingException {
       super.validate(v);
       if (v.float_val > max || v.float_val < min) {
-        throw new ExpressionParsingException(String.format("Option %s must be between %d and %d.", getOptionName(), min,
-            max));
+        throw new ExpressionParsingException(String.format("Option %s must be between %f and %f.",
+            getOptionName(), min, max));
       }
     }
 
   }
 
-  public static class BooleanValidator extends TypeValidator{
+  public static class BooleanValidator extends TypeValidator {
     public BooleanValidator(String name, boolean def) {
       super(name, Kind.BOOLEAN, OptionValue.createBoolean(OptionType.SYSTEM, name, def));
     }
   }
 
-  public static class StringValidator extends TypeValidator{
+  public static class StringValidator extends TypeValidator {
     public StringValidator(String name, String def) {
       super(name, Kind.STRING, OptionValue.createString(OptionType.SYSTEM, name, def));
     }
-
   }
 
-  public static class LongValidator extends TypeValidator{
+  public static class LongValidator extends TypeValidator {
     public LongValidator(String name, long def) {
       super(name, Kind.LONG, OptionValue.createLong(OptionType.SYSTEM, name, def));
     }
   }
 
-  public static class DoubleValidator extends TypeValidator{
+  public static class DoubleValidator extends TypeValidator {
     public DoubleValidator(String name, double def) {
       super(name, Kind.DOUBLE, OptionValue.createDouble(OptionType.SYSTEM, name, def));
     }
@@ -128,18 +127,17 @@ public class TypeValidators {
     public void validate(OptionValue v) throws ExpressionParsingException {
       super.validate(v);
       if (v.num_val > max || v.num_val < min) {
-        throw new ExpressionParsingException(String.format("Option %s must be between %d and %d.", getOptionName(), min,
-            max));
+        throw new ExpressionParsingException(String.format("Option %s must be between %d and %d.",
+            getOptionName(), min, max));
       }
     }
-
   }
 
   /**
    * Validator that checks if the given value is included in a list of acceptable values. Case insensitive.
    */
   public static class EnumeratedStringValidator extends StringValidator {
-    Set<String> valuesSet = new HashSet<>();
+    private final Set<String> valuesSet = new HashSet<>();
 
     public EnumeratedStringValidator(String name, String def, String... values) {
       super(name, def);
@@ -149,20 +147,19 @@ public class TypeValidators {
     }
 
     @Override
-    public void validate(OptionValue v) throws ExpressionParsingException {
+    public void validate(final OptionValue v) throws ExpressionParsingException {
       super.validate(v);
       if (!valuesSet.contains(v.string_val.toLowerCase())) {
         throw new ExpressionParsingException(String.format("Option %s must be one of: %s", getOptionName(), valuesSet));
       }
     }
-
   }
 
   public static abstract class TypeValidator extends OptionValidator {
-    final Kind kind;
-    private OptionValue defaultValue;
+    private final Kind kind;
+    private final OptionValue defaultValue;
 
-    public TypeValidator(String name, Kind kind, OptionValue defValue) {
+    public TypeValidator(final String name, final Kind kind, final OptionValue defValue) {
       super(name);
       this.kind = kind;
       this.defaultValue = defValue;
@@ -174,55 +171,57 @@ public class TypeValidators {
     }
 
     @Override
-    public OptionValue validate(SqlLiteral value) throws ExpressionParsingException {
-      OptionValue op = getPartialValue(getOptionName(), (OptionType) null, value);
+    public OptionValue validate(final SqlLiteral value, final OptionType optionType)
+        throws ExpressionParsingException {
+      final OptionValue op = getPartialValue(getOptionName(), optionType, value);
       validate(op);
       return op;
     }
 
     @Override
-    public void validate(OptionValue v) throws ExpressionParsingException {
+    public void validate(final OptionValue v) throws ExpressionParsingException {
       if (v.kind != kind) {
-        throw new ExpressionParsingException(String.format("Option %s must be of type %s but you tried to set to %s.",
+        throw new ExpressionParsingException(String.format(
+            "Option %s must be of type %s but you tried to set to %s.",
             getOptionName(), kind.name(), v.kind.name()));
       }
     }
-
-    public void extraValidate(OptionValue v) throws ExpressionParsingException {
-    }
-
   }
 
-  public static OptionValue getPartialValue(String name, OptionType type, SqlLiteral literal) {
-    switch (literal.getTypeName()) {
-    case DECIMAL:
-      if (((BigDecimal) literal.getValue()).scale() == 0) {
-        return OptionValue.createLong(type, name, ((BigDecimal) literal.getValue()).longValue());
+  private static OptionValue getPartialValue(final String name, final OptionType type, final SqlLiteral literal) {
+    final Object object = literal.getValue();
+    final SqlTypeName typeName = literal.getTypeName();
+    switch (typeName) {
+    case DECIMAL: {
+      final BigDecimal bigDecimal = (BigDecimal) object;
+      if (bigDecimal.scale() == 0) {
+        return OptionValue.createLong(type, name, bigDecimal.longValue());
       } else {
-        return OptionValue.createDouble(type, name, ((BigDecimal) literal.getValue()).doubleValue());
+        return OptionValue.createDouble(type, name, bigDecimal.doubleValue());
       }
+    }
+
     case DOUBLE:
     case FLOAT:
-      return OptionValue.createDouble(type, name, ((BigDecimal) literal.getValue()).doubleValue());
+      return OptionValue.createDouble(type, name, ((BigDecimal) object).doubleValue());
 
     case SMALLINT:
     case TINYINT:
     case BIGINT:
     case INTEGER:
-      return OptionValue.createLong(type, name, ((BigDecimal) literal.getValue()).longValue());
+      return OptionValue.createLong(type, name, ((BigDecimal) object).longValue());
 
     case VARBINARY:
     case VARCHAR:
     case CHAR:
-      return OptionValue.createString(type, name, ((NlsString) literal.getValue()).getValue());
+      return OptionValue.createString(type, name, ((NlsString) object).getValue());
 
     case BOOLEAN:
-      return OptionValue.createBoolean(type, name, (Boolean) literal.getValue());
+      return OptionValue.createBoolean(type, name, (Boolean) object);
 
+    default:
+      throw new ExpressionParsingException(String.format(
+          "Drill doesn't support set option expressions with literals of type %s.", typeName));
     }
-
-    throw new ExpressionParsingException(String.format(
-        "Drill doesn't support set option expressions with literals of type %s.", literal.getTypeName()));
   }
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
index 4d2a0df..775bccb 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
@@ -51,7 +51,6 @@ import org.apache.drill.exec.server.RemoteServiceSet;
 import org.apache.drill.exec.util.VectorUtil;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
-import org.junit.Rule;
 import org.junit.rules.TestRule;
 import org.junit.rules.TestWatcher;
 import org.junit.runner.Description;
@@ -60,7 +59,7 @@ import com.google.common.base.Charsets;
 import com.google.common.io.Resources;
 
 public class BaseTestQuery extends ExecTest{
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(BaseTestQuery.class);
+  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(BaseTestQuery.class);
 
   /**
    * Number of Drillbits in test cluster. Default is 1.

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java b/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
index 51f3121..875fb25 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
@@ -22,6 +22,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 
 public class TestBugFixes extends BaseTestQuery {
+  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestBugFixes.class);
 
   @Test
   public void leak1() throws Exception {

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/test/java/org/apache/drill/TestTpchDistributed.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestTpchDistributed.java b/exec/java-exec/src/test/java/org/apache/drill/TestTpchDistributed.java
index 254d9be..e0f830d 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestTpchDistributed.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestTpchDistributed.java
@@ -20,10 +20,10 @@ package org.apache.drill;
 import org.junit.Ignore;
 import org.junit.Test;
 
-public class TestTpchDistributed extends BaseTestQuery{
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestTpchDistributed.class);
+public class TestTpchDistributed extends BaseTestQuery {
+//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestTpchDistributed.class);
 
-  private void testDistributed(String fileName) throws Exception{
+  private static void testDistributed(String fileName) throws Exception {
     String query = getFile(fileName);
     test("alter session set `planner.slice_target` = 10; " + query);
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java b/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java
index 7ff00fe..f5f5b8d 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java
@@ -104,7 +104,7 @@ public class TestClassTransformation extends BaseTestQuery {
   private void compilationInnerClass(QueryClassLoader loader) throws Exception{
     CodeGenerator<ExampleInner> cg = newCodeGenerator(ExampleInner.class, ExampleTemplateWithInner.class);
 
-    ClassTransformer ct = new ClassTransformer();
+    ClassTransformer ct = new ClassTransformer(sessionOptions);
     Class<? extends ExampleInner> c = (Class<? extends ExampleInner>) ct.getImplementationClass(loader, cg.getDefinition(), cg.generateAndGet(), cg.getMaterializedClassName());
     ExampleInner t = (ExampleInner) c.newInstance();
     t.doOutside();

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/test/java/org/apache/drill/exec/compile/bytecode/ReplaceMethodInvoke.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/compile/bytecode/ReplaceMethodInvoke.java b/exec/java-exec/src/test/java/org/apache/drill/exec/compile/bytecode/ReplaceMethodInvoke.java
index f9971a7..bc2d929 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/compile/bytecode/ReplaceMethodInvoke.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/compile/bytecode/ReplaceMethodInvoke.java
@@ -23,14 +23,13 @@ import java.io.StringWriter;
 import java.net.URL;
 
 import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.compile.DrillCheckClassAdapter;
 import org.apache.drill.exec.compile.QueryClassLoader;
 import org.apache.drill.exec.server.options.SystemOptionManager;
 import org.apache.drill.exec.store.sys.local.LocalPStoreProvider;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.ClassWriter;
-import org.objectweb.asm.util.ASMifier;
-import org.objectweb.asm.util.CheckClassAdapter;
 import org.objectweb.asm.util.Textifier;
 import org.objectweb.asm.util.TraceClassVisitor;
 
@@ -38,68 +37,48 @@ import com.google.common.io.Files;
 import com.google.common.io.Resources;
 
 public class ReplaceMethodInvoke {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ReplaceMethodInvoke.class);
+//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ReplaceMethodInvoke.class);
 
-  @SuppressWarnings("unchecked")
-  public static void main(String[] args) throws Exception{
-    String e = "org/apache/drill/ExampleReplaceable.class";
-    String h = "org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.class";
-    String k = "org/apache/drill/Pickle$OutgoingBatch.class";
-    String k2 = "org/apache/drill/Pickle.class";
-    String r = "org/apache/drill/exec/test/generated/FiltererGen0.class";
-    String p = "org/apache/drill/exec/test/generated/PartitionerGen4.class";
-    URL url = Resources.getResource(k2);
-    byte[] clazz = Resources.toByteArray(url);
-    ClassReader cr = new ClassReader(clazz);
+  public static void main(String[] args) throws Exception {
+    final String k2 = "org/apache/drill/Pickle.class";
+    final URL url = Resources.getResource(k2);
+    final byte[] clazz = Resources.toByteArray(url);
+    final ClassReader cr = new ClassReader(clazz);
 
-    ClassWriter cw = writer();
-    TraceClassVisitor visitor = new TraceClassVisitor(cw, new Textifier(), new PrintWriter(System.out));
-    //getTracer(false)
-    ValueHolderReplacementVisitor v2 = new ValueHolderReplacementVisitor(visitor);
+    final ClassWriter cw = writer();
+    final TraceClassVisitor visitor = new TraceClassVisitor(cw, new Textifier(), new PrintWriter(System.out));
+    final ValueHolderReplacementVisitor v2 = new ValueHolderReplacementVisitor(visitor, true);
     cr.accept(v2, ClassReader.EXPAND_FRAMES );//| ClassReader.SKIP_DEBUG);
 
-    byte[] output = cw.toByteArray();
+    final byte[] output = cw.toByteArray();
     Files.write(output, new File("/src/scratch/bytes/S.class"));
     check(output);
 
-
-    DrillConfig c = DrillConfig.forClient();
-    SystemOptionManager m = new SystemOptionManager(c, new LocalPStoreProvider(c));
+    final DrillConfig c = DrillConfig.forClient();
+    final SystemOptionManager m = new SystemOptionManager(c, new LocalPStoreProvider(c));
     m.init();
-    QueryClassLoader ql = new QueryClassLoader(DrillConfig.create(), m);
-    ql.injectByteCode("org.apache.drill.Pickle$OutgoingBatch", output);
-    Class<?> clz = ql.loadClass("org.apache.drill.Pickle$OutgoingBatch");
-    clz.getMethod("x").invoke(null);
-
+    try (QueryClassLoader ql = new QueryClassLoader(DrillConfig.create(), m)) {
+      ql.injectByteCode("org.apache.drill.Pickle$OutgoingBatch", output);
+      Class<?> clz = ql.loadClass("org.apache.drill.Pickle$OutgoingBatch");
+      clz.getMethod("x").invoke(null);
+    }
   }
 
-
-  private static final void check(byte[] b) {
-    ClassReader cr = new ClassReader(b);
-    ClassWriter cw = writer();
-    ClassVisitor cv = new CheckClassAdapter(cw);
+  private static final void check(final byte[] b) {
+    final ClassReader cr = new ClassReader(b);
+    final ClassWriter cw = writer();
+    final ClassVisitor cv = new DrillCheckClassAdapter(cw);
     cr.accept(cv, 0);
 
-    StringWriter sw = new StringWriter();
-    PrintWriter pw = new PrintWriter(sw);
-    CheckClassAdapter.verify(new ClassReader(cw.toByteArray()), false, pw);
+    final StringWriter sw = new StringWriter();
+    final PrintWriter pw = new PrintWriter(sw);
+    DrillCheckClassAdapter.verify(new ClassReader(cw.toByteArray()), false, pw);
 
     assert sw.toString().length() == 0 : sw.toString();
   }
 
   private static ClassWriter writer() {
-    ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES);
+    final ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES);
     return cw;
   }
-
-  private static ClassVisitor getTracer(boolean asm) {
-    if (asm) {
-      return new TraceClassVisitor(null, new ASMifier(), new PrintWriter(System.out));
-    } else {
-      return new TraceClassVisitor(null, new Textifier(), new PrintWriter(System.out));
-    }
-  }
-
-
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestSimpleRepeatedFunctions.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestSimpleRepeatedFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestSimpleRepeatedFunctions.java
index 8c75feb..2372554 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestSimpleRepeatedFunctions.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestSimpleRepeatedFunctions.java
@@ -17,24 +17,23 @@
  */
 package org.apache.drill.exec.expr.fn.impl;
 
+import static org.junit.Assert.assertEquals;
+
 import org.apache.drill.PlanTestBase;
 import org.junit.Test;
 
 public class TestSimpleRepeatedFunctions extends PlanTestBase {
-
   @Test
   public void testIfDrillCanInferReturnTypeOfRepeatedContains() throws Exception {
     final String sql = "select t.arrayval from cp.`nested/nested_1.json` t where repeated_CoNTaInS(t.arrayval, 'c1')";
     final int count = testSql(sql);
-    assert count == 1 : "Unexpected number of records";
+    assertEquals(1, count);
   }
 
   @Test
   public void testIfDrillCanInferReturnTypeOfRepeatedContainsPreceedingTrue() throws Exception {
     final String sql = "select t.arrayval from cp.`nested/nested_1.json` t where true and repeated_CoNTaInS(t.arrayval, 'a1')";
     final int count = testSql(sql);
-    assert count == 1 : "Unexpected number of records";
+    assertEquals(1, count);
   }
-
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
index a6cce3c..fc4c0cc 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
@@ -30,24 +30,32 @@ import java.util.List;
 import mockit.Injectable;
 
 import org.apache.drill.BaseTestQuery;
+import org.apache.drill.exec.compile.ClassTransformer;
+import org.apache.drill.exec.compile.ClassTransformer.ScalarReplacementOption;
 import org.apache.drill.exec.expr.fn.impl.DateUtility;
 import org.apache.drill.exec.proto.UserBitShared.QueryType;
 import org.apache.drill.exec.record.RecordBatchLoader;
+import org.apache.drill.exec.rpc.RpcException;
 import org.apache.drill.exec.rpc.user.QueryResultBatch;
 import org.apache.drill.exec.rpc.user.UserServer;
+import org.apache.drill.exec.server.Drillbit;
 import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.options.OptionManager;
+import org.apache.drill.exec.server.options.OptionValue;
+import org.apache.drill.exec.server.options.OptionValue.OptionType;
 import org.apache.drill.exec.util.ByteBufUtil.HadoopWritables;
 import org.apache.drill.exec.util.VectorUtil;
 import org.apache.drill.exec.vector.ValueVector;
 import org.apache.drill.exec.vector.VarCharVector;
 import org.joda.time.DateTime;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import com.google.common.base.Charsets;
 import com.google.common.io.Resources;
 
 public class TestConvertFunctions extends BaseTestQuery {
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestConvertFunctions.class);
+//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestConvertFunctions.class);
 
   private static final String CONVERSION_TEST_LOGICAL_PLAN = "functions/conv/conversionTestWithLogicalPlan.json";
   private static final String CONVERSION_TEST_PHYSICAL_PLAN = "functions/conv/conversionTestWithPhysicalPlan.json";
@@ -318,13 +326,15 @@ public class TestConvertFunctions extends BaseTestQuery {
     verifyPhysicalPlan("convert_to('apache_drill', 'UTF8')", new byte[] {'a', 'p', 'a', 'c', 'h', 'e', '_', 'd', 'r', 'i', 'l', 'l'});
   }
 
+  @Ignore // TODO(DRILL-2326) remove this when we get rid of the scalar replacement option test cases below
   @Test
   public void testBigIntVarCharReturnTripConvertLogical() throws Exception {
-    String logicalPlan = Resources.toString(Resources.getResource(CONVERSION_TEST_LOGICAL_PLAN), Charsets.UTF_8);
-    List<QueryResultBatch> results =  testLogicalWithResults(logicalPlan);
+    final String logicalPlan = Resources.toString(
+        Resources.getResource(CONVERSION_TEST_LOGICAL_PLAN), Charsets.UTF_8);
+    final List<QueryResultBatch> results =  testLogicalWithResults(logicalPlan);
     int count = 0;
-    RecordBatchLoader loader = new RecordBatchLoader(getAllocator());
-    for (QueryResultBatch result : results){
+    final RecordBatchLoader loader = new RecordBatchLoader(getAllocator());
+    for (QueryResultBatch result : results) {
       count += result.getHeader().getRowCount();
       loader.load(result.getHeader().getDef(), result.getData());
       if (loader.getRecordCount() > 0) {
@@ -336,6 +346,97 @@ public class TestConvertFunctions extends BaseTestQuery {
     assertTrue(count == 10);
   }
 
+  /**
+   * Set up the options to test the scalar replacement retry option (see
+   * ClassTransformer.java). Scalar replacement rewrites bytecode to replace
+   * value holders (essentially boxed values) with their member variables as
+   * locals. There is still one pattern that doesn't work, and occasionally new
+   * ones are introduced. This can be used in tests that exercise failing patterns.
+   *
+   * <p>This also flushes the compiled code cache.
+   *
+   * <p>TODO this should get moved to QueryTestUtil once DRILL-2245 has been merged
+   *
+   * @param drillbit the drillbit
+   * @param srOption the scalar replacement option value to use
+   * @return the original scalar replacement option setting (so it can be restored)
+   */
+  private static OptionValue setupScalarReplacementOption(
+      final Drillbit drillbit, final ScalarReplacementOption srOption) {
+    // set the system option
+    final DrillbitContext drillbitContext = drillbit.getContext();
+    final OptionManager optionManager = drillbitContext.getOptionManager();
+    final OptionValue originalOptionValue = optionManager.getOption(ClassTransformer.SCALAR_REPLACEMENT_OPTION);
+    final OptionValue newOptionValue = OptionValue.createString(OptionType.SYSTEM,
+        ClassTransformer.SCALAR_REPLACEMENT_OPTION, srOption.name().toLowerCase());
+    optionManager.setOption(newOptionValue);
+
+    // flush the code cache
+    drillbitContext.getCompiler().flushCache();
+
+    return originalOptionValue;
+  }
+
+  /**
+   * Restore the original scalar replacement option returned from
+   * setupScalarReplacementOption().
+   *
+   * <p>This also flushes the compiled code cache.
+   *
+   * <p>TODO this should get moved to QueryTestUtil once DRILL-2245 has been merged
+   *
+   * @param drillbit the drillbit
+   * @param srOption the scalar replacement option value to use
+   */
+  private static void restoreScalarReplacementOption(final Drillbit drillbit, final OptionValue srOption) {
+    final DrillbitContext drillbitContext = drillbit.getContext();
+    final OptionManager optionManager = drillbitContext.getOptionManager();
+    optionManager.setOption(srOption);
+
+    // flush the code cache
+    drillbitContext.getCompiler().flushCache();
+  }
+
+  @Test // TODO(DRILL-2326) temporary until we fix the scalar replacement bug for this case
+  public void testBigIntVarCharReturnTripConvertLogical_ScalarReplaceTRY() throws Exception {
+    final OptionValue srOption = setupScalarReplacementOption(bits[0], ScalarReplacementOption.TRY);
+    try {
+      // this should work fine
+      testBigIntVarCharReturnTripConvertLogical();
+    } finally {
+      // restore the system option
+      restoreScalarReplacementOption(bits[0], srOption);
+    }
+  }
+
+  @Test // TODO(DRILL-2326) temporary until we fix the scalar replacement bug for this case
+  public void testBigIntVarCharReturnTripConvertLogical_ScalarReplaceON() throws Exception {
+    final OptionValue srOption = setupScalarReplacementOption(bits[0], ScalarReplacementOption.ON);
+    boolean caughtException = false;
+    try {
+      // this will fail (with a JUnit assertion) until we fix the SR bug
+      testBigIntVarCharReturnTripConvertLogical();
+    } catch(RpcException e) {
+      caughtException = true;
+    } finally {
+      restoreScalarReplacementOption(bits[0], srOption);
+    }
+
+    assertTrue(caughtException);
+  }
+
+  @Test // TODO(DRILL-2326) temporary until we fix the scalar replacement bug for this case
+  public void testBigIntVarCharReturnTripConvertLogical_ScalarReplaceOFF() throws Exception {
+    final OptionValue srOption = setupScalarReplacementOption(bits[0], ScalarReplacementOption.OFF);
+    try {
+      // this should work fine
+      testBigIntVarCharReturnTripConvertLogical();
+    } finally {
+      // restore the system option
+      restoreScalarReplacementOption(bits[0], srOption);
+    }
+  }
+
   @Test
   public void testHadooopVInt() throws Exception {
     final int _0 = 0;

http://git-wip-us.apache.org/repos/asf/drill/blob/7ea212a2/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 0e56ed6..05573a1 100644
--- a/pom.xml
+++ b/pom.xml
@@ -382,6 +382,7 @@
           <configuration>
             <argLine>-Xms512m -Xmx2g -Ddrill.exec.http.enabled=false
               -Ddrill.exec.sys.store.provider.local.write=false
+              -Dorg.apache.drill.exec.server.Drillbit.system_options="org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on"
               -XX:MaxPermSize=256M -XX:MaxDirectMemorySize=3072M
               -XX:+CMSClassUnloadingEnabled -ea</argLine>
             <forkCount>4</forkCount>


Mime
View raw message