Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/175#discussion_r69668528
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/AggregatePlan.java ---
@@ -133,7 +134,10 @@ public PeekingResultIterator
newIterator(StatementContext context, ResultIterato
Expression expression = RowKeyExpression.INSTANCE;
OrderByExpression orderByExpression = new
OrderByExpression(expression, false, true);
int threshold =
services.getProps().getInt(QueryServices.SPOOL_THRESHOLD_BYTES_ATTRIB,
QueryServicesOptions.DEFAULT_SPOOL_THRESHOLD_BYTES);
- return new OrderedResultIterator(scanner,
Collections.<OrderByExpression>singletonList(orderByExpression), threshold);
+ String spoolDirectory =
services.getProps().get(QueryServices.SPOOL_DIRECTORY,
QueryServicesOptions.DEFAULT_SPOOL_DIRECTORY);
--- End diff --
In my previous review I might not have made it very clear that the idea of
moving MemoryManager into constructors was more of a question rather than
advice. Since I hadn't studied the code so carefully, I was trying to ask you
if that would be better, or worse? Looks like these parameters are getting all
over the place and my suggestion was not a good one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
