DRILL-4715: Fix java compilation error in run-time generated code whe…

…n query has large number of expressions.

Refactor unit test in drillbit context initialization and pass in option manager.

close #521

exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java

@@ -321,4 +321,8 @@ public interface ExecConstants {
    */
   String WEB_LOGS_MAX_LINES = "web.logs.max_lines";
   OptionValidator WEB_LOGS_MAX_LINES_VALIDATOR = new PositiveLongValidator(WEB_LOGS_MAX_LINES, Integer.MAX_VALUE, 10000);
+
+  String CODE_GEN_EXP_IN_METHOD_SIZE = "exec.java.compiler.exp_in_method_size";
+  LongValidator CODE_GEN_EXP_IN_METHOD_SIZE_VALIDATOR = new LongValidator(CODE_GEN_EXP_IN_METHOD_SIZE, 50);
+
 }

exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java

@@ -28,6 +28,7 @@
 import org.apache.drill.common.types.TypeProtos;
 import org.apache.drill.common.types.TypeProtos.DataMode;
 import org.apache.drill.common.types.TypeProtos.MajorType;
+import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.compile.sig.CodeGeneratorArgument;
 import org.apache.drill.exec.compile.sig.CodeGeneratorMethod;
 import org.apache.drill.exec.compile.sig.GeneratorMapping;
@@ -54,6 +55,7 @@
 import com.sun.codemodel.JMod;
 import com.sun.codemodel.JType;
 import com.sun.codemodel.JVar;
