From dffccb1e663fa21a5ea8ffed4d956aae8d6bf35b Mon Sep 17 00:00:00 2001
From: "navis.ryu" <navis@apache.org>
Date: Tue, 22 Dec 2015 18:03:18 +0900
Subject: [PATCH 1/3] Result of GroupByQueryRunner should not be considered to
 be ordered

---
 .../query/groupby/GroupByQueryEngine.java     |   7 +-
 .../query/groupby/GroupByQueryRunnerTest.java | 133 ++++++++++--------
 .../java/io/druid/segment/TestHelper.java     |  16 +++
 3 files changed, 92 insertions(+), 64 deletions(-)

diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
index 110d8bef6d7c..cd62a0a1e9f7 100644
--- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
+++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
@@ -149,9 +149,7 @@ private static class RowUpdater
     private final BufferAggregator[] aggregators;
     private final PositionMaintainer positionMaintainer;
 
-    private final Map<ByteBuffer, Integer> positions = Maps.newTreeMap();
-    // GroupBy queries tend to do a lot of reads from this. We co-store a hash map to make those reads go faster.
-    private final Map<ByteBuffer, Integer> positionsHash = Maps.newHashMap();
+    private final Map<ByteBuffer, Integer> positions = Maps.newHashMap();
 
     public RowUpdater(
         ByteBuffer metricValues,
@@ -205,7 +203,7 @@ private List<ByteBuffer> updateValues(
         return retVal;
       } else {
         key.clear();
-        Integer position = positionsHash.get(key);
+        Integer position = positions.get(key);
         int[] increments = positionMaintainer.getIncrements();
         int thePosition;
 
@@ -220,7 +218,6 @@ private List<ByteBuffer> updateValues(
           }
 
           positions.put(keyCopy, position);
-          positionsHash.put(keyCopy, position);
           thePosition = position;
           for (int i = 0; i < aggregators.length; ++i) {
             aggregators[i].init(metricValues, thePosition);
diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
index 42be3231b6b5..6784edb2bdd1 100644
--- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
+++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
@@ -261,7 +261,22 @@ public void testGroupBy()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
+  }
+
+  private void assertExpectedObjects(List<Row> expectedResults, Iterable<Row> results, String msg)
+  {
+    TestHelper.assertObjectsUnordered(expectedResults, results, msg);
+  }
+
+  private void assertExpectedObjects(Iterable<Row> expectedResults, Sequence<Row> results, String msg)
+  {
+    TestHelper.assertObjectsUnordered(expectedResults, Sequences.toList(results, new ArrayList()), msg);
+  }
+
+  private void assertExpectedObjects(List<Result> expectedResults, Sequence results, String msg)
+  {
+    TestHelper.assertObjectsUnordered(expectedResults, Sequences.toList(results, new ArrayList()), msg);
   }
 
 
@@ -361,7 +376,7 @@ public void testGroupByWithRebucketRename()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -440,7 +455,7 @@ public void testGroupByWithSimpleRenameRetainMissingNonInjective()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -519,7 +534,7 @@ public void testGroupByWithSimpleRenameRetainMissing()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -598,7 +613,7 @@ public void testGroupByWithSimpleRenameAndMissingString()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -676,7 +691,7 @@ public void testGroupByWithSimpleRename()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -706,7 +721,7 @@ public void testGroupByWithUniques()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test(expected = IllegalArgumentException.class)
@@ -744,7 +759,7 @@ public void testGroupByWithUniquesAndPostAggWithSameName()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -774,7 +789,7 @@ public void testGroupByWithCardinality()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -832,7 +847,7 @@ public String apply(String dimValue)
         GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "t", "rows", 2L, "idx", 223L)
     );
 
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         expectedResults,
         GroupByQueryRunnerTestHelper.runQuery(factory, runner, query),
         ""
@@ -900,7 +915,7 @@ public String apply(String dimValue)
         GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "t", "rows", 2L, "idx", 223L)
     );
 
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         expectedResults,
         GroupByQueryRunnerTestHelper.runQuery(factory, runner, query),
         ""
@@ -1108,7 +1123,7 @@ public void testGroupByWithTimeZone()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -1163,8 +1178,8 @@ public Sequence<Row> run(
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(expectedResults, runner.run(fullQuery, context), "direct");
-    TestHelper.assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
+    assertExpectedObjects(expectedResults, runner.run(fullQuery, context), "direct");
+    assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
 
     List<Row> allGranExpectedResults = Arrays.asList(
         GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "automotive", "rows", 2L, "idx", 269L),
@@ -1178,8 +1193,8 @@ public Sequence<Row> run(
         GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "travel", "rows", 2L, "idx", 243L)
     );
 
-    TestHelper.assertExpectedObjects(allGranExpectedResults, runner.run(allGranQuery, context), "direct");
-    TestHelper.assertExpectedObjects(allGranExpectedResults, mergedRunner.run(allGranQuery, context), "merged");
+    assertExpectedObjects(allGranExpectedResults, runner.run(allGranQuery, context), "direct");
+    assertExpectedObjects(allGranExpectedResults, mergedRunner.run(allGranQuery, context), "merged");
   }
 
   @Test
