Skip to content

Commit 71aad7c

Browse files
jpountzjpountz
committed
Aggregations: clean-up comparisons.
1 parent ca45424 commit 71aad7c

File tree

11 files changed

+79
-74
lines changed

11 files changed

+79
-74
lines changed

‎src/main/java/org/elasticsearch/common/util/CollectionUtils.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,6 @@
2828
public enum CollectionUtils {
2929
;
3030

31-
private static int compare(long i, long j) {
32-
return i < j ? -1 : (i == j ? 0 : 1);
33-
}
34-
3531
public static void sort(LongArrayList list) {
3632
sort(list.buffer, list.size());
3733
}
@@ -50,7 +46,7 @@ protected void swap(int i, int j) {
5046

5147
@Override
5248
protected int compare(int i, int j) {
53-
return CollectionUtils.compare(array[i], array[j]);
49+
return Comparators.compare(array[i], array[j]);
5450
}
5551

5652
@Override
@@ -60,7 +56,7 @@ protected void setPivot(int i) {
6056

6157
@Override
6258
protected int comparePivot(int j) {
63-
return CollectionUtils.compare(pivot, array[j]);
59+
return Comparators.compare(pivot, array[j]);
6460
}
6561

6662
}.sort(0, len);

‎src/main/java/org/elasticsearch/common/util/Comparators.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.util;
21+
22+
/** Comparison utility methods. */
23+
public enum Comparators {
24+
;
25+
26+
public static int compare(long a, long b) {
27+
return a < b ? -1 : a == b ? 0 : 1;
28+
}
29+
30+
public static int compare(double a, double b, boolean asc) {
31+
return asc ? Double.compare(a, b) : Double.compare(b, a);
32+
}
33+
34+
}

‎src/main/java/org/elasticsearch/search/aggregations/bucket/Bucket.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.elasticsearch.search.aggregations.bucket;
2121

2222
import org.elasticsearch.ElasticsearchIllegalArgumentException;
23+
import org.elasticsearch.common.util.Comparators;
2324
import org.elasticsearch.search.aggregations.Aggregations;
2425
import org.elasticsearch.search.aggregations.metrics.MetricsAggregation;
2526

@@ -75,12 +76,7 @@ public String valueName() {
7576
public int compare(B b1, B b2) {
7677
double v1 = value(b1);
7778
double v2 = value(b2);
78-
if (v1 > v2) {
79-
return asc ? 1 : -1;
80-
} else if (v1 < v2) {
81-
return asc ? -1 : 1;
82-
}
83-
return 0;
79+
return Comparators.compare(v1, v2, asc);
8480
}
8581

8682
private double value(B bucket) {

‎src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramBase.java

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.search.aggregations.bucket.histogram;
2121

22+
import org.elasticsearch.common.util.Comparators;
2223
import org.elasticsearch.common.xcontent.ToXContent;
2324
import org.elasticsearch.search.aggregations.Aggregation;
2425

@@ -61,41 +62,37 @@ static abstract class Order implements ToXContent {
6162
public static final Order KEY_ASC = new InternalOrder((byte) 1, "_key", true, new Comparator<HistogramBase.Bucket>() {
6263
@Override
6364
public int compare(HistogramBase.Bucket b1, HistogramBase.Bucket b2) {
64-
if (b1.getKey() > b2.getKey()) {
65-
return 1;
66-
}
67-
if (b1.getKey() < b2.getKey()) {
68-
return -1;
69-
}
70-
return 0;
65+
return Comparators.compare(b1.getKey(), b2.getKey());
7166
}
7267
});
7368

7469
public static final Order KEY_DESC = new InternalOrder((byte) 2, "_key", false, new Comparator<HistogramBase.Bucket>() {
7570
@Override
7671
public int compare(HistogramBase.Bucket b1, HistogramBase.Bucket b2) {
77-
return -KEY_ASC.comparator().compare(b1, b2);
72+
return - Comparators.compare(b1.getKey(), b2.getKey());
7873
}
7974
});
8075

8176
public static final Order COUNT_ASC = new InternalOrder((byte) 3, "_count", true, new Comparator<HistogramBase.Bucket>() {
8277
@Override
8378
public int compare(HistogramBase.Bucket b1, HistogramBase.Bucket b2) {
84-
if (b1.getDocCount() > b2.getDocCount()) {
85-
return 1;
79+
int cmp = Comparators.compare(b1.getDocCount(), b2.getDocCount());
80+
if (cmp == 0) {
81+
cmp = Comparators.compare(b1.getKey(), b2.getKey());
8682
}
87-
if (b1.getDocCount() < b2.getDocCount()) {
88-
return -1;
89-
}
90-
return 0;
83+
return cmp;
9184
}
9285
});
9386

9487

9588
public static final Order COUNT_DESC = new InternalOrder((byte) 4, "_count", false, new Comparator<HistogramBase.Bucket>() {
9689
@Override
9790
public int compare(HistogramBase.Bucket b1, HistogramBase.Bucket b2) {
98-
return -COUNT_ASC.comparator().compare(b1, b2);
91+
int cmp = - Comparators.compare(b1.getDocCount(), b2.getDocCount());
92+
if (cmp == 0) {
93+
cmp = Comparators.compare(b1.getKey(), b2.getKey());
94+
}
95+
return cmp;
9996
}
10097
});
10198

‎src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,10 @@ public Number getKeyAsNumber() {
7979
}
8080

8181
@Override
82-
protected int compareTerm(Terms.Bucket other) {
83-
if (term > other.getKeyAsNumber().doubleValue()) {
84-
return 1;
85-
}
86-
if (term < other.getKeyAsNumber().doubleValue()) {
87-
return -1;
88-
}
89-
return 0;
82+
int compareTerm(Terms.Bucket other) {
83+
return Double.compare(term, other.getKeyAsNumber().doubleValue());
9084
}
85+
9186
}
9287

9388
private ValueFormatter valueFormatter;

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.elasticsearch.common.io.stream.StreamInput;
2323
import org.elasticsearch.common.io.stream.StreamOutput;
24+
import org.elasticsearch.common.util.Comparators;
2425
import org.elasticsearch.common.xcontent.XContentBuilder;
2526
import org.elasticsearch.search.aggregations.AggregationExecutionException;
2627
import org.elasticsearch.search.aggregations.Aggregator;
@@ -41,14 +42,11 @@ class InternalOrder extends Terms.Order {
4142
public static final InternalOrder COUNT_DESC = new InternalOrder((byte) 1, "_count", false, new Comparator<Terms.Bucket>() {
4243
@Override
4344
public int compare(Terms.Bucket o1, Terms.Bucket o2) {
44-
long i = o2.getDocCount() - o1.getDocCount();
45-
if (i == 0) {
46-
i = o2.compareTo(o1);
47-
if (i == 0) {
48-
i = System.identityHashCode(o2) - System.identityHashCode(o1);
49-
}
45+
int cmp = - Comparators.compare(o1.getDocCount(), o2.getDocCount());
46+
if (cmp == 0) {
47+
cmp = o1.compareTerm(o2);
5048
}
51-
return i > 0 ? 1 : -1;
49+
return cmp;
5250
}
5351
});
5452

@@ -59,7 +57,11 @@ public int compare(Terms.Bucket o1, Terms.Bucket o2) {
5957

6058
@Override
6159
public int compare(Terms.Bucket o1, Terms.Bucket o2) {
62-
return -COUNT_DESC.comparator(null).compare(o1, o2);
60+
int cmp = Comparators.compare(o1.getDocCount(), o2.getDocCount());
61+
if (cmp == 0) {
62+
cmp = o1.compareTerm(o2);
63+
}
64+
return cmp;
6365
}
6466
});
6567

@@ -70,7 +72,7 @@ public int compare(Terms.Bucket o1, Terms.Bucket o2) {
7072

7173
@Override
7274
public int compare(Terms.Bucket o1, Terms.Bucket o2) {
73-
return o2.compareTo(o1);
75+
return - o1.compareTerm(o2);
7476
}
7577
});
7678

@@ -81,7 +83,7 @@ public int compare(Terms.Bucket o1, Terms.Bucket o2) {
8183

8284
@Override
8385
public int compare(Terms.Bucket o1, Terms.Bucket o2) {
84-
return -TERM_DESC.comparator(null).compare(o1, o2);
86+
return o1.compareTerm(o2);
8587
}
8688
});
8789

‎src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
*/
3737
public abstract class InternalTerms extends InternalAggregation implements Terms, ToXContent, Streamable {
3838

39-
public static abstract class Bucket implements Terms.Bucket {
39+
public static abstract class Bucket extends Terms.Bucket {
4040

4141
long bucketOrd;
4242

@@ -58,20 +58,6 @@ public Aggregations getAggregations() {
5858
return aggregations;
5959
}
6060

61-
@Override
62-
public int compareTo(Terms.Bucket o) {
63-
long i = compareTerm(o);
64-
if (i == 0) {
65-
i = docCount - o.getDocCount();
66-
if (i == 0) {
67-
i = System.identityHashCode(this) - System.identityHashCode(o);
68-
}
69-
}
70-
return i > 0 ? 1 : -1;
71-
}
72-
73-
protected abstract int compareTerm(Terms.Bucket other);
74-
7561
public Bucket reduce(List<? extends Bucket> buckets, CacheRecycler cacheRecycler) {
7662
if (buckets.size() == 1) {
7763
return buckets.get(0);

‎src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.common.recycler.Recycler;
2626
import org.elasticsearch.common.text.StringText;
2727
import org.elasticsearch.common.text.Text;
28+
import org.elasticsearch.common.util.Comparators;
2829
import org.elasticsearch.common.xcontent.XContentBuilder;
2930
import org.elasticsearch.search.aggregations.AggregationStreams;
3031
import org.elasticsearch.search.aggregations.InternalAggregation;
@@ -80,15 +81,8 @@ public Number getKeyAsNumber() {
8081
}
8182

8283
@Override
83-
protected int compareTerm(Terms.Bucket other) {
84-
long otherTerm = other.getKeyAsNumber().longValue();
85-
if (this.term > otherTerm) {
86-
return 1;
87-
}
88-
if (this.term < otherTerm) {
89-
return -1;
90-
}
91-
return 0;
84+
int compareTerm(Terms.Bucket other) {
85+
return Comparators.compare(term, other.getKeyAsNumber().longValue());
9286
}
9387
}
9488

‎src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public Number getKeyAsNumber() {
7777
}
7878

7979
@Override
80-
protected int compareTerm(Terms.Bucket other) {
80+
int compareTerm(Terms.Bucket other) {
8181
return BytesRef.getUTF8SortedAsUnicodeComparator().compare(termBytes, ((Bucket) other).termBytes);
8282
}
8383
}

‎src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.elasticsearch.search.aggregations.bucket.terms;
2121

2222
import org.elasticsearch.common.text.Text;
23+
import org.elasticsearch.common.util.Comparators;
2324
import org.elasticsearch.common.xcontent.ToXContent;
2425
import org.elasticsearch.search.aggregations.Aggregation;
2526
import org.elasticsearch.search.aggregations.Aggregator;
@@ -59,11 +60,14 @@ static ValueType resolveType(String type) {
5960
}
6061
}
6162

62-
static interface Bucket extends Comparable<Bucket>, org.elasticsearch.search.aggregations.bucket.Bucket {
63+
static abstract class Bucket implements org.elasticsearch.search.aggregations.bucket.Bucket {
6364

64-
Text getKey();
65+
public abstract Text getKey();
66+
67+
public abstract Number getKeyAsNumber();
68+
69+
abstract int compareTerm(Terms.Bucket other);
6570

66-
Number getKeyAsNumber();
6771
}
6872

6973
Collection<Bucket> buckets();

‎src/main/java/org/elasticsearch/search/aggregations/support/FieldDataSource.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.lucene.util.InPlaceMergeSorter;
2727
import org.elasticsearch.common.lucene.ReaderContextAware;
2828
import org.elasticsearch.common.util.CollectionUtils;
29+
import org.elasticsearch.common.util.Comparators;
2930
import org.elasticsearch.index.fielddata.*;
3031
import org.elasticsearch.index.fielddata.AtomicFieldData.Order;
3132
import org.elasticsearch.script.SearchScript;
@@ -632,7 +633,7 @@ protected void swap(int i, int j) {
632633
protected int compare(int i, int j) {
633634
final long l1 = array[i];
634635
final long l2 = array[j];
635-
return l1 < l2 ? -1 : l1 == l2 ? 0 : 1;
636+
return Comparators.compare(l1, l2);
636637
}
637638
};
638639

0 commit comments

Comments
 (0)