Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/308#discussion_r206295564
--- 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 --
Please add comment about reverse sort and point to JIRA (create one if
there's not already one). Might be best to just have the code here such that
once reverse sort is supported, the hash aggregate will just work.
---