@@ -1223,7 +1238,7 @@ private void doTestMergeResultsWithValidLimit(final int limit)
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
 
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         Iterables.limit(expectedResults, limit), mergeRunner.run(fullQuery, context), String.format("limit: %d", limit)
     );
   }
@@ -1270,7 +1285,7 @@ public void testMergeResultsAcrossMultipleDaysWithLimitAndOrderBy()
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
 
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         Iterables.limit(expectedResults, limit), mergeRunner.run(fullQuery, context), String.format("limit: %d", limit)
     );
   }
@@ -1395,7 +1410,7 @@ public Sequence<Row> run(
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
+    assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
   }
 
   @Test
@@ -1432,9 +1447,9 @@ public void testGroupByOrderLimit() throws Exception
 
     Map<String, Object> context = Maps.newHashMap();
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
-    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
 
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         Iterables.limit(expectedResults, 5), mergeRunner.run(builder.limit(5).build(), context), "limited"
     );
   }
@@ -1473,8 +1488,8 @@ public void testGroupByWithOrderLimit2() throws Exception
 
     Map<String, Object> context = Maps.newHashMap();
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
-    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    assertExpectedObjects(
         Iterables.limit(expectedResults, 5), mergeRunner.run(builder.limit(5).build(), context), "limited"
     );
   }
@@ -1585,8 +1600,8 @@ public void testGroupByWithOrderLimit3() throws Exception
 
     Map<String, Object> context = Maps.newHashMap();
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
-    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    assertExpectedObjects(
         Iterables.limit(expectedResults, 5), mergeRunner.run(builder.limit(5).build(), context), "limited"
     );
   }
@@ -1648,7 +1663,7 @@ public void testGroupByWithSameCaseOrdering()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "order-limit");
+    assertExpectedObjects(expectedResults, results, "order-limit");
   }
 
   @Test
@@ -1696,7 +1711,7 @@ public void testGroupByWithOrderLimit4()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "order-limit");
+    assertExpectedObjects(expectedResults, results, "order-limit");
   }
 
   @Test
@@ -2166,7 +2181,7 @@ public Sequence<Row> run(
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
+    assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
   }
 
   @Test
@@ -2247,7 +2262,7 @@ public void testGroupByWithOrderLimitHavingSpec()
 
     GroupByQuery fullQuery = builder.build();
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, fullQuery);
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         expectedResults,
         results,
         ""
@@ -2304,7 +2319,7 @@ public void testPostAggHavingSpec()
         );
 
     final GroupByQuery fullQuery = builder.build();
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         expectedResults,
         GroupByQueryRunnerTestHelper.runQuery(factory, runner, fullQuery),
         ""
@@ -2343,7 +2358,7 @@ public void testHavingSpec()
         );
 
     final GroupByQuery fullQuery = builder.build();
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         expectedResults,
         GroupByQueryRunnerTestHelper.runQuery(factory, runner, fullQuery),
         ""
@@ -2403,7 +2418,7 @@ public Sequence<Row> run(
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
+    assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
   }
 
   @Test
@@ -2511,7 +2526,7 @@ public Sequence<Row> run(
 
     Map<String, Object> context = Maps.newHashMap();
     // add an extra layer of merging, simulate broker forwarding query to historical
-    TestHelper.assertExpectedObjects(
+    assertExpectedObjects(
         expectedResults,
         factory.getToolchest().postMergeQueryDecoration(
             factory.getToolchest().mergeResults(
@@ -2566,7 +2581,7 @@ public ByteBuffer get()
         QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator()
     ).mergeResults(runner);
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
   }
 
   @Test
@@ -2624,7 +2639,7 @@ public void testGroupByWithMetricColumnDisappears() throws Exception
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(expectedResults, runner.run(query, context), "normal");
+    assertExpectedObjects(expectedResults, runner.run(query, context), "normal");
     final GroupByQueryEngine engine = new GroupByQueryEngine(
         configSupplier,
         new StupidPool<>(
@@ -2646,7 +2661,7 @@ public ByteBuffer get()
         TestQueryRunners.pool,
         QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator()
     ).mergeResults(runner);
-    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
   }
 
   @Test
@@ -2704,7 +2719,7 @@ public void testGroupByWithNonexistentDimension() throws Exception
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    TestHelper.assertExpectedObjects(expectedResults, runner.run(query, context), "normal");
+    assertExpectedObjects(expectedResults, runner.run(query, context), "normal");
     final GroupByQueryEngine engine = new GroupByQueryEngine(
         configSupplier,
         new StupidPool<>(
@@ -2726,7 +2741,7 @@ public ByteBuffer get()
         TestQueryRunners.pool,
         QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator()
     ).mergeResults(runner);
-    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
   }
 
   // A subquery identical to the query should yield identical results
@@ -2791,7 +2806,7 @@ public void testIdenticalSubquery()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -2862,7 +2877,7 @@ public void testSubqueryWithMultipleIntervalsInOuterQuery()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -2962,7 +2977,7 @@ public void testDifferentGroupingSubquery()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3019,7 +3034,7 @@ public void testDifferentGroupingSubqueryMultipleAggregatorsOnSameField()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -3073,7 +3088,7 @@ public void testDifferentGroupingSubqueryWithFilter()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3110,7 +3125,7 @@ public void testDifferentIntervalSubquery()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3409,7 +3424,7 @@ public void testSubqueryWithPostAggregators()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3672,7 +3687,7 @@ public byte[] getCacheKey()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3843,7 +3858,7 @@ public byte[] getCacheKey()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3983,7 +3998,7 @@ public void testSubqueryWithHyperUniques()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4045,7 +4060,7 @@ public void testSubqueryWithHyperUniquesPostAggregator()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4078,7 +4093,7 @@ public void testGroupByWithTimeColumn()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4303,7 +4318,7 @@ public void testGroupByTimeExtraction()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4359,7 +4374,7 @@ public void testBySegmentResults()
         )
     );
 
-    TestHelper.assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
+    assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
     exec.shutdownNow();
   }
 
