Skip to content

Commit

Permalink
Merge pull request #18940 from jimferenczi/terms_count_asc
Browse files Browse the repository at this point in the history 
Remove support for sorting terms aggregation by ascending count
  • Loading branch information
@jimczi
jimczi authored Jun 17, 2016
2 parents 712e387 + 7557219 commit cea5f16
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 194 deletions.
19 changes: 3 additions & 16 deletions core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,8 @@
class InternalOrder extends Terms.Order {

private static final byte COUNT_DESC_ID = 1;
private static final byte COUNT_ASC_ID = 2;
private static final byte TERM_DESC_ID = 3;
private static final byte TERM_ASC_ID = 4;
private static final byte TERM_DESC_ID = 2;
private static final byte TERM_ASC_ID = 3;

/**
* Order by the (higher) count of each term.
Expand All @@ -60,17 +59,6 @@ public int compare(Terms.Bucket o1, Terms.Bucket o2) {
}
});

/**
* Order by the (lower) count of each term.
*/
public static final InternalOrder COUNT_ASC = new InternalOrder(COUNT_ASC_ID, "_count", true, new Comparator<Terms.Bucket>() {

@Override
public int compare(Terms.Bucket o1, Terms.Bucket o2) {
return Long.compare(o1.getDocCount(), o2.getDocCount());
}
});

/**
* Order by the terms.
*/
Expand All @@ -93,7 +81,7 @@ public int compare(Terms.Bucket o1, Terms.Bucket o2) {
}
});

