diff --git a/src/main/java/org/elasticsearch/common/util/CollectionUtils.java b/src/main/java/org/elasticsearch/common/util/CollectionUtils.java index 49604532966..e4c1b690ae4 100644 --- a/src/main/java/org/elasticsearch/common/util/CollectionUtils.java +++ b/src/main/java/org/elasticsearch/common/util/CollectionUtils.java @@ -28,10 +28,6 @@ import org.apache.lucene.util.IntroSorter; public enum CollectionUtils { ; - private static int compare(long i, long j) { - return i < j ? -1 : (i == j ? 0 : 1); - } - public static void sort(LongArrayList list) { sort(list.buffer, list.size()); } @@ -50,7 +46,7 @@ public enum CollectionUtils { @Override protected int compare(int i, int j) { - return CollectionUtils.compare(array[i], array[j]); + return Comparators.compare(array[i], array[j]); } @Override @@ -60,7 +56,7 @@ public enum CollectionUtils { @Override protected int comparePivot(int j) { - return CollectionUtils.compare(pivot, array[j]); + return Comparators.compare(pivot, array[j]); } }.sort(0, len); diff --git a/src/main/java/org/elasticsearch/common/util/Comparators.java b/src/main/java/org/elasticsearch/common/util/Comparators.java new file mode 100644 index 00000000000..1f53593f411 --- /dev/null +++ b/src/main/java/org/elasticsearch/common/util/Comparators.java @@ -0,0 +1,34 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.util; + +/** Comparison utility methods. */ +public enum Comparators { + ; + + public static int compare(long a, long b) { + return a < b ? -1 : a == b ? 0 : 1; + } + + public static int compare(double a, double b, boolean asc) { + return asc ? Double.compare(a, b) : Double.compare(b, a); + } + +} diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/Bucket.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/Bucket.java index 6ee78eb39ee..cd5d377f0ff 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/Bucket.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/Bucket.java @@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.ElasticsearchIllegalArgumentException; +import org.elasticsearch.common.util.Comparators; import org.elasticsearch.search.aggregations.Aggregations; import org.elasticsearch.search.aggregations.metrics.MetricsAggregation; @@ -75,12 +76,7 @@ public interface Bucket { public int compare(B b1, B b2) { double v1 = value(b1); double v2 = value(b2); - if (v1 > v2) { - return asc ? 1 : -1; - } else if (v1 < v2) { - return asc ? -1 : 1; - } - return 0; + return Comparators.compare(v1, v2, asc); } private double value(B bucket) { diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramBase.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramBase.java index 6c64e7d766d..8125b5e912d 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramBase.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramBase.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.bucket.histogram; +import org.elasticsearch.common.util.Comparators; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.search.aggregations.Aggregation; @@ -61,33 +62,25 @@ interface HistogramBase extends Aggregation, Ite public static final Order KEY_ASC = new InternalOrder((byte) 1, "_key", true, new Comparator() { @Override public int compare(HistogramBase.Bucket b1, HistogramBase.Bucket b2) { - if (b1.getKey() > b2.getKey()) { - return 1; - } - if (b1.getKey() < b2.getKey()) { - return -1; - } - return 0; + return Comparators.compare(b1.getKey(), b2.getKey()); } }); public static final Order KEY_DESC = new InternalOrder((byte) 2, "_key", false, new Comparator() { @Override public int compare(HistogramBase.Bucket b1, HistogramBase.Bucket b2) { - return -KEY_ASC.comparator().compare(b1, b2); + return - Comparators.compare(b1.getKey(), b2.getKey()); } }); public static final Order COUNT_ASC = new InternalOrder((byte) 3, "_count", true, new Comparator() { @Override public int compare(HistogramBase.Bucket b1, HistogramBase.Bucket b2) { - if (b1.getDocCount() > b2.getDocCount()) { - return 1; + int cmp = Comparators.compare(b1.getDocCount(), b2.getDocCount()); + if (cmp == 0) { + cmp = Comparators.compare(b1.getKey(), b2.getKey()); } - if (b1.getDocCount() < b2.getDocCount()) { - return -1; - } - return 0; + return cmp; } }); @@ -95,7 +88,11 @@ interface HistogramBase extends Aggregation, Ite public static final Order COUNT_DESC = new InternalOrder((byte) 4, "_count", false, new Comparator() { @Override public int compare(HistogramBase.Bucket b1, HistogramBase.Bucket b2) { - return -COUNT_ASC.comparator().compare(b1, b2); + int cmp = - Comparators.compare(b1.getDocCount(), b2.getDocCount()); + if (cmp == 0) { + cmp = Comparators.compare(b1.getKey(), b2.getKey()); + } + return cmp; } }); diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java index df05d60d1b4..11582107810 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java @@ -79,15 +79,10 @@ public class DoubleTerms extends InternalTerms { } @Override - protected int compareTerm(Terms.Bucket other) { - if (term > other.getKeyAsNumber().doubleValue()) { - return 1; - } - if (term < other.getKeyAsNumber().doubleValue()) { - return -1; - } - return 0; + int compareTerm(Terms.Bucket other) { + return Double.compare(term, other.getKeyAsNumber().doubleValue()); } + } private ValueFormatter valueFormatter; diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java index 71599d0ef3d..1bd99cc3c2d 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java @@ -21,6 +21,7 @@ package org.elasticsearch.search.aggregations.bucket.terms; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.util.Comparators; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.Aggregator; @@ -41,14 +42,11 @@ class InternalOrder extends Terms.Order { public static final InternalOrder COUNT_DESC = new InternalOrder((byte) 1, "_count", false, new Comparator() { @Override public int compare(Terms.Bucket o1, Terms.Bucket o2) { - long i = o2.getDocCount() - o1.getDocCount(); - if (i == 0) { - i = o2.compareTo(o1); - if (i == 0) { - i = System.identityHashCode(o2) - System.identityHashCode(o1); - } + int cmp = - Comparators.compare(o1.getDocCount(), o2.getDocCount()); + if (cmp == 0) { + cmp = o1.compareTerm(o2); } - return i > 0 ? 1 : -1; + return cmp; } }); @@ -59,7 +57,11 @@ class InternalOrder extends Terms.Order { @Override public int compare(Terms.Bucket o1, Terms.Bucket o2) { - return -COUNT_DESC.comparator(null).compare(o1, o2); + int cmp = Comparators.compare(o1.getDocCount(), o2.getDocCount()); + if (cmp == 0) { + cmp = o1.compareTerm(o2); + } + return cmp; } }); @@ -70,7 +72,7 @@ class InternalOrder extends Terms.Order { @Override public int compare(Terms.Bucket o1, Terms.Bucket o2) { - return o2.compareTo(o1); + return - o1.compareTerm(o2); } }); @@ -81,7 +83,7 @@ class InternalOrder extends Terms.Order { @Override public int compare(Terms.Bucket o1, Terms.Bucket o2) { - return -TERM_DESC.comparator(null).compare(o1, o2); + return o1.compareTerm(o2); } }); diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java index c339e146e6a..a8f6d1f6a7d 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java @@ -36,7 +36,7 @@ import java.util.*; */ public abstract class InternalTerms extends InternalAggregation implements Terms, ToXContent, Streamable { - public static abstract class Bucket implements Terms.Bucket { + public static abstract class Bucket extends Terms.Bucket { long bucketOrd; @@ -58,20 +58,6 @@ public abstract class InternalTerms extends InternalAggregation implements Terms return aggregations; } - @Override - public int compareTo(Terms.Bucket o) { - long i = compareTerm(o); - if (i == 0) { - i = docCount - o.getDocCount(); - if (i == 0) { - i = System.identityHashCode(this) - System.identityHashCode(o); - } - } - return i > 0 ? 1 : -1; - } - - protected abstract int compareTerm(Terms.Bucket other); - public Bucket reduce(List buckets, CacheRecycler cacheRecycler) { if (buckets.size() == 1) { return buckets.get(0); diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java index b478c2df429..4f71f534fdb 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.recycler.Recycler; import org.elasticsearch.common.text.StringText; import org.elasticsearch.common.text.Text; +import org.elasticsearch.common.util.Comparators; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.search.aggregations.AggregationStreams; import org.elasticsearch.search.aggregations.InternalAggregation; @@ -80,15 +81,8 @@ public class LongTerms extends InternalTerms { } @Override - protected int compareTerm(Terms.Bucket other) { - long otherTerm = other.getKeyAsNumber().longValue(); - if (this.term > otherTerm) { - return 1; - } - if (this.term < otherTerm) { - return -1; - } - return 0; + int compareTerm(Terms.Bucket other) { + return Comparators.compare(term, other.getKeyAsNumber().longValue()); } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java index 889e1d34429..9f2cef5e1a1 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java @@ -77,7 +77,7 @@ public class StringTerms extends InternalTerms { } @Override - protected int compareTerm(Terms.Bucket other) { + int compareTerm(Terms.Bucket other) { return BytesRef.getUTF8SortedAsUnicodeComparator().compare(termBytes, ((Bucket) other).termBytes); } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java index a39526fe6ea..27d5a11fd82 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java @@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.bucket.terms; import org.elasticsearch.common.text.Text; +import org.elasticsearch.common.util.Comparators; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.search.aggregations.Aggregation; import org.elasticsearch.search.aggregations.Aggregator; @@ -59,11 +60,14 @@ public interface Terms extends Aggregation, Iterable { } } - static interface Bucket extends Comparable, org.elasticsearch.search.aggregations.bucket.Bucket { + static abstract class Bucket implements org.elasticsearch.search.aggregations.bucket.Bucket { - Text getKey(); + public abstract Text getKey(); + + public abstract Number getKeyAsNumber(); + + abstract int compareTerm(Terms.Bucket other); - Number getKeyAsNumber(); } Collection buckets(); diff --git a/src/main/java/org/elasticsearch/search/aggregations/support/FieldDataSource.java b/src/main/java/org/elasticsearch/search/aggregations/support/FieldDataSource.java index c74fc206f9e..553225acd0d 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/support/FieldDataSource.java +++ b/src/main/java/org/elasticsearch/search/aggregations/support/FieldDataSource.java @@ -26,6 +26,7 @@ import org.apache.lucene.util.BytesRefHash; import org.apache.lucene.util.InPlaceMergeSorter; import org.elasticsearch.common.lucene.ReaderContextAware; import org.elasticsearch.common.util.CollectionUtils; +import org.elasticsearch.common.util.Comparators; import org.elasticsearch.index.fielddata.*; import org.elasticsearch.index.fielddata.AtomicFieldData.Order; import org.elasticsearch.script.SearchScript; @@ -632,7 +633,7 @@ public abstract class FieldDataSource { protected int compare(int i, int j) { final long l1 = array[i]; final long l2 = array[j]; - return l1 < l2 ? -1 : l1 == l2 ? 0 : 1; + return Comparators.compare(l1, l2); } };