@@ -4435,7 +4450,7 @@ public void testBySegmentResultsUnOptimizedDimextraction()
         )
     );
 
-    TestHelper.assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
+    assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
     exec.shutdownNow();
   }
 
@@ -4510,7 +4525,7 @@ public void testBySegmentResultsOptimizedDimextraction()
         )
     );
 
-    TestHelper.assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
+    assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
     exec.shutdownNow();
   }
 
@@ -4579,7 +4594,7 @@ public void testGroupByWithExtractionDimFilter()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
 
   }
 
@@ -4626,7 +4641,7 @@ public void testGroupByWithExtractionDimFilterCaseMappingValueIsNullOrEmpty()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4664,7 +4679,7 @@ public void testGroupByWithExtractionDimFilterWhenSearchValueNotInTheMap()
     List<Row> expectedResults = Arrays.asList();
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -4709,7 +4724,7 @@ public void testGroupByWithExtractionDimFilterKeyisNull()
         );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4775,7 +4790,7 @@ public void testGroupByWithAggregatorFilterAndExtractionFunction()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    TestHelper.assertExpectedObjects(expectedResults, results, "");
+    assertExpectedObjects(expectedResults, results, "");
 
   }
 
diff --git a/processing/src/test/java/io/druid/segment/TestHelper.java b/processing/src/test/java/io/druid/segment/TestHelper.java
index d86ff55ad093..9fa85a2e8ba4 100644
--- a/processing/src/test/java/io/druid/segment/TestHelper.java
+++ b/processing/src/test/java/io/druid/segment/TestHelper.java
@@ -30,6 +30,7 @@
 import org.junit.Assert;
 
 import java.util.Iterator;
+import java.util.List;
 
 /**
  */
@@ -167,6 +168,21 @@ private static <T> void assertResults(
     }
   }
 
