[GitHub] [incubator-iotdb] JackieTien97 commented on a change in pull request #857: [IOTDB-458] parallel aggregation query

[GitBox <gi...@apache.org>]

JackieTien97 commented on a change in pull request #857: [IOTDB-458] parallel aggregation query
URL: https://github.com/apache/incubator-iotdb/pull/857#discussion_r386165639
 
 

 ##########
 File path: server/src/main/java/org/apache/iotdb/db/query/executor/AggregationExecutor.java
 ##########
 @@ -72,131 +81,169 @@ public AggregationExecutor(AggregationPlan aggregationPlan) {
   }
 
   /**
-   * execute aggregate function with only time filter or no filter.
-   *
-   * @param context query context
+   * Aggregate one series
    */
-  public QueryDataSet executeWithoutValueFilter(QueryContext context)
-      throws StorageEngineException, IOException, QueryProcessException {
+  class AggregationTask implements Callable<Pair<Path, List<AggregateResult>>> {
 
-    Filter timeFilter = null;
-    if (expression != null) {
-      timeFilter = ((GlobalTimeExpression) expression).getFilter();
+    // path to aggregation result indexes
+    private Map.Entry<Path, List<Integer>> pathToAggrIndexes;
+    private IAggregateReader reader;
+
+    public AggregationTask(IAggregateReader reader, Map.Entry<Path, List<Integer>> pathToAggrIndexes) {
+      this.reader = reader;
+      this.pathToAggrIndexes = pathToAggrIndexes;
     }
 
-    // TODO use multi-thread
-    Map<Path, List<Integer>> pathToAggrIndexesMap = groupAggregationsBySeries(selectedSeries);
-    AggregateResult[] aggregateResultList = new AggregateResult[selectedSeries.size()];
-    for (Map.Entry<Path, List<Integer>> entry : pathToAggrIndexesMap.entrySet()) {
-      List<AggregateResult> aggregateResults = aggregateOneSeries(entry, timeFilter, context);
-      int index = 0;
-      for (int i : entry.getValue()) {
-        aggregateResultList[i] = aggregateResults.get(index);
-        index++;
-      }
+    @Override
+    public Pair<Path, List<AggregateResult>> call() throws QueryProcessException, IOException {
+      return aggregateOneSeries();
 
 Review comment:
   It seems to be no need to have the aggregateOneSeries method. You can just move it into the call() function.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services