don't recheck distribution requirement in DistributedGroupByConsumer

In some cases if shards are still initializing the requiresDistribution() method might change its result between two calls. This can lead to the NonDistributedConsumer believing that distribution is required and the DistributedConsumer believing that it isn't. To fix that just check once in the NonDistributedConsumer and rely on the order of the Consumers. In the worst case this will execute a query that could be non-distributed in a distributed way.
crate · Feb 3, 2015 · ff1a38f · ff1a38f
1 parent 7aeed45
commit ff1a38f
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 5 deletions.
diff --git a/sql/src/main/java/io/crate/planner/consumer/DistributedGroupByConsumer.java b/sql/src/main/java/io/crate/planner/consumer/DistributedGroupByConsumer.java
@@ -103,10 +103,6 @@ public AnalyzedRelation visitQueriedTable(QueriedTable table, Context context) {
 
             Routing routing = tableInfo.getRouting(whereClause, null);
 
-            if (!GroupByConsumer.requiresDistribution(tableInfo, routing)) {
-                return table;
-            }
-
             GroupByConsumer.validateGroupBySymbols(table.tableRelation(), table.querySpec().groupBy());
             PlannerContextBuilder contextBuilder = new PlannerContextBuilder(2,
                     table.querySpec().groupBy())

diff --git a/sql/src/main/java/io/crate/planner/consumer/GlobalAggregateConsumer.java b/sql/src/main/java/io/crate/planner/consumer/GlobalAggregateConsumer.java
@@ -103,10 +103,15 @@ protected PlannedAnalyzedRelation visitAnalyzedRelation(AnalyzedRelation relatio
         }
     }
 
+    private static boolean noGroupBy(List<Symbol> groupBy) {
+        return groupBy == null || groupBy.isEmpty();
+    }
+
     public static PlannedAnalyzedRelation globalAggregates(QueriedTable table,
                                                            TableRelation tableRelation,
                                                            WhereClauseContext whereClauseContext,
                                                            ColumnIndexWriterProjection indexWriterProjection){
+        assert noGroupBy(table.querySpec().groupBy()) : "must not have group by clause for global aggregate queries";
         validateAggregationOutputs(tableRelation, table.querySpec().outputs());
         // global aggregate: collect and partial aggregate on C and final agg on H
         PlannerContextBuilder contextBuilder = new PlannerContextBuilder(2).output(

diff --git a/sql/src/test/java/io/crate/integrationtests/Setup.java b/sql/src/test/java/io/crate/integrationtests/Setup.java
@@ -148,7 +148,7 @@ public void groupBySetup(String numericType) throws Exception {
             " details object," +
             " details_ignored object(ignored)" +
             ")", numericType));
-        transportExecutor.ensureGreen();
+        transportExecutor.ensureYellow();
 
         Map<String, String> details = newHashMap();
         details.put("job", "Sandwitch Maker");