+  public static <T> void assertObjectsUnordered(Iterable<T> expectedResults, Iterable<T> actualResults, String failMsg)
+  {
+    int counter = 0;
+    List expected = Lists.newLinkedList(expectedResults);
+    for (Object t : actualResults) {
+      counter++;
+      if (!expected.remove(t)) {
+        Assert.fail(failMsg + ": " + counter + " th return value " + t + " was not in expected value list");
+      }
+    }
+    if (!expected.isEmpty()) {
+      Assert.fail(failMsg + ": Some expected values was not arrived something like " + expected.get(0));
+    }
+  }
+
   private static <T> void assertObjects(Iterable<T> expectedResults, Iterable<T> actualResults, String msg)
   {
     Iterator resultsIter = actualResults.iterator();

From 204c548dd081153aca31023681d241d955a102cd Mon Sep 17 00:00:00 2001
From: "navis.ryu" <navis@apache.org>
Date: Fri, 15 Jan 2016 11:29:07 +0900
Subject: [PATCH 2/3] sort only for merge

---
 .../query/groupby/GroupByQueryEngine.java     |  87 ++++++++++--
 .../groupby/GroupByQueryQueryToolChest.java   |   3 +-
 .../groupby/GroupByQueryRunnerFactory.java    |   2 +-
 .../query/groupby/GroupByQueryRunnerTest.java | 134 ++++++++----------
 .../java/io/druid/segment/TestHelper.java     |  16 ---
 .../IncrementalIndexStorageAdapterTest.java   |   9 +-
 6 files changed, 141 insertions(+), 110 deletions(-)

diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
index cd62a0a1e9f7..7efa4a6ed6fb 100644
--- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
+++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
@@ -81,7 +81,7 @@ public GroupByQueryEngine(
     this.intermediateResultsBufferPool = intermediateResultsBufferPool;
   }
 
-  public Sequence<Row> process(final GroupByQuery query, final StorageAdapter storageAdapter)
+  public Sequence<Row> process(final GroupByQuery query, final StorageAdapter storageAdapter, final boolean sort)
   {
     if (storageAdapter == null) {
       throw new ISE(
@@ -118,7 +118,7 @@ public Sequence<Row> apply(final Cursor cursor)
                           @Override
                           public RowIterator make()
                           {
-                            return new RowIterator(query, cursor, bufferHolder.get(), config.get());
+                            return new RowIterator(query, cursor, bufferHolder.get(), config.get(), sort);
                           }
 
                           @Override
@@ -143,13 +143,13 @@ public void close() throws IOException
     );
   }
 
-  private static class RowUpdater
+  private static abstract class RowUpdater
   {
     private final ByteBuffer metricValues;
     private final BufferAggregator[] aggregators;
     private final PositionMaintainer positionMaintainer;
 
-    private final Map<ByteBuffer, Integer> positions = Maps.newHashMap();
+    protected final Map<ByteBuffer, Integer> positions = Maps.newTreeMap();
 
     public RowUpdater(
         ByteBuffer metricValues,
@@ -162,15 +162,13 @@ public RowUpdater(
       this.positionMaintainer = positionMaintainer;
     }
 
-    public int getNumRows()
-    {
+    protected int getNumRows() {
       return positions.size();
     }
 
-    public Map<ByteBuffer, Integer> getPositions()
-    {
-      return positions;
-    }
+    protected abstract Map<ByteBuffer, Integer> getPositions();
+
+    protected abstract void putPosition(ByteBuffer key, int position);
 
     private List<ByteBuffer> updateValues(
         ByteBuffer key,
@@ -217,7 +215,8 @@ private List<ByteBuffer> updateValues(
             return Lists.newArrayList(keyCopy);
           }
 
-          positions.put(keyCopy, position);
+          putPosition(keyCopy, position);
+
           thePosition = position;
           for (int i = 0; i < aggregators.length; ++i) {
             aggregators[i].init(metricValues, thePosition);
@@ -235,6 +234,55 @@ private List<ByteBuffer> updateValues(
     }
   }
 
+  private static class UnsortedRowUpdater extends RowUpdater
+  {
+    public UnsortedRowUpdater(
+        ByteBuffer metricValues,
+        BufferAggregator[] aggregators,
+        PositionMaintainer positionMaintainer
+    )
+    {
+      super(metricValues, aggregators, positionMaintainer);
+    }
+
+    protected final Map<ByteBuffer, Integer> getPositions()
+    {
+      return positions;
+    }
+
+    protected final void putPosition(ByteBuffer key, int position)
+    {
+      positions.put(key, position);
+    }
+  }
+
+  private static class SortedUpdater extends RowUpdater
+  {
+    private final Map<ByteBuffer, Integer> sortedPositions = Maps.newTreeMap();
+
+    public SortedUpdater(
+        ByteBuffer metricValues,
+        BufferAggregator[] aggregators,
+        PositionMaintainer positionMaintainer
+    )
+    {
+      super(metricValues, aggregators, positionMaintainer);
+    }
+
+    @Override
+    protected final Map<ByteBuffer, Integer> getPositions()
+    {
+      return sortedPositions;
+    }
+
+    @Override
+    protected final void putPosition(ByteBuffer key, int position)
+    {
+      positions.put(key, position);
+      sortedPositions.put(key, position);
+    }
+  }
+
   private static class PositionMaintainer
   {
     private final int[] increments;
@@ -289,6 +337,8 @@ private static class RowIterator implements CloseableIterator<Row>
     private final Cursor cursor;
     private final ByteBuffer metricsBuffer;
     private final int maxIntermediateRows;
+    private final GroupByQueryConfig config;
+    private final boolean sort;
 
     private final List<DimensionSpec> dimensionSpecs;
     private final List<DimensionSelector> dimensions;
@@ -301,18 +351,25 @@ private static class RowIterator implements CloseableIterator<Row>
     private List<ByteBuffer> unprocessedKeys;
     private Iterator<Row> delegate;
 
-    public RowIterator(GroupByQuery query, final Cursor cursor, ByteBuffer metricsBuffer, GroupByQueryConfig config)
+    public RowIterator(
+        GroupByQuery query,
+        final Cursor cursor,
+        ByteBuffer metricsBuffer,
+        GroupByQueryConfig config,
+        boolean sort
+    )
     {
       this.query = query;
       this.cursor = cursor;
       this.metricsBuffer = metricsBuffer;
-
       this.maxIntermediateRows = Math.min(
           query.getContextValue(
               CTX_KEY_MAX_INTERMEDIATE_ROWS,
               config.getMaxIntermediateRows()
           ), config.getMaxIntermediateRows()
       );
+      this.config = config;
+      this.sort = sort;
 
       unprocessedKeys = null;
       delegate = Iterators.emptyIterator();
@@ -359,7 +416,9 @@ public Row next()
       }
 
       final PositionMaintainer positionMaintainer = new PositionMaintainer(0, sizesRequired, metricsBuffer.remaining());
-      final RowUpdater rowUpdater = new RowUpdater(metricsBuffer, aggregators, positionMaintainer);
+      final RowUpdater rowUpdater = sort
+                                    ? new SortedUpdater(metricsBuffer, aggregators, positionMaintainer)
+                                    : new UnsortedRowUpdater(metricsBuffer, aggregators, positionMaintainer);
       if (unprocessedKeys != null) {
         for (ByteBuffer key : unprocessedKeys) {
           final List<ByteBuffer> unprocUnproc = rowUpdater.updateValues(key, ImmutableList.<DimensionSelector>of());
diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java
index a3c77245a832..f205629d9aa2 100644
--- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java
+++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java
@@ -238,7 +238,8 @@ public Sequence<Row> apply(Interval interval)
                           outerQuery.withQuerySegmentSpec(
                               new MultipleIntervalSegmentSpec(ImmutableList.of(interval))
                           ),
-                          new IncrementalIndexStorageAdapter(innerQueryResultIndex)
+                          new IncrementalIndexStorageAdapter(innerQueryResultIndex),
+                          true
                       );
                     }
                   }
diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryRunnerFactory.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryRunnerFactory.java
index bdafd6fef646..f2320a873e52 100644
--- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryRunnerFactory.java
+++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryRunnerFactory.java
@@ -107,7 +107,7 @@ public Sequence<Row> run(Query<Row> input, Map<String, Object> responseContext)
         throw new ISE("Got a [%s] which isn't a %s", input.getClass(), GroupByQuery.class);
       }
 
-      return engine.process((GroupByQuery) input, adapter);
+      return engine.process((GroupByQuery) input, adapter, false);
     }
   }
 }
diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
index 6784edb2bdd1..6d9e5d9f33c5 100644
--- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
+++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
@@ -108,7 +108,6 @@
 import javax.annotation.Nullable;
 import java.io.IOException;
 import java.nio.ByteBuffer;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
@@ -261,22 +260,7 @@ public void testGroupBy()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
-  }
-
-  private void assertExpectedObjects(List<Row> expectedResults, Iterable<Row> results, String msg)
-  {
-    TestHelper.assertObjectsUnordered(expectedResults, results, msg);
-  }
-
-  private void assertExpectedObjects(Iterable<Row> expectedResults, Sequence<Row> results, String msg)
-  {
-    TestHelper.assertObjectsUnordered(expectedResults, Sequences.toList(results, new ArrayList()), msg);
-  }
-
-  private void assertExpectedObjects(List<Result> expectedResults, Sequence results, String msg)
-  {
-    TestHelper.assertObjectsUnordered(expectedResults, Sequences.toList(results, new ArrayList()), msg);
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -376,7 +360,7 @@ public void testGroupByWithRebucketRename()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -455,7 +439,7 @@ public void testGroupByWithSimpleRenameRetainMissingNonInjective()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -534,7 +518,7 @@ public void testGroupByWithSimpleRenameRetainMissing()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -613,7 +597,7 @@ public void testGroupByWithSimpleRenameAndMissingString()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -691,7 +675,7 @@ public void testGroupByWithSimpleRename()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -721,7 +705,7 @@ public void testGroupByWithUniques()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test(expected = IllegalArgumentException.class)
@@ -759,7 +743,7 @@ public void testGroupByWithUniquesAndPostAggWithSameName()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -789,7 +773,7 @@ public void testGroupByWithCardinality()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -847,7 +831,7 @@ public String apply(String dimValue)
         GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "t", "rows", 2L, "idx", 223L)
     );
 
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         expectedResults,
         GroupByQueryRunnerTestHelper.runQuery(factory, runner, query),
         ""
