Skip to content

Commit 33f563f

Browse files
committed
fixed a bug where when executing on a single shard and sorting terms agg based on sub metric agg, the order (asc/desc) is not respected
- fixed tests for terms order Closes #4951
1 parent 8452ee7 commit 33f563f

File tree

2 files changed

+18
-14
lines changed

2 files changed

+18
-14
lines changed

src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ public int compare(Terms.Bucket o1, Terms.Bucket o2) {
215215
if (v1 == Double.NaN) {
216216
return asc ? 1 : -1;
217217
}
218-
return Double.compare(v1, v2);
218+
return asc ? Double.compare(v1, v2) : Double.compare(v2, v1);
219219
}
220220
};
221221
}
@@ -230,7 +230,7 @@ public int compare(Terms.Bucket o1, Terms.Bucket o2) {
230230
if (v1 == Double.NaN) {
231231
return asc ? 1 : -1;
232232
}
233-
return Double.compare(v1, v2);
233+
return asc ? Double.compare(v1, v2) : Double.compare(v2, v1);
234234
}
235235
};
236236
}

src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -800,14 +800,15 @@ public void singleValuedField_OrderedBySingleValueSubAggregationAsc() throws Exc
800800
assertThat(terms.getName(), equalTo("terms"));
801801
assertThat(terms.getBuckets().size(), equalTo(5));
802802

803-
for (int i = 0; i < 5; i++) {
804-
Terms.Bucket bucket = terms.getBucketByKey("val" + i);
803+
int i = 0;
804+
for (Terms.Bucket bucket : terms.getBuckets()) {
805805
assertThat(bucket, notNullValue());
806806
assertThat(key(bucket), equalTo("val" + i));
807807
assertThat(bucket.getDocCount(), equalTo(1l));
808808
Avg avg = bucket.getAggregations().get("avg_i");
809809
assertThat(avg, notNullValue());
810810
assertThat(avg.getValue(), equalTo((double) i));
811+
i++;
811812
}
812813
}
813814

@@ -911,16 +912,16 @@ public void singleValuedField_OrderedBySingleValueSubAggregationDesc() throws Ex
911912
assertThat(terms.getName(), equalTo("terms"));
912913
assertThat(terms.getBuckets().size(), equalTo(5));
913914

914-
for (int i = 4; i >= 0; i--) {
915-
916-
Terms.Bucket bucket = terms.getBucketByKey("val" + i);
915+
int i = 4;
916+
for (Terms.Bucket bucket : terms.getBuckets()) {
917917
assertThat(bucket, notNullValue());
918918
assertThat(key(bucket), equalTo("val" + i));
919919
assertThat(bucket.getDocCount(), equalTo(1l));
920920

921921
Avg avg = bucket.getAggregations().get("avg_i");
922922
assertThat(avg, notNullValue());
923923
assertThat(avg.getValue(), equalTo((double) i));
924+
i--;
924925
}
925926

926927
}
@@ -943,15 +944,16 @@ public void singleValuedField_OrderedByMultiValueSubAggregationAsc() throws Exce
943944
assertThat(terms.getName(), equalTo("terms"));
944945
assertThat(terms.getBuckets().size(), equalTo(5));
945946

946-
for (int i = 0; i < 5; i++) {
947-
Terms.Bucket bucket = terms.getBucketByKey("val" + i);
947+
int i = 0;
948+
for (Terms.Bucket bucket : terms.getBuckets()) {
948949
assertThat(bucket, notNullValue());
949950
assertThat(key(bucket), equalTo("val" + i));
950951
assertThat(bucket.getDocCount(), equalTo(1l));
951952

952953
Stats stats = bucket.getAggregations().get("stats");
953954
assertThat(stats, notNullValue());
954955
assertThat(stats.getMax(), equalTo((double) i));
956+
i++;
955957
}
956958

957959
}
@@ -974,15 +976,16 @@ public void singleValuedField_OrderedByMultiValueSubAggregationDesc() throws Exc
974976
assertThat(terms.getName(), equalTo("terms"));
975977
assertThat(terms.getBuckets().size(), equalTo(5));
976978

977-
for (int i = 4; i >= 0; i--) {
978-
Terms.Bucket bucket = terms.getBucketByKey("val" + i);
979+
int i = 4;
980+
for (Terms.Bucket bucket : terms.getBuckets()) {
979981
assertThat(bucket, notNullValue());
980982
assertThat(key(bucket), equalTo("val" + i));
981983
assertThat(bucket.getDocCount(), equalTo(1l));
982984

983985
Stats stats = bucket.getAggregations().get("stats");
984986
assertThat(stats, notNullValue());
985987
assertThat(stats.getMax(), equalTo((double) i));
988+
i--;
986989
}
987990

988991
}
@@ -994,7 +997,7 @@ public void singleValuedField_OrderedByMultiValueExtendedStatsAsc() throws Excep
994997
.addAggregation(terms("terms")
995998
.executionHint(randomExecutionHint())
996999
.field(SINGLE_VALUED_FIELD_NAME)
997-
.order(Terms.Order.aggregation("stats.variance", asc))
1000+
.order(Terms.Order.aggregation("stats.sum_of_squares", asc))
9981001
.subAggregation(extendedStats("stats").field("i"))
9991002
).execute().actionGet();
10001003

@@ -1005,15 +1008,16 @@ public void singleValuedField_OrderedByMultiValueExtendedStatsAsc() throws Excep
10051008
assertThat(terms.getName(), equalTo("terms"));
10061009
assertThat(terms.getBuckets().size(), equalTo(5));
10071010

1008-
for (int i = 0; i < 5; i++) {
1009-
Terms.Bucket bucket = terms.getBucketByKey("val" + i);
1011+
int i = 0;
1012+
for (Terms.Bucket bucket : terms.getBuckets()) {
10101013
assertThat(bucket, notNullValue());
10111014
assertThat(key(bucket), equalTo("val" + i));
10121015
assertThat(bucket.getDocCount(), equalTo(1l));
10131016

10141017
ExtendedStats stats = bucket.getAggregations().get("stats");
10151018
assertThat(stats, notNullValue());
10161019
assertThat(stats.getMax(), equalTo((double) i));
1020+
i++;
10171021
}
10181022

10191023
}

0 commit comments

Comments
 (0)