You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by ni...@apache.org on 2020/02/07 14:26:30 UTC
[kylin] 35/44: KYLIN-4327 TOPN Comparator may violate its general
contract
This is an automated email from the ASF dual-hosted git repository.
nic pushed a commit to branch 3.0.x
in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 7ea101db55d9524260ad8d9e63f2020f9ab32f71
Author: Yifei.Wu <va...@gmail.com>
AuthorDate: Mon Jan 6 11:28:47 2020 +0800
KYLIN-4327 TOPN Comparator may violate its general contract
---
.../org/apache/kylin/measure/topn/TopNCounter.java | 8 ++--
.../kylin/measure/topn/TopNCounterBasicTest.java | 47 ++++++++++++++++++++++
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/core-metadata/src/main/java/org/apache/kylin/measure/topn/TopNCounter.java b/core-metadata/src/main/java/org/apache/kylin/measure/topn/TopNCounter.java
index 8037eac..754fd55 100644
--- a/core-metadata/src/main/java/org/apache/kylin/measure/topn/TopNCounter.java
+++ b/core-metadata/src/main/java/org/apache/kylin/measure/topn/TopNCounter.java
@@ -238,18 +238,18 @@ public class TopNCounter<T> implements Iterable<Counter<T>>, java.io.Serializabl
}
}
- private static final Comparator ASC_COMPARATOR = new Comparator<Counter>() {
+ static final Comparator ASC_COMPARATOR = new Comparator<Counter>() {
@Override
public int compare(Counter o1, Counter o2) {
- return o1.getCount() > o2.getCount() ? 1 : o1.getCount() == o2.getCount() ? 0 : -1;
+ return Double.compare(o1.getCount(), o2.getCount());
}
};
- private static final Comparator DESC_COMPARATOR = new Comparator<Counter>() {
+ static final Comparator DESC_COMPARATOR = new Comparator<Counter>() {
@Override
public int compare(Counter o1, Counter o2) {
- return o1.getCount() > o2.getCount() ? -1 : o1.getCount() == o2.getCount() ? 0 : 1;
+ return Double.compare(o2.getCount(), o1.getCount());
}
};
diff --git a/core-metadata/src/test/java/org/apache/kylin/measure/topn/TopNCounterBasicTest.java b/core-metadata/src/test/java/org/apache/kylin/measure/topn/TopNCounterBasicTest.java
index 9d034fe..506ecf3 100644
--- a/core-metadata/src/test/java/org/apache/kylin/measure/topn/TopNCounterBasicTest.java
+++ b/core-metadata/src/test/java/org/apache/kylin/measure/topn/TopNCounterBasicTest.java
@@ -24,8 +24,11 @@ import static org.junit.Assert.assertTrue;
import java.util.Arrays;
import java.util.List;
+import org.junit.Assert;
import org.junit.Test;
+import com.google.common.collect.Lists;
+
public class TopNCounterBasicTest {
@Test
@@ -130,4 +133,48 @@ public class TopNCounterBasicTest {
}
}
+
+ @Test
+ public void testComparatorSymmetry() {
+ List<Counter> counters = Lists.newArrayList(new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 3d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 1d), new Counter<>("item", 1d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 1d),
+ new Counter<>("item", 0d), new Counter<>("item", 1d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 1d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 1d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 0d), new Counter<>("item", 2d), new Counter<>("item", 1d),
+ new Counter<>("item", 0d), new Counter<>("item", 0d), new Counter<>("item", 0d),
+ new Counter<>("item", 2d), new Counter<>("item", 4d), new Counter<>("item", 0d),
+ new Counter<>("item", 3d));
+ counters.sort(TopNCounter.ASC_COMPARATOR);
+ List<Double> expectedCounts = Lists.newArrayList(0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d,
+ 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d,
+ 0d, 0d, 0d, 0d, 0d, 0d, 1d, 1d, 1d, 1d, 1d, 1d, 1d, 2d, 2d, 3d, 3d, 4d);
+ List<Double> originCounts = Lists.newArrayList();
+ counters.stream().forEach(counter -> {
+ originCounts.add(counter.getCount());
+ });
+ Assert.assertArrayEquals(expectedCounts.toArray(), originCounts.toArray());
+
+ counters.sort(TopNCounter.DESC_COMPARATOR);
+ List<Double> expectedDescCounts = Lists.newArrayList(4d, 3d, 3d, 2d, 2d, 1d, 1d, 1d, 1d, 1d, 1d, 1d, 0d, 0d, 0d,
+ 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d,
+ 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d, 0d);
+ List<Double> originDescCounts = Lists.newArrayList();
+ counters.stream().forEach(counter -> {
+ originDescCounts.add(counter.getCount());
+ });
+ Assert.assertArrayEquals(expectedDescCounts.toArray(), originDescCounts.toArray());
+ }
}