@@ -915,7 +899,7 @@ public String apply(String dimValue)
         GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "t", "rows", 2L, "idx", 223L)
     );
 
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         expectedResults,
         GroupByQueryRunnerTestHelper.runQuery(factory, runner, query),
         ""
@@ -1123,7 +1107,7 @@ public void testGroupByWithTimeZone()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -1178,8 +1162,8 @@ public Sequence<Row> run(
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(expectedResults, runner.run(fullQuery, context), "direct");
-    assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
+    TestHelper.assertExpectedObjects(expectedResults, runner.run(fullQuery, context), "direct");
+    TestHelper.assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
 
     List<Row> allGranExpectedResults = Arrays.asList(
         GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "automotive", "rows", 2L, "idx", 269L),
@@ -1193,8 +1177,8 @@ public Sequence<Row> run(
         GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "travel", "rows", 2L, "idx", 243L)
     );
 
-    assertExpectedObjects(allGranExpectedResults, runner.run(allGranQuery, context), "direct");
-    assertExpectedObjects(allGranExpectedResults, mergedRunner.run(allGranQuery, context), "merged");
+    TestHelper.assertExpectedObjects(allGranExpectedResults, runner.run(allGranQuery, context), "direct");
+    TestHelper.assertExpectedObjects(allGranExpectedResults, mergedRunner.run(allGranQuery, context), "merged");
   }
 
   @Test
