Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for sorting terms aggregation by ascending count #18940

Merged
merged 1 commit into from
Jun 17, 2016
Merged

Remove support for sorting terms aggregation by ascending count #18940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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