groupBy v2: Always merge queries. (#4023)

This fixes #4020 because it means the timestamp will always be included for outermost
queries. Historicals receiving queries from older brokers will think they're
outermost (because CTX_KEY_OUTERMOST isn't set to "false"), so they'll include a
timestamp, so the older brokers will be OK.

processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java

@@ -108,9 +108,10 @@ public class GroupByQueryQueryToolChest extends QueryToolChest<Row, GroupByQuery
           return runner.run(query, responseContext);
         }
 
-        if (query.getContextBoolean(GROUP_BY_MERGE_KEY, true)) {
+        final GroupByQuery groupByQuery = (GroupByQuery) query;
+        if (strategySelector.strategize(groupByQuery).doMergeResults(groupByQuery)) {
           return initAndMergeGroupByResults(
-              (GroupByQuery) query,
+              groupByQuery,
               runner,
               responseContext
           );
@@ -181,7 +182,7 @@ public class GroupByQueryQueryToolChest extends QueryToolChest<Row, GroupByQuery
           subquery.withOverriddenContext(
               ImmutableMap.<String, Object>of(
                   //setting sort to false avoids unnecessary sorting while merging results. we only need to sort
-                  //in the end when returning results to user.
+                  //in the end when returning results to user. (note this is only respected by groupBy v1)
                   GroupByQueryHelper.CTX_KEY_SORT_RESULTS,
                   false
               )

processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategy.java

@@ -53,6 +53,11 @@ public interface GroupByStrategy
    */
   boolean isCacheable(boolean willMergeRunners);
 
+  /**
+   * Indicates if this query should undergo "mergeResults" or not.
+   */
+  boolean doMergeResults(final GroupByQuery query);
+
   /**
    * Decorate a runner with an interval chunking decorator.
    */

processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV1.java

@@ -101,6 +101,12 @@ public class GroupByStrategyV1 implements GroupByStrategy
     return decorator.decorate(runner, toolChest);
   }
 
+  @Override
+  public boolean doMergeResults(final GroupByQuery query)
+  {
+    return query.getContextBoolean(GroupByQueryQueryToolChest.GROUP_BY_MERGE_KEY, true);
+  }
+
   @Override
   public Sequence<Row> mergeResults(
       final QueryRunner<Row> baseRunner,
@@ -131,10 +137,10 @@ public class GroupByStrategyV1 implements GroupByStrategy
                 ImmutableMap.<String, Object>of(
                     "finalize", false,
                     //setting sort to false avoids unnecessary sorting while merging results. we only need to sort
-                    //in the end when returning results to user.
+                    //in the end when returning results to user. (note this is only respected by groupBy v1)
                     GroupByQueryHelper.CTX_KEY_SORT_RESULTS, false,
                     //no merging needed at historicals because GroupByQueryRunnerFactory.mergeRunners(..) would return
-                    //merged results
+                    //merged results. (note this is only respected by groupBy v1)
                     GroupByQueryQueryToolChest.GROUP_BY_MERGE_KEY, false,
                     GroupByQueryConfig.CTX_KEY_STRATEGY, GroupByStrategySelector.STRATEGY_V1
                 )

processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV2.java

@@ -179,6 +179,12 @@ public class GroupByStrategyV2 implements GroupByStrategy
     return willMergeRunners;
   }
 
+  @Override
+  public boolean doMergeResults(final GroupByQuery query)
+  {
+    return true;
+  }
+
   @Override
   public QueryRunner<Row> createIntervalChunkingRunner(
       final IntervalChunkingQueryRunnerDecorator decorator,