@@ -1238,7 +1222,7 @@ private void doTestMergeResultsWithValidLimit(final int limit)
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
 
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         Iterables.limit(expectedResults, limit), mergeRunner.run(fullQuery, context), String.format("limit: %d", limit)
     );
   }
@@ -1285,7 +1269,7 @@ public void testMergeResultsAcrossMultipleDaysWithLimitAndOrderBy()
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
 
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         Iterables.limit(expectedResults, limit), mergeRunner.run(fullQuery, context), String.format("limit: %d", limit)
     );
   }
@@ -1410,7 +1394,7 @@ public Sequence<Row> run(
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
+    TestHelper.assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
   }
 
   @Test
@@ -1447,9 +1431,9 @@ public void testGroupByOrderLimit() throws Exception
 
     Map<String, Object> context = Maps.newHashMap();
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
-    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
 
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         Iterables.limit(expectedResults, 5), mergeRunner.run(builder.limit(5).build(), context), "limited"
     );
   }
@@ -1488,8 +1472,8 @@ public void testGroupByWithOrderLimit2() throws Exception
 
     Map<String, Object> context = Maps.newHashMap();
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
-    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    TestHelper.assertExpectedObjects(
         Iterables.limit(expectedResults, 5), mergeRunner.run(builder.limit(5).build(), context), "limited"
     );
   }
@@ -1600,8 +1584,8 @@ public void testGroupByWithOrderLimit3() throws Exception
 
     Map<String, Object> context = Maps.newHashMap();
     QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
-    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    TestHelper.assertExpectedObjects(
         Iterables.limit(expectedResults, 5), mergeRunner.run(builder.limit(5).build(), context), "limited"
     );
   }
@@ -1663,7 +1647,7 @@ public void testGroupByWithSameCaseOrdering()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "order-limit");
+    TestHelper.assertExpectedObjects(expectedResults, results, "order-limit");
   }
 
   @Test
@@ -1711,7 +1695,7 @@ public void testGroupByWithOrderLimit4()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "order-limit");
+    TestHelper.assertExpectedObjects(expectedResults, results, "order-limit");
   }
 
   @Test
@@ -2181,7 +2165,7 @@ public Sequence<Row> run(
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
+    TestHelper.assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
   }
 
   @Test
@@ -2262,7 +2246,7 @@ public void testGroupByWithOrderLimitHavingSpec()
 
     GroupByQuery fullQuery = builder.build();
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, fullQuery);
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         expectedResults,
         results,
         ""
@@ -2319,7 +2303,7 @@ public void testPostAggHavingSpec()
         );
 
     final GroupByQuery fullQuery = builder.build();
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         expectedResults,
         GroupByQueryRunnerTestHelper.runQuery(factory, runner, fullQuery),
         ""
@@ -2358,7 +2342,7 @@ public void testHavingSpec()
         );
 
     final GroupByQuery fullQuery = builder.build();
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         expectedResults,
         GroupByQueryRunnerTestHelper.runQuery(factory, runner, fullQuery),
         ""
@@ -2418,7 +2402,7 @@ public Sequence<Row> run(
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
+    TestHelper.assertExpectedObjects(expectedResults, mergedRunner.run(fullQuery, context), "merged");
   }
 
   @Test
@@ -2526,7 +2510,7 @@ public Sequence<Row> run(
 
     Map<String, Object> context = Maps.newHashMap();
     // add an extra layer of merging, simulate broker forwarding query to historical
-    assertExpectedObjects(
+    TestHelper.assertExpectedObjects(
         expectedResults,
         factory.getToolchest().postMergeQueryDecoration(
             factory.getToolchest().mergeResults(
@@ -2581,7 +2565,7 @@ public ByteBuffer get()
         QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator()
     ).mergeResults(runner);
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
   }
 
   @Test
@@ -2639,7 +2623,7 @@ public void testGroupByWithMetricColumnDisappears() throws Exception
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(expectedResults, runner.run(query, context), "normal");
+    TestHelper.assertExpectedObjects(expectedResults, runner.run(query, context), "normal");
     final GroupByQueryEngine engine = new GroupByQueryEngine(
         configSupplier,
         new StupidPool<>(
@@ -2661,7 +2645,7 @@ public ByteBuffer get()
         TestQueryRunners.pool,
         QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator()
     ).mergeResults(runner);
-    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
   }
 
   @Test
@@ -2719,7 +2703,7 @@ public void testGroupByWithNonexistentDimension() throws Exception
     );
 
     Map<String, Object> context = Maps.newHashMap();
-    assertExpectedObjects(expectedResults, runner.run(query, context), "normal");
+    TestHelper.assertExpectedObjects(expectedResults, runner.run(query, context), "normal");
     final GroupByQueryEngine engine = new GroupByQueryEngine(
         configSupplier,
         new StupidPool<>(
@@ -2741,7 +2725,7 @@ public ByteBuffer get()
         TestQueryRunners.pool,
         QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator()
     ).mergeResults(runner);
-    assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
+    TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
   }
 
   // A subquery identical to the query should yield identical results
@@ -2806,7 +2790,7 @@ public void testIdenticalSubquery()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -2877,7 +2861,7 @@ public void testSubqueryWithMultipleIntervalsInOuterQuery()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -2977,7 +2961,7 @@ public void testDifferentGroupingSubquery()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3034,7 +3018,7 @@ public void testDifferentGroupingSubqueryMultipleAggregatorsOnSameField()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -3088,7 +3072,7 @@ public void testDifferentGroupingSubqueryWithFilter()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3125,7 +3109,7 @@ public void testDifferentIntervalSubquery()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3424,7 +3408,7 @@ public void testSubqueryWithPostAggregators()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3687,7 +3671,7 @@ public byte[] getCacheKey()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3858,7 +3842,7 @@ public byte[] getCacheKey()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -3998,7 +3982,7 @@ public void testSubqueryWithHyperUniques()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4060,7 +4044,7 @@ public void testSubqueryWithHyperUniquesPostAggregator()
 
     // Subqueries are handled by the ToolChest
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4093,7 +4077,7 @@ public void testGroupByWithTimeColumn()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4318,7 +4302,7 @@ public void testGroupByTimeExtraction()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4374,7 +4358,7 @@ public void testBySegmentResults()
         )
     );
 