public static boolean isCountDesc(Terms.Order order) {
public static boolean isCountOrder(Terms.Order order) {
if (order == COUNT_DESC) {
return true;
} else if (order instanceof CompoundOrder) {
Expand Down Expand Up @@ -348,7 +336,6 @@ public static Terms.Order readOrder(StreamInput in, boolean absoluteOrder) throw
byte id = in.readByte();
switch (id) {
case COUNT_DESC_ID: return absoluteOrder ? new CompoundOrder(Collections.singletonList((Terms.Order) InternalOrder.COUNT_DESC)) : InternalOrder.COUNT_DESC;
case COUNT_ASC_ID: return absoluteOrder ? new CompoundOrder(Collections.singletonList((Terms.Order) InternalOrder.COUNT_ASC)) : InternalOrder.COUNT_ASC;
case TERM_DESC_ID: return InternalOrder.TERM_DESC;
case TERM_ASC_ID: return InternalOrder.TERM_ASC;
case Aggregation.ID:
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
final long thisAggDocCountError;
if (terms.buckets.size() < this.shardSize || InternalOrder.isTermOrder(order)) {
thisAggDocCountError = 0;
} else if (InternalOrder.isCountDesc(this.order)) {
} else if (InternalOrder.isCountOrder(this.order)) {
thisAggDocCountError = terms.buckets.get(terms.buckets.size() - 1).docCount;
} else {
thisAggDocCountError = -1;
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ static abstract class Bucket extends InternalMultiBucketAggregation.InternalBuck
static abstract class Order implements ToXContent {

/**
* @return a bucket ordering strategy that sorts buckets by their document counts (ascending or descending)
* @return a bucket ordering strategy that sorts buckets by their document counts (descending)
*/
public static Order count(boolean asc) {
return asc ? InternalOrder.COUNT_ASC : InternalOrder.COUNT_DESC;
public static Order count() {
return InternalOrder.COUNT_DESC;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion ...main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder<Valu
public static final ParseField SHOW_TERM_DOC_COUNT_ERROR = new ParseField("show_term_doc_count_error");
public static final ParseField ORDER_FIELD = new ParseField("order");

private Terms.Order order = Terms.Order.compound(Terms.Order.count(false), Terms.Order.term(true));
private Terms.Order order = Terms.Order.compound(Terms.Order.count(), Terms.Order.term(true));
private IncludeExclude includeExclude = null;
private String executionHint = null;
private SubAggCollectionMode collectMode = null;
Expand Down
20 changes: 17 additions & 3 deletions core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,19 @@ protected TermsAggregationBuilder doCreateFactory(String aggregationName, Values
@Override
public boolean parseSpecial(String aggregationName, XContentParser parser, ParseFieldMatcher parseFieldMatcher, Token token,
String currentFieldName, Map<ParseField, Object> otherOptions) throws IOException {
if (token == XContentParser.Token.START_OBJECT) {
if (token == Token.VALUE_STRING) {
if (parseFieldMatcher.match(currentFieldName, TermsAggregationBuilder.ORDER_FIELD)) {
otherOptions.put(TermsAggregationBuilder.ORDER_FIELD, Collections.singletonList(parseOrderParam(aggregationName, parser)));
if ("_count".equals(parser.text())) {
otherOptions.put(TermsAggregationBuilder.ORDER_FIELD,
Collections.singletonList(new OrderElement(parser.text(), false)));
return true;
}
}
return false;
} else if (token == XContentParser.Token.START_OBJECT) {
if (parseFieldMatcher.match(currentFieldName, TermsAggregationBuilder.ORDER_FIELD)) {
otherOptions.put(TermsAggregationBuilder.ORDER_FIELD,
Collections.singletonList(parseOrderParam(aggregationName, parser)));
return true;
}
} else if (token == XContentParser.Token.START_ARRAY) {
Expand Down Expand Up @@ -117,6 +127,10 @@ private OrderElement parseOrderParam(String aggregationName, XContentParser pars
} else if (token == XContentParser.Token.VALUE_STRING) {
String dir = parser.text();
if ("asc".equalsIgnoreCase(dir)) {
if ("_count".equals(orderKey)) {
throw new ParsingException(parser.getTokenLocation(),
"Sort by ascending _count is not supported in [" + aggregationName + "].");
}
orderAsc = true;
} else if ("desc".equalsIgnoreCase(dir)) {
orderAsc = false;
Expand Down Expand Up @@ -167,7 +181,7 @@ static Terms.Order resolveOrder(String key, boolean asc) {
return Order.term(asc);
}
if ("_count".equals(key)) {
return Order.count(asc);
return Order.count();
}
return Order.aggregation(key, asc);
}
Expand Down
24 changes: 12 additions & 12 deletions core/src/test/java/org/elasticsearch/search/aggregations/bucket/ShardSizeTermsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void testNoShardSizeString() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -62,7 +62,7 @@ public void testShardSizeEqualsSizeString() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3).shardSize(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -86,7 +86,7 @@ public void testWithShardSizeString() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -110,7 +110,7 @@ public void testWithShardSizeStringSingleShard() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type").setRouting(routing1)
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand Down Expand Up @@ -156,7 +156,7 @@ public void testNoShardSizeLong() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -179,7 +179,7 @@ public void testShardSizeEqualsSizeLong() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3).shardSize(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -202,7 +202,7 @@ public void testWithShardSizeLong() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -226,7 +226,7 @@ public void testWithShardSizeLongSingleShard() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type").setRouting(routing1)
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand Down Expand Up @@ -272,7 +272,7 @@ public void testNoShardSizeDouble() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -295,7 +295,7 @@ public void testShardSizeEqualsSizeDouble() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3).shardSize(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -318,7 +318,7 @@ public void testWithShardSizeDouble() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand All @@ -341,7 +341,7 @@ public void testWithShardSizeDoubleSingleShard() throws Exception {
SearchResponse response = client().prepareSearch("idx").setTypes("type").setRouting(routing1)
.setQuery(matchAllQuery())
.addAggregation(terms("keys").field("key").size(3)
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count(false)))
.collectMode(randomFrom(SubAggCollectionMode.values())).shardSize(5).order(Terms.Order.count()))
.execute().actionGet();

Terms terms = response.getAggregations().get("keys");
Expand Down
93 changes: 0 additions & 93 deletions core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,37 +271,6 @@ public void testStringValueFieldWithRouting() throws Exception {
assertNoDocCountErrorSingleResponse(size, testResponse);
}

public void testStringValueFieldDocCountAsc() throws Exception {
int size = randomIntBetween(1, 20);
int shardSize = randomIntBetween(size, size * 2);
SearchResponse accurateResponse = client().prepareSearch("idx_single_shard").setTypes("type")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(STRING_FIELD_NAME)
.showTermDocCountError(true)
.size(10000).shardSize(10000)
.order(Order.count(true))
.collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet();

assertSearchResponse(accurateResponse);

SearchResponse testResponse = client().prepareSearch("idx_single_shard").setTypes("type")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(STRING_FIELD_NAME)
.showTermDocCountError(true)
.size(size)
.shardSize(shardSize)
.order(Order.count(true))
.collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet();

assertSearchResponse(testResponse);

assertUnboundedDocCountError(size, accurateResponse, testResponse);
}

public void testStringValueFieldTermSortAsc() throws Exception {
int size = randomIntBetween(1, 20);
int shardSize = randomIntBetween(size, size * 2);
Expand Down Expand Up @@ -507,37 +476,6 @@ public void testLongValueFieldWithRouting() throws Exception {
assertNoDocCountErrorSingleResponse(size, testResponse);
}

public void testLongValueFieldDocCountAsc() throws Exception {
int size = randomIntBetween(1, 20);
int shardSize = randomIntBetween(size, size * 2);
SearchResponse accurateResponse = client().prepareSearch("idx_single_shard").setTypes("type")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(LONG_FIELD_NAME)
.showTermDocCountError(true)
.size(10000).shardSize(10000)
.order(Order.count(true))
.collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet();

assertSearchResponse(accurateResponse);

SearchResponse testResponse = client().prepareSearch("idx_single_shard").setTypes("type")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(LONG_FIELD_NAME)
.showTermDocCountError(true)
.size(size)
.shardSize(shardSize)
.order(Order.count(true))
.collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet();

assertSearchResponse(testResponse);

assertUnboundedDocCountError(size, accurateResponse, testResponse);
}

public void testLongValueFieldTermSortAsc() throws Exception {
int size = randomIntBetween(1, 20);
int shardSize = randomIntBetween(size, size * 2);
Expand Down Expand Up @@ -743,37 +681,6 @@ public void testDoubleValueFieldWithRouting() throws Exception {
assertNoDocCountErrorSingleResponse(size, testResponse);
}

public void testDoubleValueFieldDocCountAsc() throws Exception {
int size = randomIntBetween(1, 20);
int shardSize = randomIntBetween(size, size * 2);
SearchResponse accurateResponse = client().prepareSearch("idx_single_shard").setTypes("type")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(DOUBLE_FIELD_NAME)
.showTermDocCountError(true)
.size(10000).shardSize(10000)
.order(Order.count(true))
.collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet();

assertSearchResponse(accurateResponse);

SearchResponse testResponse = client().prepareSearch("idx_single_shard").setTypes("type")
.addAggregation(terms("terms")
.executionHint(randomExecutionHint())
.field(DOUBLE_FIELD_NAME)
.showTermDocCountError(true)
.size(size)
.shardSize(shardSize)
.order(Order.count(true))
.collectMode(randomFrom(SubAggCollectionMode.values())))
.execute().actionGet();

assertSearchResponse(testResponse);

assertUnboundedDocCountError(size, accurateResponse, testResponse);
}

public void testDoubleValueFieldTermSortAsc() throws Exception {
int size = randomIntBetween(1, 20);
int shardSize = randomIntBetween(size, size * 2);
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private List<Terms.Order> randomOrder() {
orders.add(Terms.Order.term(randomBoolean()));
break;
case 1:
orders.add(Terms.Order.count(randomBoolean()));
orders.add(Terms.Order.count());
break;
case 2:
orders.add(Terms.Order.aggregation(randomAsciiOfLengthBetween(3, 20), randomBoolean()));
Expand Down
Loading

0 comments on commit cea5f16

Please sign in to comment.