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

[JamesRTaylor]

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

    https://github.com/apache/phoenix/pull/308#discussion_r205231380
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientAggregatePlan.java 
---
    @@ -135,17 +142,25 @@ 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) {
    +                    boolean sort = orderBy == OrderBy.FWD_ROW_KEY_ORDER_BY;
    +                    aggResultIterator = new 
ClientHashAggregatingResultIterator(context, iterator, serverAggregators, 
keyExpressions, sort);
    --- End diff --
    
    You’ll need to pass through if forward or reverse scan. You might just 
pass through orderBy instead of the boolean. Better still would be to let the 
code below insert an Ordering result iterator so you wouldn’t  need the sort 
logic at all in your new iterator.


---