-    assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
+    TestHelper.assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
     exec.shutdownNow();
   }
 
@@ -4450,7 +4434,7 @@ public void testBySegmentResultsUnOptimizedDimextraction()
         )
     );
 
-    assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
+    TestHelper.assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
     exec.shutdownNow();
   }
 
@@ -4525,7 +4509,7 @@ public void testBySegmentResultsOptimizedDimextraction()
         )
     );
 
-    assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
+    TestHelper.assertExpectedObjects(bySegmentResults, theRunner.run(fullQuery, Maps.newHashMap()), "");
     exec.shutdownNow();
   }
 
@@ -4594,7 +4578,7 @@ public void testGroupByWithExtractionDimFilter()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
 
   }
 
@@ -4641,7 +4625,7 @@ public void testGroupByWithExtractionDimFilterCaseMappingValueIsNullOrEmpty()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4679,7 +4663,7 @@ public void testGroupByWithExtractionDimFilterWhenSearchValueNotInTheMap()
     List<Row> expectedResults = Arrays.asList();
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
 
@@ -4724,7 +4708,7 @@ public void testGroupByWithExtractionDimFilterKeyisNull()
         );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
   }
 
   @Test
@@ -4790,7 +4774,7 @@ public void testGroupByWithAggregatorFilterAndExtractionFunction()
     );
 
     Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
-    assertExpectedObjects(expectedResults, results, "");
+    TestHelper.assertExpectedObjects(expectedResults, results, "");
 
   }
 
diff --git a/processing/src/test/java/io/druid/segment/TestHelper.java b/processing/src/test/java/io/druid/segment/TestHelper.java
index 9fa85a2e8ba4..d86ff55ad093 100644
--- a/processing/src/test/java/io/druid/segment/TestHelper.java
+++ b/processing/src/test/java/io/druid/segment/TestHelper.java
@@ -30,7 +30,6 @@
 import org.junit.Assert;
 
 import java.util.Iterator;
-import java.util.List;
 
 /**
  */
@@ -168,21 +167,6 @@ private static <T> void assertResults(
     }
   }
 
-  public static <T> void assertObjectsUnordered(Iterable<T> expectedResults, Iterable<T> actualResults, String failMsg)
-  {
-    int counter = 0;
-    List expected = Lists.newLinkedList(expectedResults);
-    for (Object t : actualResults) {
-      counter++;
-      if (!expected.remove(t)) {
-        Assert.fail(failMsg + ": " + counter + " th return value " + t + " was not in expected value list");
-      }
-    }
-    if (!expected.isEmpty()) {
-      Assert.fail(failMsg + ": Some expected values was not arrived something like " + expected.get(0));
-    }
-  }
-
   private static <T> void assertObjects(Iterable<T> expectedResults, Iterable<T> actualResults, String msg)
   {
     Iterator resultsIter = actualResults.iterator();
diff --git a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java
index caf353d3c0f4..ee140edc8400 100644
--- a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java
+++ b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java
@@ -135,7 +135,8 @@ public void testSanity() throws Exception
                     .addDimension("sally")
                     .addAggregator(new LongSumAggregatorFactory("cnt", "cnt"))
                     .build(),
-        new IncrementalIndexStorageAdapter(index)
+        new IncrementalIndexStorageAdapter(index),
+        true
     );
 
     final ArrayList<Row> results = Sequences.toList(rows, Lists.<Row>newArrayList());
@@ -194,7 +195,8 @@ public void testObjectColumnSelectorOnVaryingColumnSchema() throws Exception
                         )
                     )
                     .build(),
