Fixes #45, strip sorts from weight check queries

CHANGELOG.md

@@ -26,6 +26,7 @@ Current
 
 ### Fixed:
 
+- [#45, removing sorting from weight check queries](https://github.com/yahoo/fili/pull/46)
 
 
 ### Known Issues:

fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/orderby/LimitSpec.java

@@ -45,7 +45,7 @@ public LimitSpec(LinkedHashSet<OrderByColumn> sortColumns, OptionalInt limit) {
      * @return type
      */
     public String getType() {
-        return this.type;
+        return type;
     }
 
     /**
@@ -55,7 +55,7 @@ public String getType() {
      */
     @JsonInclude(JsonInclude.Include.NON_ABSENT)
     public OptionalInt getLimit() {
-        return this.limit;
+        return limit;
     }
 
     /**
@@ -67,6 +67,16 @@ public LinkedHashSet<OrderByColumn> getColumns() {
         return this.columns;
     }
 
+    // CHECKSTYLE:OFF
+    public LimitSpec withColumns(LinkedHashSet<OrderByColumn> sortColumns) {
+        return new LimitSpec(sortColumns, limit);
+    }
+
+    public LimitSpec withLimit(OptionalInt limit) {
+        return new LimitSpec(columns, limit);
+    }
+    // CHECKSTYLE:ON
+
     @Override
     public boolean equals(final Object o) {
         if (this == o) { return true; }

fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/query/WeightEvaluationQuery.java

@@ -13,6 +13,7 @@
 import com.yahoo.bard.webservice.druid.model.datasource.UnionDataSource;
 import com.yahoo.bard.webservice.druid.model.filter.Filter;
 import com.yahoo.bard.webservice.druid.model.having.Having;
+import com.yahoo.bard.webservice.druid.model.orderby.LimitSpec;
 import com.yahoo.bard.webservice.druid.model.postaggregation.ConstantPostAggregation;
 import com.yahoo.bard.webservice.druid.model.postaggregation.PostAggregation;
 import com.yahoo.bard.webservice.util.IntervalUtils;
@@ -22,6 +23,7 @@
 import org.slf4j.LoggerFactory;
 
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.List;
 
 /**
@@ -101,7 +103,7 @@ public WeightEvaluationQuery(DruidAggregationQuery<?> query, int weight) {
                 Collections.<Aggregation>singletonList(new LongSumAggregation("count", "count")),
                 Collections.<PostAggregation>emptyList(),
                 query.getIntervals(),
-                query.getQueryType() == QueryType.GROUP_BY ? ((GroupByQuery) query).getLimitSpec() : null
+                query.getQueryType() == QueryType.GROUP_BY ? stripColumnsFromLimitSpec(query) : null
         );
     }
 
@@ -200,7 +202,7 @@ private static DataSource makeInnerQuery(DruidAggregationQuery<?> query, double
                         aggregations,
                         postAggregations,
                         innerQuery.getIntervals(),
-                        ((GroupByQuery) innerQuery).getLimitSpec()
+                        stripColumnsFromLimitSpec(innerQuery)
                 );
                 return new QueryDataSource(inner);
             case TOP_N:
@@ -221,4 +223,17 @@ private static DataSource makeInnerQuery(DruidAggregationQuery<?> query, double
                 return null;
         }
     }
+
+    /**
+     * Strip the columns from the LimitSpec on the query and return it, if present.
+     *
+     * @param query  Query to strip the columns from within the LimitSpec
+     *
+     * @return the cleaned LimitSpec if there is one
+     */
+    private static LimitSpec stripColumnsFromLimitSpec(DruidFactQuery query) {
+        return ((GroupByQuery) query).getLimitSpec() == null ?
+                null :
+                ((GroupByQuery) query).getLimitSpec().withColumns(new LinkedHashSet<>());
+    }
 }

fili-core/src/test/groovy/com/yahoo/bard/webservice/druid/model/query/WeightEvaluationQuerySpec.groovy

@@ -185,6 +185,38 @@ class WeightEvaluationQuerySpec extends Specification {
         WeightEvaluationQuery.getWorstCaseWeightEstimate(groupByQuery) == 2 * 4 * 8 * queryWeightLimit
     }
 
+    def "Weight check query strips sort columns"() {
+        given: "weekly day average in outer query, use inner daily timegrain"
+        final DataApiRequest apiRequest = new DataApiRequest(
+                "shapes",
+                "week",
+                [size, shape, color, other],
+                "users,otherUsers",
+                "2014-09-01/2014-09-29",
+                null, //filters
+                null, //havings
+                "users", //sorts
+                null, //counts
+                null, //topN
+                null, //format
+                null, //timeZoneId
+                null, //asyncAfter
+                "", //perPage
+                "", //page
+                null, //uriInfo
+                dataServlet
+        )
+
+        GroupByQuery groupByQuery = builder.buildQuery(apiRequest, merger.merge(apiRequest))
+
+        expect: "The groupBy query has a sort, as a pre-condition"
+        groupByQuery.limitSpec.columns
+
+        and: "The weight-check query doesn't have a sort at either level"
+        !new WeightEvaluationQuery(groupByQuery, 1).limitSpec.columns
+        !new WeightEvaluationQuery(groupByQuery, 1).innermostQuery.limitSpec.columns
+    }
+
     @Unroll
     def "Worst estimate for big dimensions works if estimate is larger than an int but not a long for #queryType"() {
         given: "A query with one sketch, a small number of time buckets, and dimensions with very large cardinality"