Skip to content

Commit

Permalink
Fixes #45, strip sorts from weight check queries
Browse files Browse the repository at this point in the history
  • Loading branch information
cdeszaq committed Sep 22, 2016
1 parent b2f91f3 commit cad78fe
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Current

### Fixed:

- [#45, removing sorting from weight check queries](https://github.com/yahoo/fili/pull/46)


### Known Issues:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ public LinkedHashSet<OrderByColumn> getColumns() {
return this.columns;
}

// CHECKSTYLE:OFF
public LimitSpec withColumns(LinkedHashSet<OrderByColumn> sortColumns) {
return new LimitSpec(sortColumns, this.limit);
}

public LimitSpec withLimit(OptionalInt limit) {
return new LimitSpec(this.columns, limit);
}
// CHECKSTYLE:ON

@Override
public boolean equals(final Object o) {
if (this == o) { return true; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.slf4j.LoggerFactory;

import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;

/**
Expand Down Expand Up @@ -101,7 +102,11 @@ 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 ?
null :
((GroupByQuery) query).getLimitSpec() == null ?
null :
((GroupByQuery) query).getLimitSpec().withColumns(new LinkedHashSet<>())
);
}

Expand Down Expand Up @@ -200,7 +205,9 @@ private static DataSource makeInnerQuery(DruidAggregationQuery<?> query, double
aggregations,
postAggregations,
innerQuery.getIntervals(),
((GroupByQuery) innerQuery).getLimitSpec()
((GroupByQuery) innerQuery).getLimitSpec() == null ?
null :
((GroupByQuery) innerQuery).getLimitSpec().withColumns(new LinkedHashSet<>())
);
return new QueryDataSource(inner);
case TOP_N:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.empty

and: "The weight-check query doesn't have a sort at either level"
new WeightEvaluationQuery(groupByQuery, 1).limitSpec.columns.empty
new WeightEvaluationQuery(groupByQuery, 1).innermostQuery.limitSpec.columns.empty
}

@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"
Expand Down

0 comments on commit cad78fe

Please sign in to comment.