-        new IncrementalIndexStorageAdapter(index)
+        new IncrementalIndexStorageAdapter(index),
+        true
     );
 
     final ArrayList<Row> results = Sequences.toList(rows, Lists.<Row>newArrayList());
@@ -372,7 +374,8 @@ public void testFilterByNull() throws Exception
                     .addAggregator(new LongSumAggregatorFactory("cnt", "cnt"))
                     .setDimFilter(DimFilters.dimEquals("sally", (String) null))
                     .build(),
-        new IncrementalIndexStorageAdapter(index)
+        new IncrementalIndexStorageAdapter(index),
+        true
     );
 
     final ArrayList<Row> results = Sequences.toList(rows, Lists.<Row>newArrayList());

From 57643621662f4e9ca8d590d7ce2811f075f29a72 Mon Sep 17 00:00:00 2001
From: "navis.ryu" <navis@apache.org>
Date: Mon, 25 Apr 2016 10:00:30 +0900
Subject: [PATCH 3/3] remove sort param

---
 .../java/io/druid/query/groupby/GroupByQueryEngine.java  | 4 ++--
 .../druid/query/groupby/GroupByQueryQueryToolChest.java  | 3 +--
 .../druid/query/groupby/GroupByQueryRunnerFactory.java   | 2 +-
 .../io/druid/query/groupby/GroupByQueryRunnerTest.java   | 1 +
 .../incremental/IncrementalIndexStorageAdapterTest.java  | 9 +++------
 5 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
index 7efa4a6ed6fb..221794b91e8f 100644
--- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
+++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java
@@ -81,7 +81,7 @@ public GroupByQueryEngine(
     this.intermediateResultsBufferPool = intermediateResultsBufferPool;
   }
 
-  public Sequence<Row> process(final GroupByQuery query, final StorageAdapter storageAdapter, final boolean sort)
+  public Sequence<Row> process(final GroupByQuery query, final StorageAdapter storageAdapter)
   {
     if (storageAdapter == null) {
       throw new ISE(
@@ -118,7 +118,7 @@ public Sequence<Row> apply(final Cursor cursor)
                           @Override
                           public RowIterator make()
                           {
-                            return new RowIterator(query, cursor, bufferHolder.get(), config.get(), sort);
+                            return new RowIterator(query, cursor, bufferHolder.get(), config.get(), false);
                           }
 
                           @Override
diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java
index f205629d9aa2..a3c77245a832 100644
--- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java
+++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java
@@ -238,8 +238,7 @@ public Sequence<Row> apply(Interval interval)
                           outerQuery.withQuerySegmentSpec(
                               new MultipleIntervalSegmentSpec(ImmutableList.of(interval))
                           ),
-                          new IncrementalIndexStorageAdapter(innerQueryResultIndex),
-                          true
+                          new IncrementalIndexStorageAdapter(innerQueryResultIndex)
                       );
                     }
                   }
diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryRunnerFactory.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryRunnerFactory.java
index f2320a873e52..bdafd6fef646 100644
--- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryRunnerFactory.java
+++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryRunnerFactory.java
@@ -107,7 +107,7 @@ public Sequence<Row> run(Query<Row> input, Map<String, Object> responseContext)
         throw new ISE("Got a [%s] which isn't a %s", input.getClass(), GroupByQuery.class);
       }
 
-      return engine.process((GroupByQuery) input, adapter, false);
+      return engine.process((GroupByQuery) input, adapter);
     }
   }
 }
diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
index 6d9e5d9f33c5..42be3231b6b5 100644
--- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
+++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
@@ -108,6 +108,7 @@
 import javax.annotation.Nullable;
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
diff --git a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java
index ee140edc8400..caf353d3c0f4 100644
--- a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java
+++ b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java
@@ -135,8 +135,7 @@ public void testSanity() throws Exception
                     .addDimension("sally")
                     .addAggregator(new LongSumAggregatorFactory("cnt", "cnt"))
                     .build(),
-        new IncrementalIndexStorageAdapter(index),
-        true
+        new IncrementalIndexStorageAdapter(index)
     );
 
     final ArrayList<Row> results = Sequences.toList(rows, Lists.<Row>newArrayList());
@@ -195,8 +194,7 @@ public void testObjectColumnSelectorOnVaryingColumnSchema() throws Exception
                         )
                     )
                     .build(),
-        new IncrementalIndexStorageAdapter(index),
-        true
+        new IncrementalIndexStorageAdapter(index)
     );
 
     final ArrayList<Row> results = Sequences.toList(rows, Lists.<Row>newArrayList());
@@ -374,8 +372,7 @@ public void testFilterByNull() throws Exception
                     .addAggregator(new LongSumAggregatorFactory("cnt", "cnt"))
                     .setDimFilter(DimFilters.dimEquals("sally", (String) null))
                     .build(),
-        new IncrementalIndexStorageAdapter(index),
-        true
+        new IncrementalIndexStorageAdapter(index)
     );
 
     final ArrayList<Row> results = Sequences.toList(rows, Lists.<Row>newArrayList());