[GitHub] phoenix pull request #308: Client-side hash aggregation

[geraldss]

Github user geraldss commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/308#discussion_r205206944
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientAggregatePlan.java 
---
    @@ -135,17 +142,24 @@ public ResultIterator iterator(ParallelScanGrouper 
scanGrouper, Scan scan) throw
                 aggResultIterator = new 
ClientUngroupedAggregatingResultIterator(LookAheadResultIterator.wrap(iterator),
 serverAggregators);
                 aggResultIterator = new 
UngroupedAggregatingResultIterator(LookAheadResultIterator.wrap(aggResultIterator),
 clientAggregators);
             } else {
    -            if (!groupBy.isOrderPreserving()) {
    -                int thresholdBytes = 
context.getConnection().getQueryServices().getProps().getInt(
    -                        QueryServices.SPOOL_THRESHOLD_BYTES_ATTRIB, 
QueryServicesOptions.DEFAULT_SPOOL_THRESHOLD_BYTES);
    -                List<Expression> keyExpressions = 
groupBy.getKeyExpressions();
    +            List<Expression> keyExpressions = groupBy.getKeyExpressions();
    +            if (groupBy.isOrderPreserving()) {
    +                aggResultIterator = new 
ClientGroupedAggregatingResultIterator(LookAheadResultIterator.wrap(iterator), 
serverAggregators, keyExpressions);
    +            } else {
    +                int thresholdBytes = 
context.getConnection().getQueryServices().getProps().getInt
    +                    (QueryServices.SPOOL_THRESHOLD_BYTES_ATTRIB, 
QueryServicesOptions.DEFAULT_SPOOL_THRESHOLD_BYTES);
                     List<OrderByExpression> keyExpressionOrderBy = 
Lists.newArrayListWithExpectedSize(keyExpressions.size());
                     for (Expression keyExpression : keyExpressions) {
                         keyExpressionOrderBy.add(new 
OrderByExpression(keyExpression, false, true));
                     }
    -                iterator = new OrderedResultIterator(iterator, 
keyExpressionOrderBy, thresholdBytes, null, null, 
projector.getEstimatedRowByteSize());
    +
    +                if (useHashAgg) {
    +                    aggResultIterator = new 
ClientHashAggregatingResultIterator(context, iterator, serverAggregators, 
keyExpressions);
    +                } else {
    +                    iterator = new OrderedResultIterator(iterator, 
keyExpressionOrderBy, thresholdBytes, null, null, 
projector.getEstimatedRowByteSize());
    +                    aggResultIterator = new 
ClientGroupedAggregatingResultIterator(LookAheadResultIterator.wrap(iterator), 
serverAggregators, keyExpressions);
    +                }
                 }
    -            aggResultIterator = new 
ClientGroupedAggregatingResultIterator(LookAheadResultIterator.wrap(iterator), 
serverAggregators, groupBy.getKeyExpressions());
                 aggResultIterator = new 
GroupedAggregatingResultIterator(LookAheadResultIterator.wrap(aggResultIterator),
 clientAggregators);
             }
     
    --- End diff --
    
    Hi @JamesRTaylor - please review. The sort is now done only when necessary. 
Thanks.


---