Merge pull request #790 from metamx/fix-groupby-order-casing

fix casing issue when specifying ordering dimension in groupBy

histogram/src/test/java/io/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java

@@ -30,12 +30,9 @@ import io.druid.data.input.Row;
 import io.druid.jackson.DefaultObjectMapper;
 import io.druid.query.QueryRunner;
 import io.druid.query.QueryRunnerTestHelper;
-import io.druid.query.aggregation.AggregatorFactory;
-import io.druid.query.aggregation.MaxAggregatorFactory;
-import io.druid.query.aggregation.MinAggregatorFactory;
 import io.druid.query.aggregation.PostAggregator;
+import io.druid.query.dimension.DefaultDimensionSpec;
 import io.druid.query.dimension.DimensionSpec;
-import io.druid.query.dimension.LegacyDimensionSpec;
 import io.druid.query.groupby.GroupByQuery;
 import io.druid.query.groupby.GroupByQueryConfig;
 import io.druid.query.groupby.GroupByQueryEngine;
@@ -45,7 +42,6 @@ import io.druid.query.groupby.GroupByQueryRunnerTestHelper;
 import io.druid.query.groupby.orderby.DefaultLimitSpec;
 import io.druid.query.groupby.orderby.OrderByColumnSpec;
 import io.druid.segment.TestHelper;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -64,13 +60,6 @@ public class ApproximateHistogramGroupByQueryTest
 {
   private final QueryRunner<Row> runner;
   private GroupByQueryRunnerFactory factory;
-  private Supplier<GroupByQueryConfig> configSupplier;
-
-  @Before
-  public void setUp() throws Exception
-  {
-    configSupplier = Suppliers.ofInstance(new GroupByQueryConfig());
-  }
 
   @Parameterized.Parameters
   public static Collection<?> constructorFeeder() throws IOException
@@ -165,13 +154,20 @@ public class ApproximateHistogramGroupByQueryTest
     GroupByQuery query = new GroupByQuery.Builder()
         .setDataSource(QueryRunnerTestHelper.dataSource)
         .setGranularity(QueryRunnerTestHelper.allGran)
-        .setDimensions(Arrays.<DimensionSpec>asList(new LegacyDimensionSpec(QueryRunnerTestHelper.providerDimension)))
+        .setDimensions(
+            Arrays.<DimensionSpec>asList(
+                new DefaultDimensionSpec(
+                    QueryRunnerTestHelper.providerDimension,
+                    "proViderAlias"
+                )
+            )
+        )
         .setInterval(QueryRunnerTestHelper.fullOnInterval)
         .setLimitSpec(
             new DefaultLimitSpec(
                 Lists.newArrayList(
                     new OrderByColumnSpec(
-                        QueryRunnerTestHelper.providerDimension,
+                        "proViderAlias",
                         OrderByColumnSpec.Direction.DESCENDING
                     )
                 ), 1
@@ -179,20 +175,12 @@ public class ApproximateHistogramGroupByQueryTest
         )
         .setAggregatorSpecs(
             Lists.newArrayList(
-                Iterables.concat(
-                    QueryRunnerTestHelper.commonAggregators,
-                    Lists.newArrayList(
-                        new MaxAggregatorFactory("maxIndex", "index"),
-                        new MinAggregatorFactory("minIndex", "index"),
-                        aggFactory
-                    )
-                )
+                QueryRunnerTestHelper.rowsCount,
+                aggFactory
             )
         )
         .setPostAggregatorSpecs(
-            Arrays.asList(
-                QueryRunnerTestHelper.addRowsIndexConstant,
-                QueryRunnerTestHelper.dependentPostAgg,
+            Arrays.<PostAggregator>asList(
                 new QuantilePostAggregator("quantile", "apphisto", 0.5f)
             )
         )
@@ -201,31 +189,21 @@ public class ApproximateHistogramGroupByQueryTest
     List<Row> expectedResults = Arrays.asList(
         GroupByQueryRunnerTestHelper.createExpectedRow(
             "1970-01-01T00:00:00.000Z",
-            "provider", "spot",
-            "rows", 837L,
-            "addRowsIndexConstant", 96444.5703125,
-            "dependentPostAgg", 97282.5703125,
-            "index", 95606.5703125,
-            "maxIndex", 277.2735290527344,
-            "minIndex", 59.02102279663086,
-            "quantile", 101.78856f,
-            "uniques", QueryRunnerTestHelper.UNIQUES_9,
+            "provideralias", "upfront",
+            "rows", 186L,
+            "quantile", 880.9881f,
             "apphisto",
             new Histogram(
                 new float[]{
-                    4.457897186279297f,
-                    59.02102279663086f,
-                    113.58415222167969f,
-                    168.14727783203125f,
-                    222.7104034423828f,
-                    277.2735290527344f
+                    214.97299194335938f,
+                    545.9906005859375f,
+                    877.0081787109375f,
+                    1208.0257568359375f,
+                    1539.0433349609375f,
+                    1870.06103515625f
                 },
                 new double[]{
-                    0.0,
-                    462.4309997558594,
-                    357.5404968261719,
-                    15.022850036621094,
-                    2.0056631565093994
+                    0.0, 67.53287506103516, 72.22068786621094, 31.984678268432617, 14.261756896972656
                 }
             )
         )

processing/src/main/java/io/druid/query/groupby/orderby/DefaultLimitSpec.java

@@ -173,7 +173,7 @@ public class DefaultLimitSpec implements LimitSpec
                          public String apply(Row input)
                          {
                            // Multi-value dimensions have all been flattened at this point;
-                           final List<String> dimList = input.getDimension(dimension);
+                           final List<String> dimList = input.getDimension(dimension.toLowerCase());
                            return dimList.isEmpty() ? null : dimList.get(0);
                          }
                        }

processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java

@@ -733,6 +733,48 @@ public class GroupByQueryRunnerTest
     );
   }
 
+  @Test
+  public void testGroupByWithMixedCasingOrdering()
+  {
+    GroupByQuery query = new GroupByQuery.Builder()
+        .setDataSource(QueryRunnerTestHelper.dataSource)
+        .setGranularity(QueryRunnerTestHelper.allGran)
+        .setDimensions(
+            Arrays.<DimensionSpec>asList(
+                new DefaultDimensionSpec(
+                    QueryRunnerTestHelper.providerDimension,
+                    "ProviderAlias"
+                )
+            )
+        )
+        .setInterval(QueryRunnerTestHelper.fullOnInterval)
+        .setLimitSpec(
+            new DefaultLimitSpec(
+                Lists.newArrayList(
+                    new OrderByColumnSpec(
+                        "providerALIAS",
+                        OrderByColumnSpec.Direction.DESCENDING
+                    )
+                ), 3
+            )
+        )
+        .setAggregatorSpecs(
+            Lists.<AggregatorFactory>newArrayList(
+                QueryRunnerTestHelper.rowsCount
+            )
+        )
+        .build();
+
+    List<Row> expectedResults = Arrays.asList(
+        GroupByQueryRunnerTestHelper.createExpectedRow("1970-01-01T00:00:00.000Z", "provideralias", "upfront", "rows", 186L),
+        GroupByQueryRunnerTestHelper.createExpectedRow("1970-01-01T00:00:00.000Z", "provideralias", "total_market", "rows", 186L),
+        GroupByQueryRunnerTestHelper.createExpectedRow("1970-01-01T00:00:00.000Z", "provideralias", "spot", "rows", 837L)
+    );
+
+    Iterable<Row> results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query);
+    TestHelper.assertExpectedObjects(expectedResults, results, "order-limit");
+  }
+
   @Test
   public void testHavingSpec()
   {