Repository: incubator-drill Updated Branches: refs/heads/master ecaa838fe -> a88102bfa
DRILL-536: Check for number of aggregate functions. Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/8296be55 Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/8296be55 Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/8296be55 Branch: refs/heads/master Commit: 8296be5594fa98da5a1d37a5fde871b6de4bca00 Parents: ecaa838 Author: Aman Sinha <[email protected]> Authored: Wed Apr 16 19:10:21 2014 -0700 Committer: Jacques Nadeau <[email protected]> Committed: Sat Apr 19 21:06:56 2014 -0700 ---------------------------------------------------------------------- .../physical/impl/aggregate/HashAggBatch.java | 12 +++-- .../impl/aggregate/HashAggTemplate.java | 21 +++++--- .../exec/physical/impl/agg/TestHashAggr.java | 5 ++ .../src/test/resources/agg/hashagg/q7_3.json | 56 ++++++++++++++++++++ 4 files changed, 82 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/8296be55/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 9add544..a75aac9 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 @@ -181,15 +181,17 @@ public class HashAggBatch extends AbstractRecordBatch<HashAggregate> { List<VectorAllocator> keyAllocators = Lists.newArrayList(); List<VectorAllocator> valueAllocators = Lists.newArrayList(); - aggrExprs = new LogicalExpression[popConfig.getAggrExprs().length]; - groupByOutFieldIds = new TypedFieldId[popConfig.getGroupByExprs().length]; - aggrOutFieldIds = new TypedFieldId[popConfig.getAggrExprs().length]; + int numGroupByExprs = (popConfig.getGroupByExprs() != null) ? popConfig.getGroupByExprs().length : 0; + int numAggrExprs = (popConfig.getAggrExprs() != null) ? popConfig.getAggrExprs().length : 0; + aggrExprs = new LogicalExpression[numAggrExprs]; + groupByOutFieldIds = new TypedFieldId[numGroupByExprs]; + aggrOutFieldIds = new TypedFieldId[numAggrExprs]; ErrorCollector collector = new ErrorCollectorImpl(); int i; - for(i = 0; i < popConfig.getGroupByExprs().length; i++) { + for(i = 0; i < numGroupByExprs; i++) { NamedExpression ne = popConfig.getGroupByExprs()[i]; final LogicalExpression expr = ExpressionTreeMaterializer.materialize(ne.getExpr(), incoming, collector, context.getFunctionRegistry() ); if(expr == null) continue; @@ -202,7 +204,7 @@ public class HashAggBatch extends AbstractRecordBatch<HashAggregate> { groupByOutFieldIds[i] = container.add(vv); } - for(i = 0; i < aggrExprs.length; i++){ + for(i = 0; i < numAggrExprs; i++){ NamedExpression ne = popConfig.getAggrExprs()[i]; final LogicalExpression expr = ExpressionTreeMaterializer.materialize(ne.getExpr(), incoming, collector, context.getFunctionRegistry() ); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/8296be55/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 21c0c7d..b0f81ef 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 @@ -155,9 +155,14 @@ public abstract class HashAggTemplate implements HashAggregator { TypedFieldId[] groupByOutFieldIds, VectorAllocator[] keyAllocators, VectorAllocator[] valueAllocators) throws SchemaChangeException, ClassTransformationException, IOException { - - if (valueFieldIds.size() < valueExprs.length) throw new IllegalArgumentException("Wrong number of workspace variables."); - + + if (valueExprs == null || valueFieldIds == null) { + throw new IllegalArgumentException("Invalid aggr value exprs or workspace variables."); + } + if (valueFieldIds.size() < valueExprs.length) { + throw new IllegalArgumentException("Wrong number of workspace variables."); + } + this.context = context; this.incoming = incoming; this.schema = incoming.getSchema(); @@ -180,10 +185,12 @@ public abstract class HashAggTemplate implements HashAggregator { this.htIdxHolder = new IntHolder(); materializedValueFields = new MaterializedField[valueFieldIds.size()]; - int i = 0; - FieldReference ref = new FieldReference("dummy", ExpressionPosition.UNKNOWN, valueFieldIds.get(0).getType()); - for (TypedFieldId id : valueFieldIds) { - materializedValueFields[i++] = MaterializedField.create(ref, id.getType()); + if (valueFieldIds.size() > 0) { + int i = 0; + FieldReference ref = new FieldReference("dummy", ExpressionPosition.UNKNOWN, valueFieldIds.get(0).getType()); + for (TypedFieldId id : valueFieldIds) { + materializedValueFields[i++] = MaterializedField.create(ref, id.getType()); + } } ChainedHashTable ht = new ChainedHashTable(hashAggrConfig.getHtConfig(), context, incoming, null /* no incoming probe */, outgoing) ; http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/8296be55/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java index 1ab7248..9047208 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java @@ -39,6 +39,11 @@ public class TestHashAggr extends BaseTestQuery{ public void testQ7_2() throws Exception{ testPhysicalFromFile("agg/hashagg/q7_2.json"); } + + @Test + public void testQ7_3() throws Exception{ + testPhysicalFromFile("agg/hashagg/q7_3.json"); + } @Ignore // ignore temporarily since this shows memory leak in ParquetRecordReader (DRILL-443) @Test http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/8296be55/exec/java-exec/src/test/resources/agg/hashagg/q7_3.json ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/resources/agg/hashagg/q7_3.json b/exec/java-exec/src/test/resources/agg/hashagg/q7_3.json new file mode 100644 index 0000000..11d2665 --- /dev/null +++ b/exec/java-exec/src/test/resources/agg/hashagg/q7_3.json @@ -0,0 +1,56 @@ +{ + head : { + version : 1, + generator : { + type : "optiq", + info : "na" + }, + type : "APACHE_DRILL_PHYSICAL" + }, + graph : [ { + "pop" : "parquet-scan", + "@id" : 1, + "entries" : [ { + "path" : "tpch/nation.parquet" + } ], + "storage" : { + "type" : "file", + "connection" : "classpath:///" + }, + "format" : { + "type" : "parquet" + } + }, { + pop : "project", + @id : 2, + exprs : [ { + ref : "output.$f0", + expr : "N_REGIONKEY" + }, { + ref : "output.$f1", + expr : "N_NATIONKEY" + }, { + ref : "output.$f2", + expr : "N_NAME" + } ], + child : 1 + }, { + pop : "hash-aggregate", + @id : 3, + child : 2, + keys : [ { + ref : "$f0", + expr : "$f0" + }, { + ref : "$f1", + expr : "$f1" + }, { + ref : "$f2", + expr : "$f2 " + } ] + }, { + pop : "screen", + @id : 4, + child : 3 + } ] +}
