You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by js...@apache.org on 2016/04/20 18:08:53 UTC

[1/4] drill git commit: DRILL-4442: Move getSV2 and getSV4 methods to VectorAccessible

Repository: drill
Updated Branches:
  refs/heads/master c6a03eb17 -> d93a36338


DRILL-4442: Move getSV2 and getSV4 methods to VectorAccessible

Up one level from previous location RecordBatch, most implementations
already implement the method as they implement RecordBatch rather than
VectorAccessible itself. Add unsupported operation exception to others.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/01e04cdd
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/01e04cdd
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/01e04cdd

Branch: refs/heads/master
Commit: 01e04cddd6ad57f9ae146fe479e30bebcd7cc432
Parents: 1d1acc0
Author: Jason Altekruse <al...@gmail.com>
Authored: Fri Feb 26 14:11:47 2016 -0800
Committer: Jason Altekruse <al...@gmail.com>
Committed: Wed Apr 20 08:10:40 2016 -0700

----------------------------------------------------------------------
 .../impl/partitionsender/PartitionerTemplate.java      | 10 ++++++++++
 .../exec/physical/impl/window/WindowDataBatch.java     | 12 ++++++++++++
 .../drill/exec/physical/impl/xsort/BatchGroup.java     | 11 +++++++++++
 .../java/org/apache/drill/exec/record/RecordBatch.java |  5 -----
 .../apache/drill/exec/record/RecordBatchLoader.java    | 13 ++++++++++++-
 .../org/apache/drill/exec/record/RecordIterator.java   | 12 ++++++++++++
 .../org/apache/drill/exec/record/VectorAccessible.java |  8 ++++++++
 .../org/apache/drill/exec/record/VectorContainer.java  | 12 ++++++++++++
 8 files changed, 77 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/01e04cdd/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java
index 8fe0ab0..556460c 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java
@@ -399,6 +399,16 @@ public abstract class PartitionerTemplate implements Partitioner {
       return vectorContainer.iterator();
     }
 
