Github user geraldss commented on a diff in the pull request: https://github.com/apache/phoenix/pull/308#discussion_r205296815 --- 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; --- End diff -- The GROUP BY cannot cause REV_ROW_KEY_ORDER_BY, because the GROUP BY cannot specific or produce descending keys. This is a pre-existing assumption + design in SORT_MERGE_JOIN. The SORT_MERGE_JOIN creates its own forward sort, and its Tracker reports a forward sort.
---