+import org.apache.drill.exec.server.options.OptionManager;
 
 public class ClassGenerator<T>{
 
@@ -63,8 +65,6 @@ public class ClassGenerator<T>{
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClassGenerator.class);
   public static enum BlockType {SETUP, EVAL, RESET, CLEANUP};
 
-  private static final int MAX_BLOCKS_IN_FUNCTION = 50;
-
   private final SignatureHolder sig;
   private final EvaluationVisitor evaluationVisitor;
   private final Map<ValueVectorSetup, JVar> vvDeclaration = Maps.newHashMap();
@@ -74,8 +74,9 @@ public static enum BlockType {SETUP, EVAL, RESET, CLEANUP};
   private final CodeGenerator<T> codeGenerator;
 
   public final JDefinedClass clazz;
-  private final LinkedList<JBlock>[] blocks;
+  private final LinkedList<SizedJBlock>[] blocks;
   private final JCodeModel model;
+  private final OptionManager optionManager;
 
   private int index = 0;
   private int labelIndex = 0;
@@ -86,14 +87,16 @@ public static MappingSet getDefaultMapping() {
   }
 
   @SuppressWarnings("unchecked")
-  ClassGenerator(CodeGenerator<T> codeGenerator, MappingSet mappingSet, SignatureHolder signature, EvaluationVisitor eval, JDefinedClass clazz, JCodeModel model) throws JClassAlreadyExistsException {
+  ClassGenerator(CodeGenerator<T> codeGenerator, MappingSet mappingSet, SignatureHolder signature, EvaluationVisitor eval, JDefinedClass clazz, JCodeModel model, OptionManager optionManager) throws JClassAlreadyExistsException {
     this.codeGenerator = codeGenerator;
     this.clazz = clazz;
     this.mappings = mappingSet;
     this.sig = signature;
     this.evaluationVisitor = eval;
     this.model = model;
-    blocks = (LinkedList<JBlock>[]) new LinkedList[sig.size()];
+    this.optionManager = optionManager;
+
+    blocks = (LinkedList<SizedJBlock>[]) new LinkedList[sig.size()];
     for (int i =0; i < sig.size(); i++) {
       blocks[i] = Lists.newLinkedList();
     }
@@ -102,7 +105,7 @@ public static MappingSet getDefaultMapping() {
     for (SignatureHolder child : signature.getChildHolders()) {
       String innerClassName = child.getSignatureClass().getSimpleName();
       JDefinedClass innerClazz = clazz._class(Modifier.FINAL + Modifier.PRIVATE, innerClassName);
-      innerClasses.put(innerClassName, new ClassGenerator<>(codeGenerator, mappingSet, child, eval, innerClazz, model));
+      innerClasses.put(innerClassName, new ClassGenerator<>(codeGenerator, mappingSet, child, eval, innerClazz, model, optionManager));
     }
   }
 
@@ -129,7 +132,7 @@ private GeneratorMapping getCurrentMapping() {
   }
 
   public JBlock getBlock(String methodName) {
-    JBlock blk = this.blocks[sig.get(methodName)].getLast();
+    JBlock blk = this.blocks[sig.get(methodName)].getLast().getBlock();
     Preconditions.checkNotNull(blk, "Requested method name of %s was not available for signature %s.",  methodName, this.sig);
     return blk;
   }
@@ -154,7 +157,7 @@ public JBlock getCleanupBlock() {
   public void nestEvalBlock(JBlock block) {
     String methodName = getCurrentMapping().getMethodName(BlockType.EVAL);
     evaluationVisitor.newScope();
-    this.blocks[sig.get(methodName)].addLast(block);
+    this.blocks[sig.get(methodName)].addLast(new SizedJBlock(block));
   }
 
   public void unNestEvalBlock() {
@@ -215,22 +218,47 @@ public JVar declareVectorValueSetupAndMember(DirectExpression batchName, TypedFi
     return vv;
   }
 
+  public enum BlkCreateMode {
+    TRUE,  // Create new block
+    FALSE, // Do not create block; put into existing block.
+    TRUE_IF_BOUND // Create new block only if # of expressions added hit upper-bound (ExecConstants.CODE_GEN_EXP_IN_METHOD_SIZE)
+  }
+
   public HoldingContainer addExpr(LogicalExpression ex) {
-    return addExpr(ex, true);
+    // default behavior is always to put expression into new block.
+    return addExpr(ex, BlkCreateMode.TRUE);
   }
 
-  public HoldingContainer addExpr(LogicalExpression ex, boolean rotate) {
-//    logger.debug("Adding next write {}", ex);
-    if (rotate) {
-      rotateBlock();
+  public HoldingContainer addExpr(LogicalExpression ex, BlkCreateMode mode) {
+    if (mode == BlkCreateMode.TRUE || mode == BlkCreateMode.TRUE_IF_BOUND) {
+      rotateBlock(mode);
     }
+
+    for (LinkedList<SizedJBlock> b : blocks) {
+      b.getLast().incCounter();
+    }
+
     return evaluationVisitor.addExpr(ex, this);
   }
 
   public void rotateBlock() {
-    evaluationVisitor.previousExpressions.clear();
-    for (LinkedList<JBlock> b : blocks) {
-      b.add(new JBlock(true, true));
+    // default behavior is always to create new block.
+    rotateBlock(BlkCreateMode.TRUE);
+  }
+
+  private void rotateBlock(BlkCreateMode mode) {
+    boolean blockRotated = false;
+    for (LinkedList<SizedJBlock> b : blocks) {
+      if (mode == BlkCreateMode.TRUE ||
+          (mode == BlkCreateMode.TRUE_IF_BOUND &&
+            optionManager != null &&
+            b.getLast().getCount() > optionManager.getOption(ExecConstants.CODE_GEN_EXP_IN_METHOD_SIZE_VALIDATOR))) {
+        b.add(new SizedJBlock(new JBlock(true, true)));
+        blockRotated = true;
+      }
+    }
+    if (blockRotated) {
+      evaluationVisitor.previousExpressions.clear();
     }
   }
 
@@ -247,11 +275,13 @@ void flushCode() {
       outer._throws(SchemaChangeException.class);
 
       int methodIndex = 0;
-      int blocksInMethod = 0;
+      int exprsInMethod = 0;
       boolean isVoidMethod = method.getReturnType() == void.class;
-      for(JBlock b : blocks[i++]) {
+      for(SizedJBlock sb : blocks[i++]) {
+        JBlock b = sb.getBlock();
         if(!b.isEmpty()) {
-          if (blocksInMethod > MAX_BLOCKS_IN_FUNCTION) {
+          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);
             JInvocation methodCall = JExpr.invoke(inner);
             for (CodeGeneratorArgument arg : method) {
@@ -269,11 +299,11 @@ void flushCode() {
               outer.body()._return(methodCall);
             }
             outer = inner;
-            blocksInMethod = 0;
+            exprsInMethod = 0;
             ++methodIndex;
           }
           outer.body().add(b);
-          ++blocksInMethod;
+          exprsInMethod += sb.getCount();
         }
       }
     }

exec/java-exec/src/main/java/org/apache/drill/exec/expr/CodeGenerator.java

@@ -27,6 +27,7 @@
 import com.sun.codemodel.JClassAlreadyExistsException;
 import com.sun.codemodel.JCodeModel;
 import com.sun.codemodel.JDefinedClass;
+import org.apache.drill.exec.server.options.OptionManager;
 
 /**
  * A code generator is responsible for generating the Java source code required to complete the implementation of an
@@ -52,12 +53,12 @@ public class CodeGenerator<T> {
   private String generatedCode;
   private String generifiedCode;
 
-  CodeGenerator(TemplateClassDefinition<T> definition, FunctionImplementationRegistry funcRegistry) {
-    this(ClassGenerator.getDefaultMapping(), definition, funcRegistry);
+  CodeGenerator(TemplateClassDefinition<T> definition, FunctionImplementationRegistry funcRegistry, OptionManager optionManager) {
+    this(ClassGenerator.getDefaultMapping(), definition, funcRegistry, optionManager);
   }
 
   CodeGenerator(MappingSet mappingSet, TemplateClassDefinition<T> definition,
-      FunctionImplementationRegistry funcRegistry) {
+      FunctionImplementationRegistry funcRegistry, OptionManager optionManager) {
     Preconditions.checkNotNull(definition.getSignature(),
         "The signature for defintion %s was incorrectly initialized.", definition);
     this.definition = definition;
@@ -67,7 +68,7 @@ public class CodeGenerator<T> {
       this.model = new JCodeModel();
       JDefinedClass clazz = model._package(PACKAGE_NAME)._class(className);
       rootGenerator = new ClassGenerator<>(this, mappingSet, definition.getSignature(), new EvaluationVisitor(
-          funcRegistry), clazz, model);
+          funcRegistry), clazz, model, optionManager);
     } catch (JClassAlreadyExistsException e) {
       throw new IllegalStateException(e);
     }
@@ -107,22 +108,27 @@ public String getMaterializedClassName() {
 
   public static <T> CodeGenerator<T> get(TemplateClassDefinition<T> definition,
       FunctionImplementationRegistry funcRegistry) {
-    return new CodeGenerator<T>(definition, funcRegistry);
+    return get(definition, funcRegistry, null);
+  }
+
+  public static <T> CodeGenerator<T> get(TemplateClassDefinition<T> definition,
+      FunctionImplementationRegistry funcRegistry, OptionManager optionManager) {
+    return new CodeGenerator<T>(definition, funcRegistry, optionManager);
   }
 
   public static <T> ClassGenerator<T> getRoot(TemplateClassDefinition<T> definition,
-      FunctionImplementationRegistry funcRegistry) {
-    return get(definition, funcRegistry).getRoot();
+      FunctionImplementationRegistry funcRegistry, OptionManager optionManager) {
+    return get(definition, funcRegistry, optionManager).getRoot();
   }
 
   public static <T> ClassGenerator<T> getRoot(MappingSet mappingSet, TemplateClassDefinition<T> definition,
-      FunctionImplementationRegistry funcRegistry) {
-    return get(mappingSet, definition, funcRegistry).getRoot();
+      FunctionImplementationRegistry funcRegistry, OptionManager optionManager) {
+    return get(mappingSet, definition, funcRegistry, optionManager).getRoot();
   }
 
   public static <T> CodeGenerator<T> get(MappingSet mappingSet, TemplateClassDefinition<T> definition,
-      FunctionImplementationRegistry funcRegistry) {
-    return new CodeGenerator<T>(mappingSet, definition, funcRegistry);
+      FunctionImplementationRegistry funcRegistry, OptionManager optionManager) {
+    return new CodeGenerator<T>(mappingSet, definition, funcRegistry, optionManager);
   }
 
   @Override

exec/java-exec/src/main/java/org/apache/drill/exec/expr/SizedJBlock.java

@@ -0,0 +1,50 @@
+/**
+ * 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;
+
+import com.sun.codemodel.JBlock;
+
+/**
+ * Uses this class to keep track # of Drill Logical Expressions that are
+ * put to JBlock.
+ *
+ * JBlock is final class; we could not extend JBlock directly.
+ */
+public class SizedJBlock {
+  private final JBlock block;
+  private int count; // # of Drill Logical Expressions added to this block
+
+  public SizedJBlock(JBlock block) {
+    this.block = block;
+    this.count = 0;
+  }
+
+  public JBlock getBlock() {
+    return this.block;
+  }
+
+  public void incCounter() {
+    this.count ++;
+  }
+
+  public int getCount() {
+    return this.count;
+  }
+
+}

exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java

@@ -19,7 +19,6 @@
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
-import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
@@ -186,4 +185,5 @@ public boolean isFunctionComplexOutput(String name) {
     }
     return false;
   }
+
 }

exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java

@@ -330,7 +330,7 @@ private void purge() throws SchemaChangeException {
   public PriorityQueue createNewPriorityQueue(FragmentContext context, List<Ordering> orderings,
                                                      VectorAccessible batch, MappingSet mainMapping, MappingSet leftMapping, MappingSet rightMapping)
           throws ClassTransformationException, IOException, SchemaChangeException{
-    CodeGenerator<PriorityQueue> cg = CodeGenerator.get(PriorityQueue.TEMPLATE_DEFINITION, context.getFunctionRegistry());
+    CodeGenerator<PriorityQueue> cg = CodeGenerator.get(PriorityQueue.TEMPLATE_DEFINITION, context.getFunctionRegistry(), context.getOptions());
     ClassGenerator<PriorityQueue> g = cg.getRoot();
     g.setMappingSet(mainMapping);
 
@@ -342,16 +342,16 @@ public PriorityQueue createNewPriorityQueue(FragmentContext context, List<Orderi
         throw new SchemaChangeException("Failure while materializing expression. " + collector.toErrorString());
       }
       g.setMappingSet(leftMapping);
-      HoldingContainer left = g.addExpr(expr, false);
+      HoldingContainer left = g.addExpr(expr, ClassGenerator.BlkCreateMode.FALSE);
       g.setMappingSet(rightMapping);
-      HoldingContainer right = g.addExpr(expr, false);
+      HoldingContainer right = g.addExpr(expr, ClassGenerator.BlkCreateMode.FALSE);
       g.setMappingSet(mainMapping);
 
       // next we wrap the two comparison sides and add the expression block for the comparison.
       LogicalExpression fh =
         FunctionGenerationHelper.getOrderingComparator(od.nullsSortHigh(), left, right,
                                                        context.getFunctionRegistry());
-      HoldingContainer out = g.addExpr(fh, false);
+      HoldingContainer out = g.addExpr(fh, ClassGenerator.BlkCreateMode.FALSE);
       JConditional jc = g.getEvalBlock()._if(out.getValue().ne(JExpr.lit(0)));
 
       if (od.getDirection() == Direction.ASCENDING) {

exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java

@@ -176,7 +176,7 @@ private boolean createAggregator() {
   private HashAggregator createAggregatorInternal() throws SchemaChangeException, ClassTransformationException,
       IOException {
     CodeGenerator<HashAggregator> top =
-        CodeGenerator.get(HashAggregator.TEMPLATE_DEFINITION, context.getFunctionRegistry());
+        CodeGenerator.get(HashAggregator.TEMPLATE_DEFINITION, context.getFunctionRegistry(), context.getOptions());
     ClassGenerator<HashAggregator> cg = top.getRoot();
     ClassGenerator<HashAggregator> cgInner = cg.getInnerGenerator("BatchHolder");
 
@@ -257,7 +257,7 @@ private void setupUpdateAggrValues(ClassGenerator<HashAggregator> cg) {
     cg.setMappingSet(UpdateAggrValuesMapping);
 
     for (LogicalExpression aggr : aggrExprs) {
-      HoldingContainer hc = cg.addExpr(aggr, true);
+      HoldingContainer hc = cg.addExpr(aggr, ClassGenerator.BlkCreateMode.TRUE);
     }
   }