+    @Override
+    public SelectionVector2 getSelectionVector2() {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public SelectionVector4 getSelectionVector4() {
+      throw new UnsupportedOperationException();
+    }
+
     public WritableBatch getWritableBatch() {
       return WritableBatch.getBatchNoHVWrap(recordCount, this, false);
     }

http://git-wip-us.apache.org/repos/asf/drill/blob/01e04cdd/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowDataBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowDataBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowDataBatch.java
index 7abc03c..7e9f115 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowDataBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowDataBatch.java
@@ -26,6 +26,8 @@ import org.apache.drill.exec.record.TypedFieldId;
 import org.apache.drill.exec.record.VectorAccessible;
 import org.apache.drill.exec.record.VectorContainer;
 import org.apache.drill.exec.record.VectorWrapper;
+import org.apache.drill.exec.record.selection.SelectionVector2;
+import org.apache.drill.exec.record.selection.SelectionVector4;
 import org.apache.drill.exec.vector.ValueVector;
 
 import java.util.Iterator;
@@ -91,6 +93,16 @@ public class WindowDataBatch implements VectorAccessible {
     return container.iterator();
   }
 
+  @Override
+  public SelectionVector2 getSelectionVector2() {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public SelectionVector4 getSelectionVector4() {
+    throw new UnsupportedOperationException();
+  }
+
   public void clear() {
     container.clear();
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/01e04cdd/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/BatchGroup.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/BatchGroup.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/BatchGroup.java
index 5a3b305..0a818ee 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/BatchGroup.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/BatchGroup.java
@@ -34,6 +34,7 @@ import org.apache.drill.exec.record.VectorContainer;
 import org.apache.drill.exec.record.VectorWrapper;
 import org.apache.drill.exec.record.WritableBatch;
 import org.apache.drill.exec.record.selection.SelectionVector2;
+import org.apache.drill.exec.record.selection.SelectionVector4;
 import org.apache.hadoop.fs.FSDataInputStream;
 import org.apache.hadoop.fs.FSDataOutputStream;
 import org.apache.hadoop.fs.FileSystem;
@@ -210,4 +211,14 @@ public class BatchGroup implements VectorAccessible, AutoCloseable {
     return currentContainer.iterator();
   }
 
+  @Override
+  public SelectionVector2 getSelectionVector2() {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public SelectionVector4 getSelectionVector4() {
+    throw new UnsupportedOperationException();
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/01e04cdd/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java
index 8229e58..04cf8fc 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java
@@ -234,11 +234,6 @@ public interface RecordBatch extends VectorAccessible {
    */
   public void kill(boolean sendUpstream);
 
-  public abstract SelectionVector2 getSelectionVector2();
-
-  public abstract SelectionVector4 getSelectionVector4();
-
-
   public VectorContainer getOutgoingContainer();
 
   /**

http://git-wip-us.apache.org/repos/asf/drill/blob/01e04cdd/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
index 84c616b..ea99fcb 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
@@ -30,6 +30,8 @@ import org.apache.drill.exec.expr.TypeHelper;
 import org.apache.drill.exec.memory.BufferAllocator;
 import org.apache.drill.exec.proto.UserBitShared.RecordBatchDef;
 import org.apache.drill.exec.proto.UserBitShared.SerializedField;
+import org.apache.drill.exec.record.selection.SelectionVector2;
+import org.apache.drill.exec.record.selection.SelectionVector4;
 import org.apache.drill.exec.vector.AllocationHelper;
 import org.apache.drill.exec.vector.ValueVector;
 import org.slf4j.Logger;
@@ -50,7 +52,6 @@ public class RecordBatchLoader implements VectorAccessible, Iterable<VectorWrapp
   private int valueCount;
   private BatchSchema schema;
 
-
   /**
    * Constructs a loader using the given allocator for vector buffer allocation.
    */
@@ -188,6 +189,16 @@ public class RecordBatchLoader implements VectorAccessible, Iterable<VectorWrapp
   }
 
   @Override
+  public SelectionVector2 getSelectionVector2() {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public SelectionVector4 getSelectionVector4() {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
   public BatchSchema getSchema() {
     return schema;
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/01e04cdd/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordIterator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordIterator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordIterator.java
index 918a8da..01acd7f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordIterator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordIterator.java
@@ -27,6 +27,8 @@ import org.apache.drill.exec.record.RecordBatch.IterOutcome;
 
 import com.google.common.collect.Range;
 import com.google.common.collect.TreeRangeMap;
+import org.apache.drill.exec.record.selection.SelectionVector2;
+import org.apache.drill.exec.record.selection.SelectionVector4;
 
 /**
  * RecordIterator iterates over incoming record batches one record at a time.
@@ -322,6 +324,16 @@ public class RecordIterator implements VectorAccessible {
     return container.iterator();
   }
 
+  @Override
+  public SelectionVector2 getSelectionVector2() {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public SelectionVector4 getSelectionVector4() {
+    throw new UnsupportedOperationException();
+  }
+
   // Release all vectors held by record batches, clear out range map.
   public void clear() {
     if (container != null) {

http://git-wip-us.apache.org/repos/asf/drill/blob/01e04cdd/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorAccessible.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorAccessible.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorAccessible.java
index 6eb58c5..f1a250c 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorAccessible.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorAccessible.java
@@ -18,6 +18,8 @@
 package org.apache.drill.exec.record;
 
 import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.exec.record.selection.SelectionVector2;
+import org.apache.drill.exec.record.selection.SelectionVector4;
 
 // TODO javadoc
 public interface VectorAccessible extends Iterable<VectorWrapper<?>> {
@@ -50,4 +52,10 @@ public interface VectorAccessible extends Iterable<VectorWrapper<?>> {
    * @return number of records
    */
   public int getRecordCount();
+
+  public abstract SelectionVector2 getSelectionVector2();
+
+  public abstract SelectionVector4 getSelectionVector4();
+
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/01e04cdd/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
index 663cf22..96d9ba6 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
@@ -31,6 +31,8 @@ import org.apache.drill.exec.expr.TypeHelper;
 import org.apache.drill.exec.memory.BufferAllocator;
 import org.apache.drill.exec.ops.OperatorContext;
 import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
+import org.apache.drill.exec.record.selection.SelectionVector2;
+import org.apache.drill.exec.record.selection.SelectionVector4;
 import org.apache.drill.exec.vector.SchemaChangeCallBack;
 import org.apache.drill.exec.vector.ValueVector;
 
@@ -345,6 +347,16 @@ public class VectorContainer implements Iterable<VectorWrapper<?>>, VectorAccess
     return recordCount;
   }
 
+  @Override
+  public SelectionVector2 getSelectionVector2() {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public SelectionVector4 getSelectionVector4() {
+    throw new UnsupportedOperationException();
+  }
+
   /**
    * Clears the contained vectors.  (See {@link ValueVector#clear}).
    */


[3/4] drill git commit: DRILL-4445: Standardize the Physical and Logical plan nodes to use Lists instead of arrays for their inputs

Posted by js...@apache.org.
DRILL-4445: Standardize the Physical and Logical plan nodes to use Lists instead of arrays for their inputs

Remove some extra translation logic used to move between the
two representations.

TODO - look back the the Join logical node, has two JsonCreator annotations,
but only one will be used. Not sure if the behavior of which is chosen
is considered documented behavior, should just fix it on our end.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/d24205d4
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/d24205d4
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/d24205d4

Branch: refs/heads/master
Commit: d24205d4e795a1aab54b64708dde1e7deeca668b
Parents: c6a03eb
Author: Jason Altekruse <al...@gmail.com>
Authored: Wed Feb 10 17:36:47 2016 -0800
Committer: Jason Altekruse <al...@gmail.com>
Committed: Wed Apr 20 08:10:40 2016 -0700

----------------------------------------------------------------------
 .../apache/drill/exec/opt/BasicOptimizer.java   | 18 +++++++----------
 .../exec/physical/base/AbstractMultiple.java    |  9 +++++----
 .../exec/physical/config/HashAggregate.java     | 15 +++++++++-----
 .../physical/config/StreamingAggregate.java     | 12 ++++++-----
 .../drill/exec/physical/config/UnionAll.java    |  4 ++--
 .../drill/exec/physical/config/WindowPOP.java   | 20 ++++++++++---------
 .../physical/impl/aggregate/HashAggBatch.java   | 11 +++++-----
 .../impl/aggregate/HashAggTemplate.java         |  2 +-
 .../impl/aggregate/StreamingAggBatch.java       | 14 ++++++-------
 .../physical/impl/common/ChainedHashTable.java  | 10 +++++-----
 .../physical/impl/common/HashTableConfig.java   | 14 +++++++------
 .../physical/impl/common/HashTableTemplate.java |  2 +-
 .../exec/physical/impl/join/HashJoinBatch.java  |  9 +++++----
 .../impl/window/WindowFrameRecordBatch.java     |  2 +-
 .../physical/impl/window/WindowFunction.java    |  4 ++--
 .../exec/planner/physical/HashAggPrel.java      |  5 +----
 .../exec/planner/physical/StreamAggPrel.java    |  3 +--
 .../exec/planner/physical/UnionAllPrel.java     |  2 +-
 .../planner/physical/UnionDistinctPrel.java     |  2 +-
 .../drill/exec/planner/physical/WindowPrel.java |  6 +++---
 .../common/logical/data/AbstractBuilder.java    | 17 ----------------
 .../common/logical/data/GroupingAggregate.java  | 12 +++++------
 .../apache/drill/common/logical/data/Join.java  | 13 +++++++-----
 .../apache/drill/common/logical/data/Order.java |  8 ++++----
 .../drill/common/logical/data/Project.java      | 21 +++++---------------
 .../common/logical/data/RunningAggregate.java   |  8 +++++---
 .../drill/common/logical/data/Sequence.java     |  9 +++++++--
 .../drill/common/logical/data/Transform.java    |  7 ++++---
 .../apache/drill/common/logical/data/Union.java | 15 +++++---------
 .../drill/common/logical/data/Window.java       | 20 +++++++++----------
 30 files changed, 139 insertions(+), 155 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java
index 3f064d4..27c853a 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java
@@ -56,15 +56,12 @@ import org.apache.drill.exec.physical.config.WindowPOP;
 import org.apache.drill.exec.rpc.user.UserServer.UserClientConnection;
 import org.apache.drill.exec.server.options.OptionManager;
 import org.apache.drill.exec.store.StoragePlugin;
-import org.apache.drill.exec.work.foreman.ForemanException;
 import org.apache.calcite.rel.RelFieldCollation.Direction;
 import org.apache.calcite.rel.RelFieldCollation.NullDirection;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.List;
 
 public class BasicOptimizer extends Optimizer {
@@ -139,7 +136,7 @@ public class BasicOptimizer extends Optimizer {
       final List<Ordering> orderDefs = Lists.newArrayList();
       PhysicalOperator input = groupBy.getInput().accept(this, value);
 
-      if (groupBy.getKeys().length > 0) {
+      if (groupBy.getKeys().size() > 0) {
         for(NamedExpression e : groupBy.getKeys()) {
           orderDefs.add(new Ordering(Direction.ASCENDING, e.getExpr(), NullDirection.FIRST));
         }
@@ -197,8 +194,7 @@ public class BasicOptimizer extends Optimizer {
       rightOp = new Sort(rightOp, rightOrderDefs, false);
       rightOp = new SelectionVectorRemover(rightOp);
 
-      final MergeJoinPOP mjp = new MergeJoinPOP(leftOp, rightOp, Arrays.asList(join.getConditions()),
-          join.getJoinType());
+      final MergeJoinPOP mjp = new MergeJoinPOP(leftOp, rightOp, join.getConditions(), join.getJoinType());
       return new SelectionVectorRemover(mjp);
     }
 
@@ -221,17 +217,17 @@ public class BasicOptimizer extends Optimizer {
 
     @Override
     public PhysicalOperator visitStore(final Store store, final Object obj) throws OptimizerException {
-      final Iterator<LogicalOperator> iterator = store.iterator();
-      if (!iterator.hasNext()) {
+      LogicalOperator input = store.getInput();
+      if (input == null) {
         throw new OptimizerException("Store node in logical plan does not have a child.");
       }
-      return new Screen(iterator.next().accept(this, obj), queryContext.getCurrentEndpoint());
+      return new Screen(store.getInput().accept(this, obj), queryContext.getCurrentEndpoint());
     }
 
     @Override
     public PhysicalOperator visitProject(final Project project, final Object obj) throws OptimizerException {
       return new org.apache.drill.exec.physical.config.Project(
-          Arrays.asList(project.getSelections()), project.iterator().next().accept(this, obj));
+          project.getSelections(), project.getInput().accept(this, obj));
     }
 
     @Override
@@ -239,7 +235,7 @@ public class BasicOptimizer extends Optimizer {
       final TypeProtos.MajorType.Builder b = TypeProtos.MajorType.getDefaultInstance().newBuilderForType();
       b.setMode(DataMode.REQUIRED);
       b.setMinorType(MinorType.BIGINT);
-      final PhysicalOperator child = filter.iterator().next().accept(this, obj);
+      final PhysicalOperator child = filter.getInput().accept(this, obj);
       return new SelectionVectorRemover(new org.apache.drill.exec.physical.config.Filter(child, filter.getExpr(), 1.0f));
     }
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractMultiple.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractMultiple.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractMultiple.java
index 909a152..075643d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractMultiple.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractMultiple.java
@@ -18,6 +18,7 @@
 package org.apache.drill.exec.physical.base;
 
 import java.util.Iterator;
+import java.util.List;
 
 import com.google.common.collect.Iterators;
 
@@ -27,19 +28,19 @@ import com.google.common.collect.Iterators;
 public abstract class AbstractMultiple extends AbstractBase{
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(AbstractMultiple.class);
 
-  protected final PhysicalOperator[] children;
+  protected final List<PhysicalOperator> children;
 
-  protected AbstractMultiple(PhysicalOperator[] children) {
+  protected AbstractMultiple(List<PhysicalOperator> children) {
     this.children = children;
   }
 
-  public PhysicalOperator[] getChildren() {
+  public List<PhysicalOperator> getChildren() {
     return children;
   }
 
   @Override
   public Iterator<PhysicalOperator> iterator() {
-    return Iterators.forArray(children);
+    return children.iterator();
   }
 
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashAggregate.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashAggregate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashAggregate.java
index 694570c..4dafbe8 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashAggregate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashAggregate.java
@@ -27,29 +27,34 @@ import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonTypeName;
 
+import java.util.List;
+
 @JsonTypeName("hash-aggregate")
 public class HashAggregate extends AbstractSingle {
 
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(HashAggregate.class);
 
-  private final NamedExpression[] groupByExprs;
-  private final NamedExpression[] aggrExprs;
+  private final List<NamedExpression> groupByExprs;
+  private final List<NamedExpression> aggrExprs;
 
   private final float cardinality;
 
   @JsonCreator
-  public HashAggregate(@JsonProperty("child") PhysicalOperator child, @JsonProperty("keys") NamedExpression[] groupByExprs, @JsonProperty("exprs") NamedExpression[] aggrExprs, @JsonProperty("cardinality") float cardinality) {
+  public HashAggregate(@JsonProperty("child") PhysicalOperator child,
+                       @JsonProperty("keys") List<NamedExpression> groupByExprs,
+                       @JsonProperty("exprs") List<NamedExpression> aggrExprs,
+                       @JsonProperty("cardinality") float cardinality) {
     super(child);
     this.groupByExprs = groupByExprs;
     this.aggrExprs = aggrExprs;
     this.cardinality = cardinality;
   }
 
-  public NamedExpression[] getGroupByExprs() {
+  public List<NamedExpression> getGroupByExprs() {
     return groupByExprs;
   }
 
-  public NamedExpression[] getAggrExprs() {
+  public List<NamedExpression> getAggrExprs() {
     return aggrExprs;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/StreamingAggregate.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/StreamingAggregate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/StreamingAggregate.java
index 9486f6d..5669166 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/StreamingAggregate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/StreamingAggregate.java
@@ -27,29 +27,31 @@ import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonTypeName;
 
+import java.util.List;
+
 @JsonTypeName("streaming-aggregate")
 public class StreamingAggregate extends AbstractSingle {
 
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(StreamingAggregate.class);
 
-  private final NamedExpression[] keys;
-  private final NamedExpression[] exprs;
+  private final List<NamedExpression> keys;
+  private final List<NamedExpression> exprs;
 
   private final float cardinality;
 
   @JsonCreator
-  public StreamingAggregate(@JsonProperty("child") PhysicalOperator child, @JsonProperty("keys") NamedExpression[] keys, @JsonProperty("exprs") NamedExpression[] exprs, @JsonProperty("cardinality") float cardinality) {
+  public StreamingAggregate(@JsonProperty("child") PhysicalOperator child, @JsonProperty("keys") List<NamedExpression> keys, @JsonProperty("exprs") List<NamedExpression> exprs, @JsonProperty("cardinality") float cardinality) {
     super(child);
     this.keys = keys;
     this.exprs = exprs;
     this.cardinality = cardinality;
   }
 
-  public NamedExpression[] getKeys() {
+  public List<NamedExpression> getKeys() {
     return keys;
   }
 
-  public NamedExpression[] getExprs() {
+  public List<NamedExpression> getExprs() {
     return exprs;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/UnionAll.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/UnionAll.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/UnionAll.java
index b703a9d..54ba588 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/UnionAll.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/UnionAll.java
@@ -35,7 +35,7 @@ public class UnionAll extends AbstractMultiple {
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(Filter.class);
 
   @JsonCreator
-  public UnionAll(@JsonProperty("children") PhysicalOperator[] children) {
+  public UnionAll(@JsonProperty("children") List<PhysicalOperator> children) {
     super(children);
   }
 
@@ -46,7 +46,7 @@ public class UnionAll extends AbstractMultiple {
 
   @Override
   public PhysicalOperator getNewWithChildren(List<PhysicalOperator> children) {
-    return new UnionAll(children.toArray(new PhysicalOperator[children.size()]));
+    return new UnionAll(children);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/WindowPOP.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/WindowPOP.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/WindowPOP.java
index ec5f361..e98585d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/WindowPOP.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/WindowPOP.java
@@ -29,20 +29,22 @@ import org.apache.drill.exec.physical.base.PhysicalOperator;
 import org.apache.drill.exec.physical.base.PhysicalVisitor;
 import org.apache.drill.exec.proto.UserBitShared;
 
+import java.util.List;
+
 @JsonTypeName("window")
 public class WindowPOP extends AbstractSingle {
 
-  private final NamedExpression[] withins;
-  private final NamedExpression[] aggregations;
-  private final Order.Ordering[] orderings;
+  private final List<NamedExpression> withins;
+  private final List<NamedExpression> aggregations;
+  private final List<Order.Ordering> orderings;
   private final boolean frameUnitsRows;
   private final Bound start;
   private final Bound end;
 
   public WindowPOP(@JsonProperty("child") PhysicalOperator child,
-                   @JsonProperty("within") NamedExpression[] withins,
-                   @JsonProperty("aggregations") NamedExpression[] aggregations,
-                   @JsonProperty("orderings") Order.Ordering[] orderings,
+                   @JsonProperty("within") List<NamedExpression> withins,
+                   @JsonProperty("aggregations") List<NamedExpression> aggregations,
+                   @JsonProperty("orderings") List<Order.Ordering> orderings,
                    @JsonProperty("frameUnitsRows") boolean frameUnitsRows,
                    @JsonProperty("start") Bound start,
                    @JsonProperty("end") Bound end) {
@@ -78,15 +80,15 @@ public class WindowPOP extends AbstractSingle {
     return end;
   }
 
-  public NamedExpression[] getAggregations() {
+  public List<NamedExpression> getAggregations() {
     return aggregations;
   }
 
-  public NamedExpression[] getWithins() {
+  public List<NamedExpression> getWithins() {
     return withins;
   }
 
-  public Order.Ordering[] getOrderings() {
+  public List<Order.Ordering> getOrderings() {
     return orderings;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java
index 3595ecf..d826922 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java
@@ -182,8 +182,8 @@ public class HashAggBatch extends AbstractRecordBatch<HashAggregate> {
 
     container.clear();
 
-    int numGroupByExprs = (popConfig.getGroupByExprs() != null) ? popConfig.getGroupByExprs().length : 0;
-    int numAggrExprs = (popConfig.getAggrExprs() != null) ? popConfig.getAggrExprs().length : 0;
+    int numGroupByExprs = (popConfig.getGroupByExprs() != null) ? popConfig.getGroupByExprs().size() : 0;
+    int numAggrExprs = (popConfig.getAggrExprs() != null) ? popConfig.getAggrExprs().size() : 0;
     aggrExprs = new LogicalExpression[numAggrExprs];
     groupByOutFieldIds = new TypedFieldId[numGroupByExprs];
     aggrOutFieldIds = new TypedFieldId[numAggrExprs];
@@ -193,7 +193,7 @@ public class HashAggBatch extends AbstractRecordBatch<HashAggregate> {
     int i;
 
     for (i = 0; i < numGroupByExprs; i++) {
-      NamedExpression ne = popConfig.getGroupByExprs()[i];
+      NamedExpression ne = popConfig.getGroupByExprs().get(i);
       final LogicalExpression expr =
           ExpressionTreeMaterializer.materialize(ne.getExpr(), incoming, collector, context.getFunctionRegistry());
       if (expr == null) {
@@ -208,7 +208,7 @@ public class HashAggBatch extends AbstractRecordBatch<HashAggregate> {
     }
 
     for (i = 0; i < numAggrExprs; i++) {
-      NamedExpression ne = popConfig.getAggrExprs()[i];
+      NamedExpression ne = popConfig.getAggrExprs().get(i);
       final LogicalExpression expr =
           ExpressionTreeMaterializer.materialize(ne.getExpr(), incoming, collector, context.getFunctionRegistry());
 
@@ -239,7 +239,8 @@ public class HashAggBatch extends AbstractRecordBatch<HashAggregate> {
     HashAggregator agg = context.getImplementationClass(top);
 
     HashTableConfig htConfig =
-        new HashTableConfig(context.getOptions().getOption(ExecConstants.MIN_HASH_TABLE_SIZE_KEY).num_val.intValue(),
+        // TODO - fix the validator on this option
+        new HashTableConfig((int)context.getOptions().getOption(ExecConstants.MIN_HASH_TABLE_SIZE),
             HashTable.DEFAULT_LOAD_FACTOR, popConfig.getGroupByExprs(), null /* no probe exprs */);
 
     agg.setup(popConfig, htConfig, context, this.stats,

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
index 9ff874f..5e08163 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
@@ -247,7 +247,7 @@ public abstract class HashAggTemplate implements HashAggregator {
     //      e.g SELECT COUNT(DISTINCT a1) FROM t1 ;
     // we need to build a hash table on the aggregation column a1.
     // TODO:  This functionality will be added later.
-    if (hashAggrConfig.getGroupByExprs().length == 0) {
+    if (hashAggrConfig.getGroupByExprs().size() == 0) {
       throw new IllegalArgumentException("Currently, hash aggregation is only applicable if there are group-by " +
           "expressions.");
     }

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
index 19232f8..9d883f3 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
@@ -139,7 +139,7 @@ public class StreamingAggBatch extends AbstractRecordBatch<StreamingAggregate> {
       logger.debug("Next outcome of {}", outcome);
       switch (outcome) {
       case NONE:
-        if (first && popConfig.getKeys().length == 0) {
+        if (first && popConfig.getKeys().size() == 0) {
           // if we have a straight aggregate and empty input batch, we need to handle it in a different way
           constructSpecialBatch();
           first = false;
@@ -225,7 +225,7 @@ public class StreamingAggBatch extends AbstractRecordBatch<StreamingAggregate> {
            * buffer
            */
           throw new DrillRuntimeException("FixedWidth vectors is the expected output vector type. " +
-              "Corresponding expression: " + popConfig.getExprs()[exprIndex].toString());
+              "Corresponding expression: " + popConfig.getExprs().get(exprIndex).toString());
         }
       }
       exprIndex++;
@@ -260,14 +260,14 @@ public class StreamingAggBatch extends AbstractRecordBatch<StreamingAggregate> {
     ClassGenerator<StreamingAggregator> cg = CodeGenerator.getRoot(StreamingAggTemplate.TEMPLATE_DEFINITION, context.getFunctionRegistry());
     container.clear();
 
-    LogicalExpression[] keyExprs = new LogicalExpression[popConfig.getKeys().length];
-    LogicalExpression[] valueExprs = new LogicalExpression[popConfig.getExprs().length];
-    TypedFieldId[] keyOutputIds = new TypedFieldId[popConfig.getKeys().length];
+    LogicalExpression[] keyExprs = new LogicalExpression[popConfig.getKeys().size()];
+    LogicalExpression[] valueExprs = new LogicalExpression[popConfig.getExprs().size()];
+    TypedFieldId[] keyOutputIds = new TypedFieldId[popConfig.getKeys().size()];
 
     ErrorCollector collector = new ErrorCollectorImpl();
 
     for (int i = 0; i < keyExprs.length; i++) {
-      final NamedExpression ne = popConfig.getKeys()[i];
+      final NamedExpression ne = popConfig.getKeys().get(i);
       final LogicalExpression expr = ExpressionTreeMaterializer.materialize(ne.getExpr(), incoming, collector,context.getFunctionRegistry() );
       if (expr == null) {
         continue;
@@ -279,7 +279,7 @@ public class StreamingAggBatch extends AbstractRecordBatch<StreamingAggregate> {
     }
 
     for (int i = 0; i < valueExprs.length; i++) {
-      final NamedExpression ne = popConfig.getExprs()[i];
+      final NamedExpression ne = popConfig.getExprs().get(i);
       final LogicalExpression expr = ExpressionTreeMaterializer.materialize(ne.getExpr(), incoming, collector, context.getFunctionRegistry());
       if (expr instanceof IfExpression) {
         throw UserException.unsupportedError(new UnsupportedOperationException("Union type not supported in aggregate functions")).build(logger);

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
index bd34d76..cfd95e9 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
@@ -143,17 +143,17 @@ public class ChainedHashTable {
     ClassGenerator<HashTable> cg = top.getRoot();
     ClassGenerator<HashTable> cgInner = cg.getInnerGenerator("BatchHolder");
 
-    LogicalExpression[] keyExprsBuild = new LogicalExpression[htConfig.getKeyExprsBuild().length];
+    LogicalExpression[] keyExprsBuild = new LogicalExpression[htConfig.getKeyExprsBuild().size()];
     LogicalExpression[] keyExprsProbe = null;
     boolean isProbe = (htConfig.getKeyExprsProbe() != null);
     if (isProbe) {
-      keyExprsProbe = new LogicalExpression[htConfig.getKeyExprsProbe().length];
+      keyExprsProbe = new LogicalExpression[htConfig.getKeyExprsProbe().size()];
     }
 
     ErrorCollector collector = new ErrorCollectorImpl();
     VectorContainer htContainerOrig = new VectorContainer(); // original ht container from which others may be cloned
-    LogicalExpression[] htKeyExprs = new LogicalExpression[htConfig.getKeyExprsBuild().length];
-    TypedFieldId[] htKeyFieldIds = new TypedFieldId[htConfig.getKeyExprsBuild().length];
+    LogicalExpression[] htKeyExprs = new LogicalExpression[htConfig.getKeyExprsBuild().size()];
+    TypedFieldId[] htKeyFieldIds = new TypedFieldId[htConfig.getKeyExprsBuild().size()];
 
     int i = 0;
     for (NamedExpression ne : htConfig.getKeyExprsBuild()) {
@@ -210,7 +210,7 @@ public class ChainedHashTable {
     setupSetValue(cgInner, keyExprsBuild, htKeyFieldIds);
     if (outgoing != null) {
 
-      if (outKeyFieldIds.length > htConfig.getKeyExprsBuild().length) {
+      if (outKeyFieldIds.length > htConfig.getKeyExprsBuild().size()) {
         throw new IllegalArgumentException("Mismatched number of output key fields.");
       }
     }

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableConfig.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableConfig.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableConfig.java
index fa6d4b5..a6b2587 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableConfig.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableConfig.java
@@ -23,6 +23,8 @@ import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonTypeName;
 
+import java.util.List;
+
 @JsonTypeName("hashtable-config")
 public class HashTableConfig  {
 
@@ -30,13 +32,13 @@ public class HashTableConfig  {
 
   private final int initialCapacity;
   private final float loadFactor;
-  private final NamedExpression[] keyExprsBuild;
-  private final NamedExpression[] keyExprsProbe;
+  private final List<NamedExpression> keyExprsBuild;
+  private final List<NamedExpression> keyExprsProbe;
 
   @JsonCreator
   public HashTableConfig(@JsonProperty("initialCapacity") int initialCapacity, @JsonProperty("loadFactor") float loadFactor,
-                         @JsonProperty("keyExprsBuild") NamedExpression[] keyExprsBuild,
-                         @JsonProperty("keyExprsProbe") NamedExpression[] keyExprsProbe) {
+                         @JsonProperty("keyExprsBuild") List<NamedExpression> keyExprsBuild,
+                         @JsonProperty("keyExprsProbe") List<NamedExpression> keyExprsProbe) {
     this.initialCapacity = initialCapacity;
     this.loadFactor = loadFactor;
     this.keyExprsBuild = keyExprsBuild;
@@ -51,11 +53,11 @@ public class HashTableConfig  {
     return loadFactor;
   }
 
-  public NamedExpression[] getKeyExprsBuild() {
+  public List<NamedExpression> getKeyExprsBuild() {
     return keyExprsBuild;
   }
 
-  public NamedExpression[] getKeyExprsProbe() {
+  public List<NamedExpression> getKeyExprsProbe() {
     return keyExprsProbe;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
index 7d9f568..efd695e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
@@ -447,7 +447,7 @@ public abstract class HashTableTemplate implements HashTable {
       throw new IllegalArgumentException("The initial capacity must be less than maximum capacity allowed");
     }
 
-    if (htConfig.getKeyExprsBuild() == null || htConfig.getKeyExprsBuild().length == 0) {
+    if (htConfig.getKeyExprsBuild() == null || htConfig.getKeyExprsBuild().size() == 0) {
       throw new IllegalArgumentException("Hash table must have at least 1 key expression");
     }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
index 3ea97c6..2ba54dd 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
@@ -18,6 +18,7 @@
 package org.apache.drill.exec.physical.impl.join;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.drill.common.expression.FieldReference;
@@ -281,14 +282,14 @@ public class HashJoinBatch extends AbstractRecordBatch<HashJoinPOP> {
   public void setupHashTable() throws IOException, SchemaChangeException, ClassTransformationException {
     // Setup the hash table configuration object
     int conditionsSize = conditions.size();
-    final NamedExpression rightExpr[] = new NamedExpression[conditionsSize];
-    NamedExpression leftExpr[] = new NamedExpression[conditionsSize];
+    final List<NamedExpression> rightExpr = new ArrayList<>(conditionsSize);
+    List<NamedExpression> leftExpr = new ArrayList<>(conditionsSize);
 
     JoinComparator comparator = JoinComparator.NONE;
     // Create named expressions from the conditions
     for (int i = 0; i < conditionsSize; i++) {
-      rightExpr[i] = new NamedExpression(conditions.get(i).getRight(), new FieldReference("build_side_" + i));
-      leftExpr[i] = new NamedExpression(conditions.get(i).getLeft(), new FieldReference("probe_side_" + i));
+      rightExpr.add(new NamedExpression(conditions.get(i).getRight(), new FieldReference("build_side_" + i)));
+      leftExpr.add(new NamedExpression(conditions.get(i).getLeft(), new FieldReference("probe_side_" + i)));
 
       // Hash join only supports certain types of comparisons
       comparator = JoinUtils.checkAndSetComparison(conditions.get(i), comparator);

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java
index 46a6c0e..d2c9e45 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java
@@ -260,7 +260,7 @@ public class WindowFrameRecordBatch extends AbstractRecordBatch<WindowPOP> {
     boolean useDefaultFrame = false; // at least one window function uses the DefaultFrameTemplate
     boolean useCustomFrame = false; // at least one window function uses the CustomFrameTemplate
 
-    hasOrderBy = popConfig.getOrderings().length > 0;
+    hasOrderBy = popConfig.getOrderings().size() > 0;
 
     // all existing vectors will be transferred to the outgoing container in framer.doWork()
     for (final VectorWrapper<?> wrapper : batch) {

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFunction.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFunction.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFunction.java
index 191dad1..cd14b8a 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFunction.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFunction.java
@@ -157,7 +157,7 @@ public abstract class WindowFunction {
 
     @Override
     public boolean requiresFullPartition(final WindowPOP pop) {
-      return pop.getOrderings().length == 0 || pop.getEnd().isUnbounded();
+      return pop.getOrderings().isEmpty() || pop.getEnd().isUnbounded();
     }
 
     @Override
@@ -451,7 +451,7 @@ public abstract class WindowFunction {
 
     @Override
     public boolean requiresFullPartition(final WindowPOP pop) {
-      return pop.getOrderings().length == 0 || pop.getEnd().isUnbounded();
+      return pop.getOrderings().isEmpty() || pop.getEnd().isUnbounded();
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashAggPrel.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashAggPrel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashAggPrel.java
index 637fb8e..44bf170 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashAggPrel.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashAggPrel.java
@@ -73,10 +73,7 @@ public class HashAggPrel extends AggPrelBase implements Prel{
   public PhysicalOperator getPhysicalOperator(PhysicalPlanCreator creator) throws IOException {
 
     Prel child = (Prel) this.getInput();
-    HashAggregate g = new HashAggregate(child.getPhysicalOperator(creator),
-        keys.toArray(new NamedExpression[keys.size()]),
-        aggExprs.toArray(new NamedExpression[aggExprs.size()]),
-        1.0f);
+    HashAggregate g = new HashAggregate(child.getPhysicalOperator(creator), keys, aggExprs, 1.0f);
 
     return creator.addMetadata(this, g);
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamAggPrel.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamAggPrel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamAggPrel.java
index 80fe1a1..c3e8afa 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamAggPrel.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/StreamAggPrel.java
@@ -87,8 +87,7 @@ public class StreamAggPrel extends AggPrelBase implements Prel{
   public PhysicalOperator getPhysicalOperator(PhysicalPlanCreator creator) throws IOException {
 
     Prel child = (Prel) this.getInput();
-    StreamingAggregate g = new StreamingAggregate(child.getPhysicalOperator(creator), keys.toArray(new NamedExpression[keys.size()]),
-        aggExprs.toArray(new NamedExpression[aggExprs.size()]), 1.0f);
+    StreamingAggregate g = new StreamingAggregate(child.getPhysicalOperator(creator), keys, aggExprs, 1.0f);
 
     return creator.addMetadata(this, g);
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionAllPrel.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionAllPrel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionAllPrel.java
index ff2ad1b..4282a3f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionAllPrel.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionAllPrel.java
@@ -79,7 +79,7 @@ public class UnionAllPrel extends UnionPrel {
       inputPops.add( ((Prel)this.getInputs().get(i)).getPhysicalOperator(creator));
     }
 
-    UnionAll unionall = new UnionAll(inputPops.toArray(new PhysicalOperator[inputPops.size()]));
+    UnionAll unionall = new UnionAll(inputPops);
     return creator.addMetadata(this, unionall);
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionDistinctPrel.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionDistinctPrel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionDistinctPrel.java
index 4ef3375..5cda5a6 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionDistinctPrel.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnionDistinctPrel.java
@@ -78,7 +78,7 @@ public class UnionDistinctPrel extends UnionPrel {
     }
 
     ///TODO: change this to UnionDistinct once implemented end-to-end..
-    UnionAll unionAll = new UnionAll(inputPops.toArray(new PhysicalOperator[inputPops.size()]));
+    UnionAll unionAll = new UnionAll(inputPops);
     return creator.addMetadata(this, unionAll);
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrel.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrel.java
index 1a89bd7..bf52366 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrel.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WindowPrel.java
@@ -102,9 +102,9 @@ public class WindowPrel extends DrillWindowRelBase implements Prel {
 
     WindowPOP windowPOP = new WindowPOP(
         childPOP,
-        withins.toArray(new NamedExpression[withins.size()]),
-        aggs.toArray(new NamedExpression[aggs.size()]),
-        orderings.toArray(new Order.Ordering[orderings.size()]),
+        withins,
+        aggs,
+        orderings,
         window.isRows,
         WindowPOP.newBound(window.lowerBound),
         WindowPOP.newBound(window.upperBound));

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/AbstractBuilder.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/AbstractBuilder.java b/logical/src/main/java/org/apache/drill/common/logical/data/AbstractBuilder.java
index b178cd9..d3e1330 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/AbstractBuilder.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/AbstractBuilder.java
@@ -26,21 +26,4 @@ public abstract class AbstractBuilder<T extends LogicalOperator> {
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(AbstractBuilder.class);
 
   public abstract T build();
-
-  protected LogicalExpression[] aL(List<LogicalExpression> exprs){
-    return exprs.toArray(new LogicalExpression[exprs.size()]);
-  }
-
-  protected FieldReference[] aF(List<FieldReference> exprs){
-    return exprs.toArray(new FieldReference[exprs.size()]);
-  }
-
-  protected NamedExpression[] aN(List<NamedExpression> exprs){
-    return exprs.toArray(new NamedExpression[exprs.size()]);
-  }
-
-  protected Order.Ordering[] aO(List<Order.Ordering> orderings) {
-    return orderings.toArray(new Order.Ordering[orderings.size()]);
-  }
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/GroupingAggregate.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/GroupingAggregate.java b/logical/src/main/java/org/apache/drill/common/logical/data/GroupingAggregate.java
index 2913a7e..f554d96 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/GroupingAggregate.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/GroupingAggregate.java
@@ -33,10 +33,10 @@ import com.google.common.collect.Lists;
 public class GroupingAggregate extends SingleInputOperator{
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(GroupingAggregate.class);
 
-  private final NamedExpression[] keys;
-  private final NamedExpression[] exprs;
+  private final List<NamedExpression> keys;
+  private final List<NamedExpression> exprs;
 
-  public GroupingAggregate(@JsonProperty("keys") NamedExpression[] keys, @JsonProperty("exprs") NamedExpression[] exprs) {
+  public GroupingAggregate(@JsonProperty("keys") List<NamedExpression> keys, @JsonProperty("exprs") List<NamedExpression> exprs) {
     super();
     this.keys = keys;
     this.exprs = exprs;
@@ -56,11 +56,11 @@ public class GroupingAggregate extends SingleInputOperator{
     return new Builder();
   }
 
-  public NamedExpression[] getKeys(){
+  public List<NamedExpression> getKeys(){
     return keys;
   }
 
-  public NamedExpression[] getExprs(){
+  public List<NamedExpression> getExprs(){
     return exprs;
   }
 
@@ -90,7 +90,7 @@ public class GroupingAggregate extends SingleInputOperator{
 
     @Override
     public GroupingAggregate internalBuild(){
-      GroupingAggregate ga =  new GroupingAggregate(aN(keys), aN(exprs));
+      GroupingAggregate ga =  new GroupingAggregate(keys, exprs);
       return ga;
     }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/Join.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/Join.java b/logical/src/main/java/org/apache/drill/common/logical/data/Join.java
index 2d7e6e1..bb1159c 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/Join.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/Join.java
@@ -38,7 +38,7 @@ public class Join extends LogicalOperatorBase {
   private final LogicalOperator left;
   private final LogicalOperator right;
   private final JoinRelType type;
-  private final JoinCondition[] conditions;
+  private final List<JoinCondition> conditions;
 
   public static JoinRelType resolve(String val) {
     for (JoinRelType jt : JoinRelType.values()) {
@@ -49,14 +49,17 @@ public class Join extends LogicalOperatorBase {
     throw new ExpressionParsingException(String.format("Unable to determine join type for value '%s'.", val));
   }
 
+  // TODO - should not have two @JsonCreators, need to figure out which one is being used
+  // I'm guess this one, is the case insensitive match in resolve() actually needed?
   @JsonCreator
   public Join(@JsonProperty("left") LogicalOperator left, @JsonProperty("right") LogicalOperator right,
-      @JsonProperty("conditions") JoinCondition[] conditions, @JsonProperty("type") String type) {
+      @JsonProperty("conditions") List<JoinCondition> conditions, @JsonProperty("type") String type) {
     this(left, right, conditions, resolve(type));
   }
 
   @JsonCreator
-  public Join(@JsonProperty("left") LogicalOperator left, @JsonProperty("right") LogicalOperator right, @JsonProperty("conditions")JoinCondition[] conditions, @JsonProperty("type") JoinRelType type) {
+  public Join(@JsonProperty("left") LogicalOperator left, @JsonProperty("right") LogicalOperator right,
+              @JsonProperty("conditions") List<JoinCondition> conditions, @JsonProperty("type") JoinRelType type) {
     super();
     this.conditions = conditions;
     this.left = left;
@@ -75,7 +78,7 @@ public class Join extends LogicalOperatorBase {
     return right;
   }
 
-  public JoinCondition[] getConditions() {
+  public List<JoinCondition> getConditions() {
     return conditions;
   }
 
@@ -132,7 +135,7 @@ public class Join extends LogicalOperatorBase {
       Preconditions.checkNotNull(left);
       Preconditions.checkNotNull(right);
       Preconditions.checkNotNull(type);
-      return new Join(left, right, conditions.toArray(new JoinCondition[conditions.size()]), type);
+      return new Join(left, right, conditions, type);
     }
 
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/Order.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/Order.java b/logical/src/main/java/org/apache/drill/common/logical/data/Order.java
index fca6010..ca3eec4 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/Order.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/Order.java
@@ -38,16 +38,16 @@ import com.google.common.collect.Lists;
 @JsonTypeName("order")
 public class Order extends SingleInputOperator {
 
-  private final Ordering[] orderings;
+  private final List<Ordering> orderings;
   private final FieldReference within;
 
   @JsonCreator
-  public Order(@JsonProperty("within") FieldReference within, @JsonProperty("orderings") Ordering... orderings) {
+  public Order(@JsonProperty("within") FieldReference within, @JsonProperty("orderings") List<Ordering> orderings) {
     this.orderings = orderings;
     this.within = within;
   }
 
-  public Ordering[] getOrderings() {
+  public List<Ordering> getOrderings() {
     return orderings;
   }
 
@@ -244,7 +244,7 @@ public class Order extends SingleInputOperator {
 
     @Override
     public Order internalBuild() {
-      return new Order(within, orderings.toArray(new Ordering[orderings.size()]));
+      return new Order(within, orderings);
     }
 
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/Project.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/Project.java b/logical/src/main/java/org/apache/drill/common/logical/data/Project.java
index 4345c80..fc981ce 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/Project.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/Project.java
@@ -32,30 +32,19 @@ import com.google.common.collect.Lists;
 @JsonTypeName("project")
 public class Project extends SingleInputOperator {
 
-  private final NamedExpression[] selections;
+  private final List<NamedExpression> selections;
 
   @JsonCreator
-  public Project(@JsonProperty("projections") NamedExpression[] selections) {
+  public Project(@JsonProperty("projections") List<NamedExpression> selections) {
     this.selections = selections;
-    if (selections == null || selections.length == 0) {
+    if (selections == null || selections.size() == 0) {
       throw new ExpressionParsingException(
           "Project did not provide any projection selections.  At least one projection must be provided.");
-//    for (int i = 0; i < selections.length; i++) {
-//      PathSegment segment = selections[i].getRef().getRootSegment();
-//      CharSequence path = segment.getNameSegment().getPath();
-//      if (!segment.isNamed() || !path.equals("output"))
-//        throw new ExpressionParsingException(
-//            String
-//                .format(
-//                    "Outputs for projections always have to start with named path of output. First segment was named '%s' or was named [%s]",
-//                    path, segment.isNamed()));
-//
-//    }
     }
   }
 
   @JsonProperty("projections")
-  public NamedExpression[] getSelections() {
+  public List<NamedExpression> getSelections() {
     return selections;
   }
 
@@ -84,7 +73,7 @@ public class Project extends SingleInputOperator {
 
     @Override
     public Project internalBuild() {
-      return new Project(aN(exprs));
+      return new Project(exprs);
     }
 
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/RunningAggregate.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/RunningAggregate.java b/logical/src/main/java/org/apache/drill/common/logical/data/RunningAggregate.java
index e280627..d71814b 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/RunningAggregate.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/RunningAggregate.java
@@ -18,6 +18,7 @@
 package org.apache.drill.common.logical.data;
 
 import java.util.Iterator;
+import java.util.List;
 
 import org.apache.drill.common.expression.FieldReference;
 import org.apache.drill.common.logical.data.visitors.LogicalVisitor;
@@ -31,10 +32,11 @@ import com.google.common.collect.Iterators;
 public class RunningAggregate extends SingleInputOperator{
 
   private final FieldReference within;
-  private final NamedExpression[]  aggregations;
+  private final List<NamedExpression> aggregations;
 
   @JsonCreator
-  public RunningAggregate(@JsonProperty("within") FieldReference within, @JsonProperty("aggregations") NamedExpression[] aggregations) {
+  public RunningAggregate(@JsonProperty("within") FieldReference within,
+                          @JsonProperty("aggregations") List<NamedExpression> aggregations) {
     super();
     this.within = within;
     this.aggregations = aggregations;
@@ -44,7 +46,7 @@ public class RunningAggregate extends SingleInputOperator{
     return within;
   }
 
-  public NamedExpression[] getAggregations() {
+  public List<NamedExpression> getAggregations() {
     return aggregations;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/Sequence.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/Sequence.java b/logical/src/main/java/org/apache/drill/common/logical/data/Sequence.java
index f04b09c..357c9b8 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/Sequence.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/Sequence.java
@@ -19,6 +19,7 @@ package org.apache.drill.common.logical.data;
 
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.List;
 
 import org.apache.drill.common.logical.data.Sequence.De;
 import org.apache.drill.common.logical.data.visitors.LogicalVisitor;
@@ -42,11 +43,15 @@ import com.fasterxml.jackson.databind.deser.impl.ReadableObjectId;
 import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
 import com.google.common.collect.Iterators;
 
+// TODO - is this even ever used anymore? I don't believe the planner will ever
+// generate this, we might have some tests with old logical plans that use this
+// but it should probably be removed
 /**
  * Describes a list of operators where each operator only has one input and that
  * input is the operator that came before.
  *
  */
+@Deprecated
 @JsonDeserialize(using = De.class)
 @JsonTypeName("sequence")
 public class Sequence extends LogicalOperatorBase {
@@ -57,7 +62,7 @@ public class Sequence extends LogicalOperatorBase {
   public boolean openTop;
   public LogicalOperator input;
   @JsonProperty("do")
-  public LogicalOperator[] stream;
+  public List<LogicalOperator> stream;
 
     @Override
     public <T, X, E extends Throwable> T accept(LogicalVisitor<T, X, E> logicalVisitor, X value) throws E {
@@ -66,7 +71,7 @@ public class Sequence extends LogicalOperatorBase {
 
     @Override
     public Iterator<LogicalOperator> iterator() {
-        return Iterators.singletonIterator(stream[stream.length - 1]);
+        return Iterators.singletonIterator(stream.get(stream.size() - 1));
     }
 
     public static class De extends StdDeserializer<LogicalOperator> {

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/Transform.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/Transform.java b/logical/src/main/java/org/apache/drill/common/logical/data/Transform.java
index 129ae2c..5fc6c70 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/Transform.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/Transform.java
@@ -18,6 +18,7 @@
 package org.apache.drill.common.logical.data;
 
 import java.util.Iterator;
+import java.util.List;
 
 import org.apache.drill.common.logical.data.visitors.LogicalVisitor;
 
@@ -29,15 +30,15 @@ import com.google.common.collect.Iterators;
 @JsonTypeName("transform")
 public class Transform extends SingleInputOperator {
 
-  private final NamedExpression[] transforms;
+  private final List<NamedExpression> transforms;
 
   @JsonCreator
-  public Transform(@JsonProperty("transforms") NamedExpression[] transforms) {
+  public Transform(@JsonProperty("transforms") List<NamedExpression> transforms) {
     super();
     this.transforms = transforms;
   }
 
-  public NamedExpression[] getTransforms() {
+  public List<NamedExpression> getTransforms() {
     return transforms;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/Union.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/Union.java b/logical/src/main/java/org/apache/drill/common/logical/data/Union.java
index 113ffe0..909b126 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/Union.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/Union.java
@@ -30,16 +30,11 @@ import com.google.common.collect.Lists;
 
 @JsonTypeName("union")
 public class Union extends LogicalOperatorBase {
-  private final LogicalOperator[] inputs;
+  private final List<LogicalOperator> inputs;
   private final boolean distinct;
 
-//  @JsonCreator
-//  public Union(@JsonProperty("inputs") LogicalOperator[] inputs){
-//    this(inputs, false);
-//  }
-
   @JsonCreator
-  public Union(@JsonProperty("inputs") LogicalOperator[] inputs, @JsonProperty("distinct") Boolean distinct){
+  public Union(@JsonProperty("inputs") List<LogicalOperator> inputs, @JsonProperty("distinct") Boolean distinct){
     this.inputs = inputs;
       for (LogicalOperator o : inputs) {
           o.registerAsSubscriber(this);
@@ -47,7 +42,7 @@ public class Union extends LogicalOperatorBase {
     this.distinct = distinct == null ? false : distinct;
   }
 
-  public LogicalOperator[] getInputs() {
+  public List<LogicalOperator> getInputs() {
     return inputs;
   }
 
@@ -62,7 +57,7 @@ public class Union extends LogicalOperatorBase {
 
     @Override
     public Iterator<LogicalOperator> iterator() {
-        return Iterators.forArray(inputs);
+        return inputs.iterator();
     }
 
 
@@ -86,7 +81,7 @@ public class Union extends LogicalOperatorBase {
 
       @Override
       public Union build() {
-        return new Union(inputs.toArray(new LogicalOperator[inputs.size()]), distinct);
+        return new Union(inputs, distinct);
       }
 
     }

http://git-wip-us.apache.org/repos/asf/drill/blob/d24205d4/logical/src/main/java/org/apache/drill/common/logical/data/Window.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/Window.java b/logical/src/main/java/org/apache/drill/common/logical/data/Window.java
index 0f550dc..8a33416 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/Window.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/Window.java
@@ -36,17 +36,17 @@ import static com.google.common.base.Preconditions.checkState;
 
 @JsonTypeName("window")
 public class Window extends SingleInputOperator {
-  private final NamedExpression[] withins;
-  private final NamedExpression[] aggregations;
-  private final Order.Ordering[] orderings;
+  private final List<NamedExpression> withins;
+  private final List<NamedExpression> aggregations;
+  private final List<Order.Ordering> orderings;
   private final long start;
   private final long end;
 
 
   @JsonCreator
-  public Window(@JsonProperty("withins") NamedExpression[] withins,
-                @JsonProperty("aggregations") NamedExpression[] aggregations,
-                @JsonProperty("orderings") Order.Ordering[] orderings,
+  public Window(@JsonProperty("withins") List<NamedExpression> withins,
+                @JsonProperty("aggregations") List<NamedExpression> aggregations,
+                @JsonProperty("orderings") List<Order.Ordering> orderings,
                 @JsonProperty("start") Long start,
                 @JsonProperty("end") Long end) {
     super();
@@ -57,7 +57,7 @@ public class Window extends SingleInputOperator {
     this.orderings = orderings;
   }
 
-  public NamedExpression[] getWithins() {
+  public List<NamedExpression> getWithins() {
     return withins;
   }
 
@@ -69,11 +69,11 @@ public class Window extends SingleInputOperator {
     return end;
   }
 
-  public NamedExpression[] getAggregations() {
+  public List<NamedExpression> getAggregations() {
     return aggregations;
   }
 
-  public Order.Ordering[] getOrderings() {
+  public List<Order.Ordering> getOrderings() {
     return orderings;
   }
 
@@ -109,7 +109,7 @@ public class Window extends SingleInputOperator {
       //TODO withins can actually be empty: over(), over(order by <expression>), ...
       checkState(!withins.isEmpty(), "Withins in window must not be empty.");
       checkState(!aggregations.isEmpty(), "Aggregations in window must not be empty.");
-      return new Window(aN(withins), aN(aggregations), aO(orderings), start, end);
+      return new Window(withins, aggregations, orderings, start, end);
     }
 
     public Builder addOrdering(Order.Ordering ordering) {


[4/4] drill git commit: DRILL-4437: Operator unit test framework

Posted by js...@apache.org.
DRILL-4437: Operator unit test framework

Closes #394


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/d93a3633
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/d93a3633
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/d93a3633

Branch: refs/heads/master
Commit: d93a3633815ed1c7efd6660eae62b7351a2c9739
Parents: 01e04cd
Author: Jason Altekruse <al...@gmail.com>
Authored: Fri Feb 26 14:55:30 2016 -0800
Committer: Jason Altekruse <al...@gmail.com>
Committed: Wed Apr 20 09:07:13 2016 -0700

----------------------------------------------------------------------
 .../org/apache/drill/exec/ExecConstants.java    |   6 +-
 .../drill/exec/compile/ClassTransformer.java    |   4 +-
 .../drill/exec/physical/impl/ImplCreator.java   |   4 +-
 .../exec/physical/impl/join/HashJoinBatch.java  |   2 +-
 .../physical/impl/xsort/ExternalSortBatch.java  |   4 +
 .../exec/store/easy/json/JSONRecordReader.java  |   4 +-
 .../java/org/apache/drill/DrillTestWrapper.java | 215 ++++++++----
 .../test/java/org/apache/drill/TestBuilder.java |   4 -
 .../physical/unit/BasicPhysicalOpUnitTest.java  | 322 +++++++++++++++++
 .../physical/unit/PhysicalOpUnitTestBase.java   | 341 +++++++++++++++++++
 .../apache/drill/jdbc/test/JdbcDataTest.java    |   6 +-
 11 files changed, 834 insertions(+), 78 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
index a490116..6a0889d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
@@ -149,7 +149,7 @@ public interface ExecConstants {
   OptionValidator FILESYSTEM_PARTITION_COLUMN_LABEL_VALIDATOR = new StringValidator(FILESYSTEM_PARTITION_COLUMN_LABEL, "dir");
 
   String JSON_READ_NUMBERS_AS_DOUBLE = "store.json.read_numbers_as_double";
-  OptionValidator JSON_READ_NUMBERS_AS_DOUBLE_VALIDATOR = new BooleanValidator(JSON_READ_NUMBERS_AS_DOUBLE, false);
+  BooleanValidator JSON_READ_NUMBERS_AS_DOUBLE_VALIDATOR = new BooleanValidator(JSON_READ_NUMBERS_AS_DOUBLE, false);
 
   String MONGO_ALL_TEXT_MODE = "store.mongo.all_text_mode";
   OptionValidator MONGO_READER_ALL_TEXT_MODE_VALIDATOR = new BooleanValidator(MONGO_ALL_TEXT_MODE, false);
@@ -178,9 +178,9 @@ public interface ExecConstants {
    * HashTable runtime settings
    */
   String MIN_HASH_TABLE_SIZE_KEY = "exec.min_hash_table_size";
-  OptionValidator MIN_HASH_TABLE_SIZE = new PositiveLongValidator(MIN_HASH_TABLE_SIZE_KEY, HashTable.MAXIMUM_CAPACITY, HashTable.DEFAULT_INITIAL_CAPACITY);
+  PositiveLongValidator MIN_HASH_TABLE_SIZE = new PositiveLongValidator(MIN_HASH_TABLE_SIZE_KEY, HashTable.MAXIMUM_CAPACITY, HashTable.DEFAULT_INITIAL_CAPACITY);
   String MAX_HASH_TABLE_SIZE_KEY = "exec.max_hash_table_size";
-  OptionValidator MAX_HASH_TABLE_SIZE = new PositiveLongValidator(MAX_HASH_TABLE_SIZE_KEY, HashTable.MAXIMUM_CAPACITY, HashTable.MAXIMUM_CAPACITY);
+  PositiveLongValidator MAX_HASH_TABLE_SIZE = new PositiveLongValidator(MAX_HASH_TABLE_SIZE_KEY, HashTable.MAXIMUM_CAPACITY, HashTable.MAXIMUM_CAPACITY);
 
   /**
    * Limits the maximum level of parallelization to this factor time the number of Drillbits

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java
index 3c93599..02323a9 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java
@@ -216,9 +216,7 @@ public class ClassTransformer {
       final String entireClass,
       final String materializedClassName) throws ClassTransformationException {
     // unfortunately, this hasn't been set up at construction time, so we have to do it here
-    final OptionValue optionValue = optionManager.getOption(SCALAR_REPLACEMENT_OPTION);
-    final ScalarReplacementOption scalarReplacementOption =
-        ScalarReplacementOption.fromString((String) optionValue.getValue()); // TODO(DRILL-2474)
+    final ScalarReplacementOption scalarReplacementOption = ScalarReplacementOption.fromString(optionManager.getOption(SCALAR_REPLACEMENT_VALIDATOR));
 
     try {
       final long t1 = System.nanoTime();

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java
index 8a8a1ae..5872ef1 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java
@@ -23,6 +23,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.drill.common.AutoCloseables;
 import org.apache.drill.common.exceptions.ExecutionSetupException;
 import org.apache.drill.exec.ops.FragmentContext;
@@ -120,7 +121,8 @@ public class ImplCreator {
 
 
   /** Create a RecordBatch and its children for given PhysicalOperator */
-  private RecordBatch getRecordBatch(final PhysicalOperator op, final FragmentContext context) throws ExecutionSetupException {
+  @VisibleForTesting
+  public RecordBatch getRecordBatch(final PhysicalOperator op, final FragmentContext context) throws ExecutionSetupException {
     Preconditions.checkNotNull(op);
 
     final List<RecordBatch> childRecordBatches = getChildren(op, context);

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
index 2ba54dd..2ace69e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
@@ -308,7 +308,7 @@ public class HashJoinBatch extends AbstractRecordBatch<HashJoinPOP> {
     }
 
     final HashTableConfig htConfig =
-        new HashTableConfig(context.getOptions().getOption(ExecConstants.MIN_HASH_TABLE_SIZE_KEY).num_val.intValue(),
+        new HashTableConfig((int) context.getOptions().getOption(ExecConstants.MIN_HASH_TABLE_SIZE),
             HashTable.DEFAULT_LOAD_FACTOR, rightExpr, leftExpr);
 
     // Create the chained hash table

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/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 7797339..0ee518e 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
@@ -374,6 +374,10 @@ public class ExternalSortBatch extends AbstractRecordBatch<ExternalSort> {
                 (spillCount == 0 && !hasMemoryForInMemorySort(totalCount)) ||
                 // If we haven't spilled so far, make sure we don't exceed the maximum number of batches SV4 can address
                 (spillCount == 0 && totalBatches > Character.MAX_VALUE) ||
+                // TODO(DRILL-4438) - consider setting this threshold more intelligently,
+                // lowering caused a failing low memory condition (test in BasicPhysicalOpUnitTest)
+                // to complete successfully (although it caused perf decrease as there was more spilling)
+
                 // current memory used is more than 95% of memory usage limit of this operator
                 (oAllocator.getAllocatedMemory() > .95 * oAllocator.getLimit()) ||
                 // Number of incoming batches (BatchGroups) exceed the limit and number of incoming batches accumulated

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java
index e943401..dbbe6b0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java
@@ -112,8 +112,8 @@ public class JSONRecordReader extends AbstractRecordReader {
 
     // only enable all text mode if we aren't using embedded content mode.
     this.enableAllTextMode = embeddedContent == null && fragmentContext.getOptions().getOption(ExecConstants.JSON_READER_ALL_TEXT_MODE_VALIDATOR);
-    this.readNumbersAsDouble = fragmentContext.getOptions().getOption(ExecConstants.JSON_READ_NUMBERS_AS_DOUBLE).bool_val;
-    this.unionEnabled = fragmentContext.getOptions().getOption(ExecConstants.ENABLE_UNION_TYPE);
+    this.readNumbersAsDouble = embeddedContent == null && fragmentContext.getOptions().getOption(ExecConstants.JSON_READ_NUMBERS_AS_DOUBLE_VALIDATOR);
+    this.unionEnabled = embeddedContent == null && fragmentContext.getOptions().getOption(ExecConstants.ENABLE_UNION_TYPE);
     setColumns(columns);
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java b/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java
index f853414..2a9c03d 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java
@@ -24,8 +24,10 @@ import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Array;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -44,7 +46,10 @@ import org.apache.drill.exec.record.BatchSchema;
 import org.apache.drill.exec.record.HyperVectorWrapper;
 import org.apache.drill.exec.record.MaterializedField;
 import org.apache.drill.exec.record.RecordBatchLoader;
+import org.apache.drill.exec.record.VectorAccessible;
 import org.apache.drill.exec.record.VectorWrapper;
+import org.apache.drill.exec.record.selection.SelectionVector2;
+import org.apache.drill.exec.record.selection.SelectionVector4;
 import org.apache.drill.exec.rpc.user.QueryDataBatch;
 import org.apache.drill.exec.util.Text;
 import org.apache.drill.exec.vector.ValueVector;
@@ -147,19 +152,19 @@ public class DrillTestWrapper {
         i++;
       }
     }
-    for (HyperVectorValueIterator hvi : expectedRecords.values()) {
-      for (ValueVector vv : hvi.getHyperVector().getValueVectors()) {
-        vv.clear();
-      }
-    }
-    for (HyperVectorValueIterator hvi : actualRecords.values()) {
+    cleanupHyperValueIterators(expectedRecords.values());
+    cleanupHyperValueIterators(actualRecords.values());
+  }
+
+  private void cleanupHyperValueIterators(Collection<HyperVectorValueIterator> hyperBatches) {
+    for (HyperVectorValueIterator hvi : hyperBatches) {
       for (ValueVector vv : hvi.getHyperVector().getValueVectors()) {
         vv.clear();
       }
     }
   }
 
-  private void compareMergedVectors(Map<String, List<Object>> expectedRecords, Map<String, List<Object>> actualRecords) throws Exception {
+  public static void compareMergedVectors(Map<String, List<Object>> expectedRecords, Map<String, List<Object>> actualRecords) throws Exception {
     for (String s : actualRecords.keySet()) {
       assertNotNull("Unexpected extra column " + s + " returned by query.", expectedRecords.get(s));
       assertEquals("Incorrect number of rows returned by query.", expectedRecords.get(s).size(), actualRecords.get(s).size());
@@ -180,7 +185,7 @@ public class DrillTestWrapper {
     }
   }
 
-  private String printNearbyRecords(Map<String, List<Object>> expectedRecords, Map<String, List<Object>> actualRecords, int offset) {
+  private static String printNearbyRecords(Map<String, List<Object>> expectedRecords, Map<String, List<Object>> actualRecords, int offset) {
     StringBuilder expected = new StringBuilder();
     StringBuilder actual = new StringBuilder();
     expected.append("Expected Records near verification failure:\n");
@@ -208,8 +213,9 @@ public class DrillTestWrapper {
 
   }
 
-  private Map<String, HyperVectorValueIterator> addToHyperVectorMap(List<QueryDataBatch> records, RecordBatchLoader loader,
-                                                                      BatchSchema schema) throws SchemaChangeException, UnsupportedEncodingException {
+  private Map<String, HyperVectorValueIterator> addToHyperVectorMap(final List<QueryDataBatch> records,
+                                                                    final RecordBatchLoader loader)
+      throws SchemaChangeException, UnsupportedEncodingException {
     // TODO - this does not handle schema changes
     Map<String, HyperVectorValueIterator> combinedVectors = new TreeMap<>();
 
@@ -218,7 +224,6 @@ public class DrillTestWrapper {
     int size = records.size();
     for (int i = 0; i < size; i++) {
       batch = records.get(i);
-      loader = new RecordBatchLoader(getAllocator());
       loader.load(batch.getHeader().getDef(), batch.getData());
       logger.debug("reading batch with " + loader.getRecordCount() + " rows, total read so far " + totalRecords);
       totalRecords += loader.getRecordCount();
@@ -241,30 +246,70 @@ public class DrillTestWrapper {
     return combinedVectors;
   }
 
+  private static class BatchIterator implements Iterable<VectorAccessible>, AutoCloseable {
+    private final List<QueryDataBatch> dataBatches;
+    private final RecordBatchLoader batchLoader;
+
+    public BatchIterator(List<QueryDataBatch> dataBatches, RecordBatchLoader batchLoader) {
+      this.dataBatches = dataBatches;
+      this.batchLoader = batchLoader;
+    }
+
+    @Override
+    public Iterator<VectorAccessible> iterator() {
+      return new Iterator<VectorAccessible>() {
+
+        int index = -1;
+
+        @Override
+        public boolean hasNext() {
+          return index < dataBatches.size() - 1;
+        }
+
+        @Override
+        public VectorAccessible next() {
+          index++;
+          if (index == dataBatches.size()) {
+            throw new RuntimeException("Tried to call next when iterator had no more items.");
+          }
+          batchLoader.clear();
+          QueryDataBatch batch = dataBatches.get(index);
+          try {
+            batchLoader.load(batch.getHeader().getDef(), batch.getData());
+          } catch (SchemaChangeException e) {
+            throw new RuntimeException(e);
+          }
+          return batchLoader;
+        }
+
+        @Override
+        public void remove() {
+          throw new UnsupportedOperationException("Removing is not supported");
+        }
+      };
+    }
+
+    @Override
+    public void close() throws Exception {
+      batchLoader.clear();
+    }
+
+  }
+
   /**
-   * Only use this method if absolutely needed. There are utility methods to compare results of single queries.
-   * The current use case for exposing this is setting session or system options between the test and verification
-   * queries.
-   *
-   * TODO - evaluate adding an interface to allow setting session and system options before running queries
-   * @param records
-   * @param loader
-   * @param schema
+   * @param batches
    * @return
    * @throws SchemaChangeException
    * @throws UnsupportedEncodingException
    */
-   private Map<String, List<Object>> addToCombinedVectorResults(List<QueryDataBatch> records, RecordBatchLoader loader,
-                                                         BatchSchema schema) throws SchemaChangeException, UnsupportedEncodingException {
+  public static Map<String, List<Object>> addToCombinedVectorResults(Iterable<VectorAccessible> batches)
+       throws SchemaChangeException, UnsupportedEncodingException {
     // TODO - this does not handle schema changes
     Map<String, List<Object>> combinedVectors = new TreeMap<>();
 
     long totalRecords = 0;
-    QueryDataBatch batch;
-    int size = records.size();
-    for (int i = 0; i < size; i++) {
-      batch = records.get(0);
-      loader.load(batch.getHeader().getDef(), batch.getData());
+    BatchSchema schema = null;
+    for (VectorAccessible loader : batches)  {
       // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
       // SchemaChangeException, so check/clean throws clause above.
       if (schema == null) {
@@ -272,24 +317,66 @@ public class DrillTestWrapper {
         for (MaterializedField mf : schema) {
           combinedVectors.put(SchemaPath.getSimplePath(mf.getPath()).toExpr(), new ArrayList<Object>());
         }
+      } else {
+        // TODO - actually handle schema changes, this is just to get access to the SelectionVectorMode
+        // of the current batch, the check for a null schema is used to only mutate the schema once
+        // need to add new vectors and null fill for previous batches? distinction between null and non-existence important?
+        schema = loader.getSchema();
       }
       logger.debug("reading batch with " + loader.getRecordCount() + " rows, total read so far " + totalRecords);
       totalRecords += loader.getRecordCount();
       for (VectorWrapper<?> w : loader) {
         String field = SchemaPath.getSimplePath(w.getField().getPath()).toExpr();
-        for (int j = 0; j < loader.getRecordCount(); j++) {
-          Object obj = w.getValueVector().getAccessor().getObject(j);
-          if (obj != null) {
-            if (obj instanceof Text) {
-              obj = obj.toString();
+        ValueVector[] vectors;
+        if (w.isHyper()) {
+          vectors = w.getValueVectors();
+        } else {
+          vectors = new ValueVector[] {w.getValueVector()};
+        }
+        SelectionVector2 sv2 = null;
+        SelectionVector4 sv4 = null;
+        switch(schema.getSelectionVectorMode()) {
+          case TWO_BYTE:
+            sv2 = loader.getSelectionVector2();
+            break;
+          case FOUR_BYTE:
+            sv4 = loader.getSelectionVector4();
+            break;
+        }
+        if (sv4 != null) {
+          for (int j = 0; j < sv4.getCount(); j++) {
+            int complexIndex = sv4.get(j);
+            int batchIndex = complexIndex >> 16;
+            int recordIndexInBatch = complexIndex & 65535;
+            Object obj = vectors[batchIndex].getAccessor().getObject(recordIndexInBatch);
+            if (obj != null) {
+              if (obj instanceof Text) {
+                obj = obj.toString();
+              }
+            }
+            combinedVectors.get(field).add(obj);
+          }
+        }
+        else {
+          for (ValueVector vv : vectors) {
+            for (int j = 0; j < loader.getRecordCount(); j++) {
+              int index;
+              if (sv2 != null) {
+                index = sv2.getIndex(j);
+              } else {
+                index = j;
+              }
+              Object obj = vv.getAccessor().getObject(index);
+              if (obj != null) {
+                if (obj instanceof Text) {
+                  obj = obj.toString();
+                }
+              }
+              combinedVectors.get(field).add(obj);
             }
           }
-          combinedVectors.get(field).add(obj);
         }
       }
-      records.remove(0);
-      batch.release();
-      loader.clear();
     }
     return combinedVectors;
   }
@@ -342,7 +429,6 @@ public class DrillTestWrapper {
    */
   protected void compareUnorderedResults() throws Exception {
     RecordBatchLoader loader = new RecordBatchLoader(getAllocator());
-    BatchSchema schema = null;
 
     List<QueryDataBatch> actual = Collections.emptyList();
     List<QueryDataBatch> expected = Collections.emptyList();
@@ -356,14 +442,14 @@ public class DrillTestWrapper {
       checkNumBatches(actual);
 
       addTypeInfoIfMissing(actual.get(0), testBuilder);
-      addToMaterializedResults(actualRecords, actual, loader, schema);
+      addToMaterializedResults(actualRecords, actual, loader);
 
       // If baseline data was not provided to the test builder directly, we must run a query for the baseline, this includes
       // the cases where the baseline is stored in a file.
       if (baselineRecords == null) {
         BaseTestQuery.test(baselineOptionSettingQueries);
         expected = BaseTestQuery.testRunAndReturn(baselineQueryType, testBuilder.getValidationQuery());
-        addToMaterializedResults(expectedRecords, expected, loader, schema);
+        addToMaterializedResults(expectedRecords, expected, loader);
       } else {
         expectedRecords = baselineRecords;
       }
@@ -409,28 +495,24 @@ public class DrillTestWrapper {
       // To avoid extra work for test writers, types can optionally be inferred from the test query
       addTypeInfoIfMissing(actual.get(0), testBuilder);
 
-      actualSuperVectors = addToCombinedVectorResults(actual, loader, schema);
+      BatchIterator batchIter = new BatchIterator(actual, loader);
+      actualSuperVectors = addToCombinedVectorResults(batchIter);
+      batchIter.close();
 
       // If baseline data was not provided to the test builder directly, we must run a query for the baseline, this includes
       // the cases where the baseline is stored in a file.
       if (baselineRecords == null) {
         BaseTestQuery.test(baselineOptionSettingQueries);
         expected = BaseTestQuery.testRunAndReturn(baselineQueryType, testBuilder.getValidationQuery());
-        expectedSuperVectors = addToCombinedVectorResults(expected, loader, schema);
+        BatchIterator exBatchIter = new BatchIterator(expected, loader);
+        expectedSuperVectors = addToCombinedVectorResults(exBatchIter);
+        exBatchIter.close();
       } else {
         // data is built in the TestBuilder in a row major format as it is provided by the user
         // translate it here to vectorized, the representation expected by the ordered comparison
-        expectedSuperVectors = new TreeMap<>();
-        expected = new ArrayList<>();
-        for (String s : baselineRecords.get(0).keySet()) {
-          expectedSuperVectors.put(s, new ArrayList<>());
-        }
-        for (Map<String, Object> m : baselineRecords) {
-          for (String s : m.keySet()) {
-            expectedSuperVectors.get(s).add(m.get(s));
-          }
-        }
+        expectedSuperVectors = translateRecordListToHeapVectors(baselineRecords);
       }
+
       compareMergedVectors(expectedSuperVectors, actualSuperVectors);
     } catch (Exception e) {
       throw new Exception(e.getMessage() + "\nFor query: " + query , e);
@@ -439,9 +521,21 @@ public class DrillTestWrapper {
     }
   }
 
+  public static Map<String, List<Object>> translateRecordListToHeapVectors(List<Map<String, Object>> records) {
+    Map<String, List<Object>> ret = new TreeMap<>();
+    for (String s : records.get(0).keySet()) {
+      ret.put(s, new ArrayList<>());
+    }
+    for (Map<String, Object> m : records) {
+      for (String s : m.keySet()) {
+        ret.get(s).add(m.get(s));
+      }
+    }
+    return ret;
+  }
+
   public void compareResultsHyperVector() throws Exception {
     RecordBatchLoader loader = new RecordBatchLoader(getAllocator());
-    BatchSchema schema = null;
 
     BaseTestQuery.test(testOptionSettingQueries);
     List<QueryDataBatch> results = BaseTestQuery.testRunAndReturn(queryType, query);
@@ -451,12 +545,12 @@ public class DrillTestWrapper {
     // To avoid extra work for test writers, types can optionally be inferred from the test query
     addTypeInfoIfMissing(results.get(0), testBuilder);
 
-    Map<String, HyperVectorValueIterator> actualSuperVectors = addToHyperVectorMap(results, loader, schema);
+    Map<String, HyperVectorValueIterator> actualSuperVectors = addToHyperVectorMap(results, loader);
 
     BaseTestQuery.test(baselineOptionSettingQueries);
     List<QueryDataBatch> expected = BaseTestQuery.testRunAndReturn(baselineQueryType, testBuilder.getValidationQuery());
 
-    Map<String, HyperVectorValueIterator> expectedSuperVectors = addToHyperVectorMap(expected, loader, schema);
+    Map<String, HyperVectorValueIterator> expectedSuperVectors = addToHyperVectorMap(expected, loader);
 
     compareHyperVectors(expectedSuperVectors, actualSuperVectors);
     cleanupBatches(results, expected);
@@ -496,8 +590,10 @@ public class DrillTestWrapper {
     }
   }
 
-  protected void addToMaterializedResults(List<Map<String, Object>> materializedRecords, List<QueryDataBatch> records, RecordBatchLoader loader,
-                                          BatchSchema schema) throws SchemaChangeException, UnsupportedEncodingException {
+  public static void addToMaterializedResults(List<Map<String, Object>> materializedRecords,
+                                          List<QueryDataBatch> records,
+                                          RecordBatchLoader loader)
+      throws SchemaChangeException, UnsupportedEncodingException {
     long totalRecords = 0;
     QueryDataBatch batch;
     int size = records.size();
@@ -506,9 +602,6 @@ public class DrillTestWrapper {
       loader.load(batch.getHeader().getDef(), batch.getData());
       // TODO:  Clean:  DRILL-2933:  That load(...) no longer throws
       // SchemaChangeException, so check/clean throws clause above.
-      if (schema == null) {
-        schema = loader.getSchema();
-      }
       logger.debug("reading batch with " + loader.getRecordCount() + " rows, total read so far " + totalRecords);
       totalRecords += loader.getRecordCount();
       for (int j = 0; j < loader.getRecordCount(); j++) {
@@ -531,7 +624,7 @@ public class DrillTestWrapper {
     }
   }
 
-  public boolean compareValuesErrorOnMismatch(Object expected, Object actual, int counter, String column) throws Exception {
+  public static boolean compareValuesErrorOnMismatch(Object expected, Object actual, int counter, String column) throws Exception {
 
     if (compareValues(expected, actual, counter, column)) {
       return true;
@@ -554,7 +647,7 @@ public class DrillTestWrapper {
     return true;
   }
 
-  public boolean compareValues(Object expected, Object actual, int counter, String column) throws Exception {
+  public static boolean compareValues(Object expected, Object actual, int counter, String column) throws Exception {
     if (expected == null) {
       if (actual == null) {
         if (VERBOSE_DEBUG) {
@@ -648,7 +741,7 @@ public class DrillTestWrapper {
     assertEquals(0, actualRecords.size());
   }
 
-  private String findMissingColumns(Set<String> expected, Set<String> actual) {
+  private static String findMissingColumns(Set<String> expected, Set<String> actual) {
     String missingCols = "";
     for (String colName : expected) {
       if (!actual.contains(colName)) {

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java b/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java
index 8702eb5..b073371 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java
@@ -194,16 +194,12 @@ public class TestBuilder {
   // modified code from SchemaPath.De class. This should be used sparingly and only in tests if absolutely needed.
   public static SchemaPath parsePath(String path) {
     try {
-      // logger.debug("Parsing expression string '{}'", expr);
       ExprLexer lexer = new ExprLexer(new ANTLRStringStream(path));
       CommonTokenStream tokens = new CommonTokenStream(lexer);
       ExprParser parser = new ExprParser(tokens);
 
-      //TODO: move functionregistry and error collector to injectables.
-      //ctxt.findInjectableValue(valueId, forProperty, beanInstance)
       ExprParser.parse_return ret = parser.parse();
 
-      // ret.e.resolveAndValidate(expr, errorCollector);
       if (ret.e instanceof SchemaPath) {
         return (SchemaPath) ret.e;
       } else {

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/test/java/org/apache/drill/exec/physical/unit/BasicPhysicalOpUnitTest.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/unit/BasicPhysicalOpUnitTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/unit/BasicPhysicalOpUnitTest.java
new file mode 100644
index 0000000..6f2f160
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/unit/BasicPhysicalOpUnitTest.java
@@ -0,0 +1,322 @@
+/**
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.physical.unit;
+
+import com.google.common.collect.Lists;
+import org.apache.calcite.rel.RelFieldCollation;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.drill.exec.physical.MinorFragmentEndpoint;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.base.SubScan;
+import org.apache.drill.exec.physical.config.ComplexToJson;
+import org.apache.drill.exec.physical.config.ExternalSort;
+import org.apache.drill.exec.physical.config.Filter;
+import org.apache.drill.exec.physical.config.HashAggregate;
+import org.apache.drill.exec.physical.config.HashJoinPOP;
+import org.apache.drill.exec.physical.config.MergeJoinPOP;
+import org.apache.drill.exec.physical.config.MergingReceiverPOP;
+import org.apache.drill.exec.physical.config.Project;
+import org.apache.drill.exec.physical.config.StreamingAggregate;
+import org.apache.drill.exec.physical.config.TopN;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import java.lang.reflect.Constructor;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Set;
+
+import static org.apache.drill.TestBuilder.mapOf;
+
+public class BasicPhysicalOpUnitTest extends PhysicalOpUnitTestBase {
+
+  @Test
+  public void testSimpleProject() {
+    Project projectConf = new Project(parseExprs("x+5", "x"), null);
+    List<String> jsonBatches = Lists.newArrayList(
+        "[{\"x\": 5 },{\"x\": 10 }]",
+        "[{\"x\": 20 },{\"x\": 30 },{\"x\": 40 }]");
+    opTestBuilder()
+        .physicalOperator(projectConf)
+        .inputDataStreamJson(jsonBatches)
+        .baselineColumns("x")
+        .baselineValues(10l)
+        .baselineValues(15l)
+        .baselineValues(25l)
+        .baselineValues(35l)
+        .baselineValues(45l)
+        .go();
+  }
+
+  @Test
+  public void testProjectComplexOutput() {
+    Project projectConf = new Project(parseExprs("convert_from(json_col, 'JSON')", "complex_col"), null);
+    List<String> jsonBatches = Lists.newArrayList(
+        "[{\"json_col\": \"{ \\\"a\\\" : 1 }\"}]",
+        "[{\"json_col\": \"{ \\\"a\\\" : 5 }\"}]");
+    opTestBuilder()
+        .physicalOperator(projectConf)
+        .inputDataStreamJson(jsonBatches)
+        .baselineColumns("complex_col")
+        .baselineValues(mapOf("a", 1l))
+        .baselineValues(mapOf("a", 5l))
+        .go();
+  }
+
+  @Test
+  public void testSimpleHashJoin() {
+    HashJoinPOP joinConf = new HashJoinPOP(null, null, Lists.newArrayList(joinCond("x", "EQUALS", "x1")), JoinRelType.LEFT);
+    // TODO - figure out where to add validation, column names must be unique, even between the two batches,
+    // for all columns, not just the one in the join condition
+    // TODO - if any are common between the two, it is failing in the generated setup method in HashJoinProbeGen
+    List<String> leftJsonBatches = Lists.newArrayList(
+        "[{\"x\": 5, \"a\" : \"a string\"}]",
+        "[{\"x\": 5, \"a\" : \"a different string\"},{\"x\": 5, \"a\" : \"meh\"}]");
+    List<String> rightJsonBatches = Lists.newArrayList(
+        "[{\"x1\": 5, \"a2\" : \"asdf\"}]",
+        "[{\"x1\": 6, \"a2\" : \"qwerty\"},{\"x1\": 5, \"a2\" : \"12345\"}]");
+    opTestBuilder()
+        .physicalOperator(joinConf)
+        .inputDataStreamsJson(Lists.newArrayList(leftJsonBatches, rightJsonBatches))
+        .baselineColumns("x", "a", "a2", "x1")
+        .baselineValues(5l, "a string", "asdf", 5l)
+        .baselineValues(5l, "a string", "12345", 5l)
+        .baselineValues(5l, "a different string", "asdf", 5l)
+        .baselineValues(5l, "a different string", "12345", 5l)
+        .baselineValues(5l, "meh", "asdf", 5l)
+        .baselineValues(5l, "meh", "12345", 5l)
+        .go();
+  }
+
+  @Test
+  public void testSimpleMergeJoin() {
+    MergeJoinPOP joinConf = new MergeJoinPOP(null, null, Lists.newArrayList(joinCond("x", "EQUALS", "x1")), JoinRelType.LEFT);
+    // TODO - figure out where to add validation, column names must be unique, even between the two batches,
+    // for all columns, not just the one in the join condition
+    List<String> leftJsonBatches = Lists.newArrayList(
+        "[{\"x\": 5, \"a\" : \"a string\"}]",
+        "[{\"x\": 5, \"a\" : \"a different string\"},{\"x\": 5, \"a\" : \"meh\"}]");
+    List<String> rightJsonBatches = Lists.newArrayList(
+        "[{\"x1\": 5, \"a2\" : \"asdf\"}]",
+        "[{\"x1\": 5, \"a2\" : \"12345\"}, {\"x1\": 6, \"a2\" : \"qwerty\"}]");
+    opTestBuilder()
+        .physicalOperator(joinConf)
+        .inputDataStreamsJson(Lists.newArrayList(leftJsonBatches, rightJsonBatches))
+        .baselineColumns("x", "a", "a2", "x1")
+        .baselineValues(5l, "a string", "asdf", 5l)
+        .baselineValues(5l, "a string", "12345", 5l)
+        .baselineValues(5l, "a different string", "asdf", 5l)
+        .baselineValues(5l, "a different string", "12345", 5l)
+        .baselineValues(5l, "meh", "asdf", 5l)
+        .baselineValues(5l, "meh", "12345", 5l)
+        .go();
+  }
+
+  @Test
+  public void testSimpleHashAgg() {
+    HashAggregate aggConf = new HashAggregate(null, parseExprs("a", "a"), parseExprs("sum(b)", "b_sum"), 1.0f);
+    List<String> inputJsonBatches = Lists.newArrayList(
+        "[{\"a\": 5, \"b\" : 1 }]",
+        "[{\"a\": 5, \"b\" : 5},{\"a\": 3, \"b\" : 8}]");
+    opTestBuilder()
+        .physicalOperator(aggConf)
+        .inputDataStreamJson(inputJsonBatches)
+        .baselineColumns("b_sum", "a")
+        .baselineValues(6l, 5l)
+        .baselineValues(8l, 3l)
+        .go();
+  }
+
+  @Test
+  public void testSimpleStreamAgg() {
+    StreamingAggregate aggConf = new StreamingAggregate(null, parseExprs("a", "a"), parseExprs("sum(b)", "b_sum"), 1.0f);
+    List<String> inputJsonBatches = Lists.newArrayList(
+        "[{\"a\": 5, \"b\" : 1 }]",
+        "[{\"a\": 5, \"b\" : 5},{\"a\": 3, \"b\" : 8}]");
+    opTestBuilder()
+        .physicalOperator(aggConf)
+        .inputDataStreamJson(inputJsonBatches)
+        .baselineColumns("b_sum", "a")
+        .baselineValues(6l, 5l)
+        .baselineValues(8l, 3l)
+        .go();
+  }
+
+  @Test
+  public void testComplexToJson() {
+    ComplexToJson complexToJson = new ComplexToJson(null);
+    List<String> inputJsonBatches = Lists.newArrayList(
+        "[{\"a\": {\"b\" : 1 }}]",
+        "[{\"a\": {\"b\" : 5}},{\"a\": {\"b\" : 8}}]");
+    opTestBuilder()
+        .physicalOperator(complexToJson)
+        .inputDataStreamJson(inputJsonBatches)
+        .baselineColumns("a")
+        .baselineValues("{\n  \"b\" : 1\n}")
+        .baselineValues("{\n  \"b\" : 5\n}")
+        .baselineValues("{\n  \"b\" : 8\n}")
+        .go();
+  }
+
+  @Test
+  public void testFilter() {
+    Filter filterConf = new Filter(null, parseExpr("a=5"), 1.0f);
+    List<String> inputJsonBatches = Lists.newArrayList(
+        "[{\"a\": 5, \"b\" : 1 }]",
+        "[{\"a\": 5, \"b\" : 5},{\"a\": 3, \"b\" : 8}]",
+        "[{\"a\": 40, \"b\" : 3},{\"a\": 13, \"b\" : 100}]");
+    opTestBuilder()
+        .physicalOperator(filterConf)
+        .inputDataStreamJson(inputJsonBatches)
+        .baselineColumns("a", "b")
+        .baselineValues(5l, 1l)
+        .baselineValues(5l, 5l)
+        .go();
+  }
+
+  @Test
+  public void testExternalSort() {
+    ExternalSort sortConf = new ExternalSort(null,
+        Lists.newArrayList(ordering("b", RelFieldCollation.Direction.ASCENDING, RelFieldCollation.NullDirection.FIRST)), false);
+    List<String> inputJsonBatches = Lists.newArrayList(
+        "[{\"a\": 5, \"b\" : 1 }]",
+        "[{\"a\": 5, \"b\" : 5},{\"a\": 3, \"b\" : 8}]",
+        "[{\"a\": 40, \"b\" : 3},{\"a\": 13, \"b\" : 100}]");
+    opTestBuilder()
+        .physicalOperator(sortConf)
+        .inputDataStreamJson(inputJsonBatches)
+        .baselineColumns("a", "b")
+        .baselineValues(5l, 1l)
+        .baselineValues(40l, 3l)
+        .baselineValues(5l, 5l)
+        .baselineValues(3l, 8l)
+        .baselineValues(13l, 100l)
+        .go();
+  }
+
+  private void externalSortLowMemoryHelper(int batchSize, int numberOfBatches, long initReservation, long maxAllocation) {
+    ExternalSort sortConf = new ExternalSort(null,
+        Lists.newArrayList(ordering("b", RelFieldCollation.Direction.ASCENDING, RelFieldCollation.NullDirection.FIRST)), false);
+    List<String> inputJsonBatches = Lists.newArrayList();
+    StringBuilder batchString = new StringBuilder();
+    for (int j = 0; j < numberOfBatches; j++) {
+      batchString.append("[");
+      for (int i = 0; i < batchSize; i++) {
+        batchString.append("{\"a\": 5, \"b\" : 5},{\"a\": 3, \"b\" : 8},");
+      }
+      batchString.append("{\"a\": 5, \"b\" : 1 }");
+      batchString.append("]");
+      inputJsonBatches.add(batchString.toString());
+    }
+
+    OperatorTestBuilder opTestBuilder =
+        opTestBuilder()
+            .initReservation(initReservation)
+            .maxAllocation(maxAllocation)
+            .physicalOperator(sortConf)
+            .inputDataStreamJson(inputJsonBatches)
+            .baselineColumns("a", "b");
+    for (int i = 0; i < numberOfBatches; i++) {
+      opTestBuilder.baselineValues(5l, 1l);
+    }
+    for (int i = 0; i < batchSize * numberOfBatches; i++) {
+      opTestBuilder.baselineValues(5l, 5l);
+    }
+    for (int i = 0; i < batchSize * numberOfBatches; i++) {
+      opTestBuilder.baselineValues(3l, 8l);
+    }
+    opTestBuilder.go();
+  }
+
+  // TODO - Failing with - org.apache.drill.exec.exception.OutOfMemoryException: Unable to allocate buffer of size 262144 (rounded from 147456) due to memory limit. Current allocation: 16422656
+  // look in ExternalSortBatch for this JIRA number, changing this percentage of the allocator limit that is
+  // the threshold for spilling (it worked with 0.65 for me) "fixed" the problem but hurt perf, will want
+  // to find a better solutions to this problem. When it is fixed this threshold will likely become unnecessary
+  @Test
+  @Ignore("DRILL-4438")
+  public void testExternalSortLowMemory1() {
+    externalSortLowMemoryHelper(4960, 100, 10000000, 16500000);
+  }
+
+  // TODO- believe this was failing in the scan not the sort, may not require a fix
+  @Test
+  @Ignore("DRILL-4438")
+  public void testExternalSortLowMemory2() {
+    externalSortLowMemoryHelper(4960, 100, 10000000, 15000000);
+  }
+
+  // TODO - believe this was failing in the scan not the sort, may not require a fix
+  @Test
+  @Ignore("DRILL-4438")
+  public void testExternalSortLowMemory3() {
+    externalSortLowMemoryHelper(40960, 10, 10000000, 10000000);
+  }
+
+  // TODO - Failing with - org.apache.drill.exec.exception.OutOfMemoryException: Unable to allocate sv2 buffer after repeated attempts
+  // see comment above testExternalSortLowMemory1 about TODO left in ExternalSortBatch
+  @Test
+  @Ignore("DRILL-4438")
+  public void testExternalSortLowMemory4() {
+    externalSortLowMemoryHelper(15960, 30, 10000000, 14500000);
+  }
+
+  @Test
+  public void testTopN() {
+    TopN sortConf = new TopN(null,
+        Lists.newArrayList(ordering("b", RelFieldCollation.Direction.ASCENDING, RelFieldCollation.NullDirection.FIRST)), false, 3);
+    List<String> inputJsonBatches = Lists.newArrayList(
+        "[{\"a\": 5, \"b\" : 1 }]",
+        "[{\"a\": 5, \"b\" : 5},{\"a\": 3, \"b\" : 8}]",
+        "[{\"a\": 40, \"b\" : 3},{\"a\": 13, \"b\" : 100}]");
+    opTestBuilder()
+        .physicalOperator(sortConf)
+        .inputDataStreamJson(inputJsonBatches)
+        .baselineColumns("a", "b")
+        .baselineValues(5l, 1l)
+        .baselineValues(40l, 3l)
+        .baselineValues(5l, 5l)
+        .go();
+  }
+
+  // TODO(DRILL-4439) - doesn't expect incoming batches, uses instead RawFragmentBatch
+  // need to figure out how to mock these
+  @Ignore
+  @Test
+  public void testSimpleMergingReceiver() {
+    MergingReceiverPOP mergeConf = new MergingReceiverPOP(-1, Lists.<MinorFragmentEndpoint>newArrayList(),
+        Lists.newArrayList(ordering("x", RelFieldCollation.Direction.ASCENDING, RelFieldCollation.NullDirection.FIRST)), false);
+    List<String> leftJsonBatches = Lists.newArrayList(
+        "[{\"x\": 5, \"a\" : \"a string\"}]",
+        "[{\"x\": 5, \"a\" : \"a different string\"},{\"x\": 5, \"a\" : \"meh\"}]");
+    List<String> rightJsonBatches = Lists.newArrayList(
+        "[{\"x\": 5, \"a\" : \"asdf\"}]",
+        "[{\"x\": 5, \"a\" : \"12345\"}, {\"x\": 6, \"a\" : \"qwerty\"}]");
+    opTestBuilder()
+        .physicalOperator(mergeConf)
+        .inputDataStreamsJson(Lists.newArrayList(leftJsonBatches, rightJsonBatches))
+        .baselineColumns("x", "a")
+        .baselineValues(5l, "a string")
+        .baselineValues(5l, "a different string")
+        .baselineValues(5l, "meh")
+        .baselineValues(5l, "asdf")
+        .baselineValues(5l, "12345")
+        .baselineValues(6l, "qwerty")
+        .go();
+  }
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/java-exec/src/test/java/org/apache/drill/exec/physical/unit/PhysicalOpUnitTestBase.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/unit/PhysicalOpUnitTestBase.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/unit/PhysicalOpUnitTestBase.java
new file mode 100644
index 0000000..245e5bb
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/unit/PhysicalOpUnitTestBase.java
@@ -0,0 +1,341 @@
+/**
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.physical.unit;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import mockit.Delegate;
+import mockit.Injectable;
+import mockit.NonStrictExpectations;
+import org.antlr.runtime.ANTLRStringStream;
+import org.antlr.runtime.CommonTokenStream;
+import org.antlr.runtime.RecognitionException;
+import org.apache.calcite.rel.RelFieldCollation;
+import org.apache.drill.DrillTestWrapper;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.expression.FieldReference;
+import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.expression.PathSegment;
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.expression.parser.ExprLexer;
+import org.apache.drill.common.expression.parser.ExprParser;
+import org.apache.drill.common.logical.data.JoinCondition;
+import org.apache.drill.common.logical.data.NamedExpression;
+import org.apache.drill.common.logical.data.Order;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.common.scanner.persistence.ScanResult;
+import org.apache.drill.exec.compile.CodeCompiler;
+import org.apache.drill.exec.compile.TemplateClassDefinition;
+import org.apache.drill.exec.exception.ClassTransformationException;
+import org.apache.drill.exec.exception.SchemaChangeException;
+import org.apache.drill.exec.expr.ClassGenerator;
+import org.apache.drill.exec.expr.CodeGenerator;
+import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry;
+import org.apache.drill.exec.memory.BufferAllocator;
+import org.apache.drill.exec.memory.RootAllocatorFactory;
+import org.apache.drill.exec.ops.BufferManagerImpl;
+import org.apache.drill.exec.ops.FragmentContext;
+import org.apache.drill.exec.ops.OperatorContext;
+import org.apache.drill.exec.ops.OperatorStats;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.OperatorCreatorRegistry;
+import org.apache.drill.exec.physical.impl.ScanBatch;
+import org.apache.drill.exec.physical.impl.project.Projector;
+import org.apache.drill.exec.physical.impl.project.ProjectorTemplate;
+import org.apache.drill.exec.proto.ExecProtos;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.VectorAccessible;
+import org.apache.drill.exec.server.options.OptionManager;
+import org.apache.drill.exec.server.options.TypeValidators;
+import org.apache.drill.exec.store.RecordReader;
+import org.apache.drill.exec.store.easy.json.JSONRecordReader;
+import org.apache.drill.exec.testing.ExecutionControls;
+import org.apache.drill.test.DrillTest;
+
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Look! Doesn't extend BaseTestQuery!!
+ */
+public class PhysicalOpUnitTestBase extends DrillTest {
+
+  @Injectable FragmentContext fragContext;
+  @Injectable OperatorContext opContext;
+  @Injectable OperatorStats opStats;
+  @Injectable OptionManager optManager;
+  @Injectable PhysicalOperator popConf;
+  @Injectable ExecutionControls executionControls;
+
+  private final DrillConfig drillConf = DrillConfig.create();
+  private final BufferAllocator allocator = RootAllocatorFactory.newRoot(drillConf);
+  private final BufferManagerImpl bufManager = new BufferManagerImpl(allocator);
+  private final ScanResult classpathScan = ClassPathScanner.fromPrescan(drillConf);
+  private final FunctionImplementationRegistry funcReg = new FunctionImplementationRegistry(drillConf, classpathScan);
+  private final TemplateClassDefinition templateClassDefinition = new TemplateClassDefinition<>(Projector.class, ProjectorTemplate.class);
+  private final OperatorCreatorRegistry opCreatorReg = new OperatorCreatorRegistry(classpathScan);
+
+  protected LogicalExpression parseExpr(String expr) {
+    ExprLexer lexer = new ExprLexer(new ANTLRStringStream(expr));
+    CommonTokenStream tokens = new CommonTokenStream(lexer);
+    ExprParser parser = new ExprParser(tokens);
+    try {
+      return parser.parse().e;
+    } catch (RecognitionException e) {
+      throw new RuntimeException("Error parsing expression: " + expr);
+    }
+  }
+
+  protected Order.Ordering ordering(String expression, RelFieldCollation.Direction direction, RelFieldCollation.NullDirection nullDirection) {
+    return new Order.Ordering(direction, parseExpr(expression), nullDirection);
+  }
+
+  protected JoinCondition joinCond(String leftExpr, String relationship, String rightExpr) {
+    return new JoinCondition(relationship, parseExpr(leftExpr), parseExpr(rightExpr));
+  }
+
+  protected List<NamedExpression> parseExprs(String... expressionsAndOutputNames) {
+    Preconditions.checkArgument(expressionsAndOutputNames.length %2 ==0, "List of expressions and output field names" +
+        " is not complete, each expression must explicitly give and output name,");
+    List<NamedExpression> ret = new ArrayList<>();
+    for (int i = 0; i < expressionsAndOutputNames.length; i += 2) {
+      ret.add(new NamedExpression(parseExpr(expressionsAndOutputNames[i]),
+          new FieldReference(new SchemaPath(new PathSegment.NameSegment(expressionsAndOutputNames[i+1])))));
+    }
+    return ret;
+  }
+
+
+  void runTest(OperatorTestBuilder testBuilder) {
+    BatchCreator<PhysicalOperator> opCreator;
+    RecordBatch testOperator;
+    try {
+      mockFragmentContext(testBuilder.initReservation, testBuilder.maxAllocation);
+      opCreator = (BatchCreator<PhysicalOperator>)
+          opCreatorReg.getOperatorCreator(testBuilder.popConfig.getClass());
+       List<RecordBatch> incomingStreams = Lists.newArrayList();
+       for (List<String> batchesJson : testBuilder.inputStreamsJSON) {
+         incomingStreams.add(new ScanBatch(null, fragContext,
+             getRecordReadersForJsonBatches(batchesJson, fragContext)));
+       }
+       testOperator = opCreator.getBatch(fragContext, testBuilder.popConfig, incomingStreams);
+
+      Map<String, List<Object>> actualSuperVectors = DrillTestWrapper.addToCombinedVectorResults(new BatchIterator(testOperator));
+      Map<String, List<Object>> expectedSuperVectors = DrillTestWrapper.translateRecordListToHeapVectors(testBuilder.baselineRecords);
+      DrillTestWrapper.compareMergedVectors(expectedSuperVectors, actualSuperVectors);
+
+    } catch (ExecutionSetupException e) {
+      throw new RuntimeException(e);
+    } catch (UnsupportedEncodingException e) {
+      throw new RuntimeException(e);
+    } catch (SchemaChangeException e) {
+      throw new RuntimeException(e);
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  private static class BatchIterator implements Iterable<VectorAccessible> {
+
+    private RecordBatch operator;
+    public BatchIterator(RecordBatch operator) {
+      this.operator = operator;
+    }
+
+    @Override
+    public Iterator<VectorAccessible> iterator() {
+      return new Iterator<VectorAccessible>() {
+        boolean needToGrabNext = true;
+        RecordBatch.IterOutcome lastResultOutcome;
+        @Override
+        public boolean hasNext() {
+          if (needToGrabNext) {
+            lastResultOutcome = operator.next();
+            needToGrabNext = false;
+          }
+          if (lastResultOutcome == RecordBatch.IterOutcome.NONE
+            || lastResultOutcome == RecordBatch.IterOutcome.STOP) {
+            return false;
+          } else if (lastResultOutcome == RecordBatch.IterOutcome.OUT_OF_MEMORY) {
+            throw new RuntimeException("Operator ran out of memory");
+          } else {
+            return true;
+          }
+        }
+
+        @Override
+        public VectorAccessible next() {
+          if (needToGrabNext) {
+            lastResultOutcome = operator.next();
+          }
+          needToGrabNext = true;
+          return operator;
+        }
+
+        @Override
+        public void remove() {
+          throw new UnsupportedOperationException("Remove is not supported.");
+        }
+      };
+    }
+  }
+
+  protected OperatorTestBuilder opTestBuilder() {
+    return new OperatorTestBuilder();
+  }
+
+  protected class OperatorTestBuilder {
+
+    private PhysicalOperator popConfig;
+    private String[] baselineColumns;
+    private List<Map<String, Object>> baselineRecords;
+    private List<List<String>> inputStreamsJSON;
+    private long initReservation = 10000000;
+    private long maxAllocation = 15000000;
+
+    public void go() {
+      runTest(this);
+    }
+
+    public OperatorTestBuilder physicalOperator(PhysicalOperator batch) {
+      this.popConfig = batch;
+      return this;
+    }
+
+    public OperatorTestBuilder initReservation(long initReservation) {
+      this.initReservation = initReservation;
+      return this;
+    }
+
+    public OperatorTestBuilder maxAllocation(long maxAllocation) {
+      this.maxAllocation = maxAllocation;
+      return this;
+    }
+
+    public OperatorTestBuilder inputDataStreamJson(List<String> jsonBatches) {
+      this.inputStreamsJSON = new ArrayList<>();
+      this.inputStreamsJSON.add(jsonBatches);
+      return this;
+    }
+
+    public OperatorTestBuilder inputDataStreamsJson(List<List<String>> childStreams) {
+      this.inputStreamsJSON = childStreams;
+      return this;
+    }
+
+    public OperatorTestBuilder baselineColumns(String... columns) {
+      for (int i = 0; i < columns.length; i++) {
+        LogicalExpression ex = parseExpr(columns[i]);
+        if (ex instanceof SchemaPath) {
+          columns[i] = ((SchemaPath)ex).toExpr();
+        } else {
+          throw new IllegalStateException("Schema path is not a valid format.");
+        }
+      }
+      this.baselineColumns = columns;
+      return this;
+    }
+
+    public OperatorTestBuilder baselineValues(Object ... baselineValues) {
+      if (baselineRecords == null) {
+        baselineRecords = new ArrayList();
+      }
+      Map<String, Object> ret = new HashMap();
+      int i = 0;
+      Preconditions.checkArgument(baselineValues.length == baselineColumns.length,
+          "Must supply the same number of baseline values as columns.");
+      for (String s : baselineColumns) {
+        ret.put(s, baselineValues[i]);
+        i++;
+      }
+      this.baselineRecords.add(ret);
+      return this;
+    }
+  }
+
+  private void mockFragmentContext(long initReservation, long maxAllocation) {
+    final CodeCompiler compiler = new CodeCompiler(drillConf, optManager);
+    final BufferAllocator allocator = this.allocator.newChildAllocator("allocator_for_operator_test", initReservation, maxAllocation);
+    new NonStrictExpectations() {
+      {
+        optManager.getOption(withAny(new TypeValidators.BooleanValidator("", false))); result = false;
+        // TODO(DRILL-4450) - Probably want to just create a default option manager, this is a hack to prevent
+        // the code compilation from failing when trying to decide of scalar replacement is turned on
+        // this will cause other code paths to fail because this return value won't be valid for most
+        // string options
+        optManager.getOption(withAny(new TypeValidators.StringValidator("", "try"))); result = "try";
+        optManager.getOption(withAny(new TypeValidators.PositiveLongValidator("", 1l, 1l))); result = 10;
+        fragContext.getOptions(); result = optManager;
+        fragContext.getManagedBuffer(); result = bufManager.getManagedBuffer();
+        fragContext.shouldContinue(); result = true;
+        fragContext.getExecutionControls(); result = executionControls;
+        fragContext.getFunctionRegistry(); result = funcReg;
+        fragContext.getConfig(); result = drillConf;
+        fragContext.getHandle(); result = ExecProtos.FragmentHandle.getDefaultInstance();
+        try {
+          fragContext.getImplementationClass(withAny(CodeGenerator.get(templateClassDefinition, funcReg)));
+          result = new Delegate()
+          {
+            Object getImplementationClass(CodeGenerator gen) throws IOException, ClassTransformationException {
+              return compiler.getImplementationClass(gen);
+            }
+          };
+          fragContext.getImplementationClass(withAny(CodeGenerator.get(templateClassDefinition, funcReg).getRoot()));
+          result = new Delegate()
+          {
+            Object getImplementationClass(ClassGenerator gen) throws IOException, ClassTransformationException {
+              return compiler.getImplementationClass(gen.getCodeGenerator());
+            }
+          };
+        } catch (ClassTransformationException e) {
+          throw new RuntimeException(e);
+        } catch (IOException e) {
+          throw new RuntimeException(e);
+        }
+        opContext.getStats();result = opStats;
+        opContext.getAllocator(); result = allocator;
+        fragContext.newOperatorContext(withAny(popConf));result = opContext;
+      }
+    };
+  }
+
+  private Iterator<RecordReader> getRecordReadersForJsonBatches(List<String> jsonBatches, FragmentContext fragContext) {
+    ObjectMapper mapper = new ObjectMapper();
+    List<RecordReader> readers = new ArrayList<>();
+    for (String batchJason : jsonBatches) {
+      JsonNode records;
+      try {
+        records = mapper.readTree(batchJason);
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+      readers.add(new JSONRecordReader(fragContext, records, null, Collections.singletonList(SchemaPath.getSimplePath("*"))));
+    }
+    return readers.iterator();
+  }
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/d93a3633/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcDataTest.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcDataTest.java b/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcDataTest.java
index 56e58dc..fd5d4f0 100644
--- a/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcDataTest.java
+++ b/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcDataTest.java
@@ -186,7 +186,7 @@ public class JdbcDataTest extends JdbcTestBase {
     Scan scan = findOnlyOperator(plan, Scan.class);
     Assert.assertEquals("donuts-json", scan.getStorageEngine());
     Project project = findOnlyOperator(plan, Project.class);
-    Assert.assertEquals(1, project.getSelections().length);
+    Assert.assertEquals(1, project.getSelections().size());
     Assert.assertEquals(Scan.class, project.getInput().getClass());
     Store store = findOnlyOperator(plan, Store.class);
     Assert.assertEquals("queue", store.getStorageEngine());
@@ -244,9 +244,9 @@ public class JdbcDataTest extends JdbcTestBase {
     Assert.assertTrue(filter.getInput() instanceof Scan);
     Project[] projects = Iterables.toArray(findOperator(plan, Project.class), Project.class);
     Assert.assertEquals(2, projects.length);
-    Assert.assertEquals(1, projects[0].getSelections().length);
+    Assert.assertEquals(1, projects[0].getSelections().size());
     Assert.assertEquals(Filter.class, projects[0].getInput().getClass());
-    Assert.assertEquals(2, projects[1].getSelections().length);
+    Assert.assertEquals(2, projects[1].getSelections().size());
     Assert.assertEquals(Project.class, projects[1].getInput().getClass());
     Store store = findOnlyOperator(plan, Store.class);
     Assert.assertEquals("queue", store.getStorageEngine());


[2/4] drill git commit: DRILL-4448: Clean up deserialization of oderings in sorts

Posted by js...@apache.org.
DRILL-4448: Clean up deserialization of oderings in sorts

Fix sort operator deserialization and validation to respect existing
contract specified in the tests.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/1d1acc09
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/1d1acc09
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/1d1acc09

Branch: refs/heads/master
Commit: 1d1acc09ec30167f0653d99cee6f30c7b1413859
Parents: d24205d
Author: Jason Altekruse <al...@gmail.com>
Authored: Wed Feb 24 19:37:21 2016 -0800
Committer: Jason Altekruse <al...@gmail.com>
Committed: Wed Apr 20 08:10:40 2016 -0700

----------------------------------------------------------------------
 .../apache/drill/common/logical/data/Order.java | 107 ++++++++++++++-----
 .../drill/common/logical/data/OrderTest.java    |  14 ++-
 2 files changed, 85 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/1d1acc09/logical/src/main/java/org/apache/drill/common/logical/data/Order.java
----------------------------------------------------------------------
diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/Order.java b/logical/src/main/java/org/apache/drill/common/logical/data/Order.java
index ca3eec4..1bf587d 100644
--- a/logical/src/main/java/org/apache/drill/common/logical/data/Order.java
+++ b/logical/src/main/java/org/apache/drill/common/logical/data/Order.java
@@ -19,7 +19,10 @@ package org.apache.drill.common.logical.data;
 
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 
+import com.google.common.collect.ImmutableMap;
+import org.apache.calcite.util.PartiallyOrderedSet;
 import org.apache.drill.common.exceptions.DrillRuntimeException;
 import org.apache.drill.common.expression.FieldReference;
 import org.apache.drill.common.expression.LogicalExpression;
@@ -76,6 +79,17 @@ public class Order extends SingleInputOperator {
     private final Direction direction;
     /** Net &lt;null ordering> */
     private final NullDirection nullOrdering;
+    /** The values in the plans for ordering specification are ASC, DESC, not the
+     * full words featured in the calcite {@link Direction} Enum, need to map between them. */
+    private static ImmutableMap<String, Direction> DRILL_TO_CALCITE_DIR_MAPPING =
+        ImmutableMap.<String, Direction>builder()
+        .put("ASC", Direction.ASCENDING)
+        .put("DESC", Direction.DESCENDING).build();
+    private static ImmutableMap<String, NullDirection> DRILL_TO_CALCITE_NULL_DIR_MAPPING =
+        ImmutableMap.<String, NullDirection>builder()
+            .put("FIRST", NullDirection.FIRST)
+            .put("LAST", NullDirection.LAST)
+            .put("UNSPECIFIED", NullDirection.UNSPECIFIED).build();
 
     /**
      * Constructs a sort specification.
@@ -91,17 +105,17 @@ public class Order extends SingleInputOperator {
      *             (omitted / {@link NullDirection#UNSPECIFIED}, interpreted later)
      */
     @JsonCreator
-    public Ordering( @JsonProperty("expr") LogicalExpression expr,
-                     @JsonProperty("order") String strOrderingSpec,
+    public Ordering( @JsonProperty("order") String strOrderingSpec,
+                     @JsonProperty("expr") LogicalExpression expr,
                      @JsonProperty("nullDirection") String strNullOrdering ) {
       this.expr = expr;
-      this.direction = getOrderingSpecFromString( strOrderingSpec );
-      this.nullOrdering = getNullOrderingFromString( strNullOrdering );
+      this.direction = getOrderingSpecFromString(strOrderingSpec);
+      this.nullOrdering = getNullOrderingFromString(strNullOrdering);
     }
 
     public Ordering(Direction direction, LogicalExpression e, NullDirection nullOrdering) {
       this.expr = e;
-      this.direction = direction;
+      this.direction = filterDrillSupportedDirections(direction);
       this.nullOrdering = nullOrdering;
     }
 
@@ -110,41 +124,78 @@ public class Order extends SingleInputOperator {
     }
 
     private static Direction getOrderingSpecFromString( String strDirection ) {
-      final Direction direction;
-      if ( null == strDirection
-          || Direction.ASCENDING.shortString.equals( strDirection ) ) {
-        direction = Direction.ASCENDING;
+      Direction dir = DRILL_TO_CALCITE_DIR_MAPPING.get(strDirection);
+      if (dir != null || strDirection == null) {
+        return filterDrillSupportedDirections(dir);
+      } else {
+        throw new DrillRuntimeException(
+            "Unknown <ordering specification> string (not \"ASC\", \"DESC\", "
+                + "or null): \"" + strDirection + "\"" );
+      }
+    }
+
+    private static NullDirection getNullOrderingFromString( String strNullOrdering ) {
+      NullDirection nullDir = DRILL_TO_CALCITE_NULL_DIR_MAPPING.get(strNullOrdering);
+      if (nullDir != null || strNullOrdering == null) {
+        return filterDrillSupportedNullDirections(nullDir);
+      } else {
+        throw new DrillRuntimeException(
+            "Internal error:  Unknown <null ordering> string (not "
+                + "\"" + NullDirection.FIRST.name() + "\", "
+                + "\"" + NullDirection.LAST.name() + "\", or "
+                + "\"" + NullDirection.UNSPECIFIED.name() + "\" or null): "
+                + "\"" + strNullOrdering + "\"" );
+      }
+    }
+
+    /**
+     * Disallows unsupported values for ordering direction (provided by Calcite but not implemented by Drill)
+     *
+     * Provides a default value of ASCENDING in the case of a null.
+     *
+     * @param direction
+     * @return - a sanitized direction value
+     */
+    private static Direction filterDrillSupportedDirections(Direction direction) {
+      if (direction == null || direction == Direction.ASCENDING) {
+        return Direction.ASCENDING;
       }
-      else if ( Direction.DESCENDING.shortString.equals( strDirection ) ) {
-        direction = Direction.DESCENDING;
+      else if (Direction.DESCENDING.equals( direction) ) {
+        return direction;
       }
       else {
         throw new DrillRuntimeException(
             "Unknown <ordering specification> string (not \"ASC\", \"DESC\", "
-            + "or null): \"" + strDirection + "\"" );
+            + "or null): \"" + direction + "\"" );
       }
-      return direction;
     }
 
-    private static NullDirection getNullOrderingFromString( String strNullOrdering ) {
-      final RelFieldCollation.NullDirection nullOrdering;
-      if ( null == strNullOrdering ) {
-        nullOrdering = NullDirection.UNSPECIFIED;
+    /**
+     * Disallows unsupported values for null ordering (provided by Calcite but not implemented by Drill),
+     * currently all values are supported.
+     *
+     * Provides a default value of UNSPECIFIED in the case of a null.
+     *
+     * @param nullDirection
+     * @return - a sanitized direction value
+     */
+    private static NullDirection filterDrillSupportedNullDirections(NullDirection nullDirection) {
+      if ( null == nullDirection) {
+        return NullDirection.UNSPECIFIED;
       }
-      else {
-        try {
-          nullOrdering = NullDirection.valueOf( strNullOrdering );
-        }
-        catch ( IllegalArgumentException e ) {
+      switch (nullDirection) {
+        case LAST:
+        case FIRST:
+        case UNSPECIFIED:
+          return nullDirection;
+        default:
           throw new DrillRuntimeException(
               "Internal error:  Unknown <null ordering> string (not "
-              + "\"" + NullDirection.FIRST.name() + "\", "
-              + "\"" + NullDirection.LAST.name() + "\", or "
-              + "\"" + NullDirection.UNSPECIFIED.name() + "\" or null): "
-              + "\"" + strNullOrdering + "\"" );
-        }
+                  + "\"" + NullDirection.FIRST.name() + "\", "
+                  + "\"" + NullDirection.LAST.name() + "\", or "
+                  + "\"" + NullDirection.UNSPECIFIED.name() + "\" or null): "
+                  + "\"" + nullDirection + "\"" );
       }
-      return nullOrdering;
    }
 
     @Override

http://git-wip-us.apache.org/repos/asf/drill/blob/1d1acc09/logical/src/test/java/org/apache/drill/common/logical/data/OrderTest.java
----------------------------------------------------------------------
diff --git a/logical/src/test/java/org/apache/drill/common/logical/data/OrderTest.java b/logical/src/test/java/org/apache/drill/common/logical/data/OrderTest.java
index 80b4e3c..9d55742 100644
--- a/logical/src/test/java/org/apache/drill/common/logical/data/OrderTest.java
+++ b/logical/src/test/java/org/apache/drill/common/logical/data/OrderTest.java
@@ -42,7 +42,7 @@ public class OrderTest {
   public void test_Ordering_roundTripAscAndNullsFirst() {
     Ordering src = new Ordering( Direction.ASCENDING, null, NullDirection.FIRST);
     Ordering reconstituted =
-        new Ordering( (LogicalExpression) null, src.getOrder(), src.getNullDirection().toString() );
+        new Ordering( src.getDirection(), (LogicalExpression) null, src.getNullDirection() );
     assertThat( reconstituted.getDirection(), equalTo( RelFieldCollation.Direction.ASCENDING  ) );
     assertThat( reconstituted.getNullDirection(), equalTo( NullDirection.FIRST  ) );
   }
@@ -51,7 +51,7 @@ public class OrderTest {
   public void test_Ordering_roundTripDescAndNullsLast() {
     Ordering src = new Ordering( Direction.DESCENDING, null, NullDirection.LAST);
     Ordering reconstituted =
-        new Ordering( (LogicalExpression) null, src.getOrder(), src.getNullDirection().toString() );
+        new Ordering( src.getDirection(), (LogicalExpression) null, src.getNullDirection() );
     assertThat( reconstituted.getDirection(), equalTo( RelFieldCollation.Direction.DESCENDING  ) );
     assertThat( reconstituted.getNullDirection(), equalTo( NullDirection.LAST  ) );
   }
@@ -60,22 +60,20 @@ public class OrderTest {
   public void test_Ordering_roundTripDescAndNullsUnspecified() {
     Ordering src = new Ordering( Direction.DESCENDING, null, NullDirection.UNSPECIFIED);
     Ordering reconstituted =
-        new Ordering( (LogicalExpression) null, src.getOrder(), src.getNullDirection().toString() );
+        new Ordering( src.getDirection(), (LogicalExpression) null, src.getNullDirection() );
     assertThat( reconstituted.getDirection(), equalTo( RelFieldCollation.Direction.DESCENDING  ) );
     assertThat( reconstituted.getNullDirection(), equalTo( NullDirection.UNSPECIFIED  ) );
   }
 
-
   // Basic input validation:
-
   @Test( expected = DrillRuntimeException.class )  // (Currently.)
   public void test_Ordering_garbageOrderRejected() {
-    new Ordering( (LogicalExpression) null, "AS CE ND IN G", (String) null );
+    new Ordering( "AS CE ND IN G", null, null );
   }
 
   @Test( expected = DrillRuntimeException.class )  // (Currently.)
   public void test_Ordering_garbageNullOrderingRejected() {
-    new Ordering( (LogicalExpression) null, (String) null, "HIGH" );
+    new Ordering( null, null, "HIGH" );
   }
 
 
@@ -83,7 +81,7 @@ public class OrderTest {
 
   @Test
   public void testOrdering_nullStrings() {
-    Ordering ordering = new Ordering( (LogicalExpression) null, null, null );
+    Ordering ordering = new Ordering( (String) null, (LogicalExpression) null, null );
     assertThat( ordering.getDirection(), equalTo( RelFieldCollation.Direction.ASCENDING ) );
     assertThat( ordering.getNullDirection(), equalTo( RelFieldCollation.NullDirection.UNSPECIFIED ) );
     assertThat( ordering.getOrder(), equalTo( "ASC" ) );