cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jasobr...@apache.org
Subject [2/6] cassandra git commit: Range.compareTo() violates the contract of Comparable
Date Fri, 26 Feb 2016 22:51:56 GMT
Range.compareTo() violates the contract of Comparable

patch by jasobrown and blambov for CASSANDRA-11216


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/ecbeb084
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ecbeb084
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ecbeb084

Branch: refs/heads/cassandra-3.0
Commit: ecbeb0841e8627fa74a34346e0c380061588366a
Parents: 941d13d
Author: Jason Brown <jasedbrown@gmail.com>
Authored: Fri Feb 26 11:53:03 2016 -0800
Committer: Jason Brown <jasedbrown@gmail.com>
Committed: Fri Feb 26 14:47:57 2016 -0800

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 src/java/org/apache/cassandra/dht/Range.java    | 20 +++++++++++---------
 .../org/apache/cassandra/dht/RangeTest.java     | 15 +++++++++++++++
 3 files changed, 27 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/ecbeb084/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 9162165..aa3adf5 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.2.6
+ * Range.compareTo() violates the contract of Comparable (CASSANDRA-11216)
  * Avoid NPE when serializing ErrorMessage with null message (CASSANDRA-11167)
  * Replacing an aggregate with a new version doesn't reset INITCOND (CASSANDRA-10840)
  * (cqlsh) cqlsh cannot be called through symlink (CASSANDRA-11037)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/ecbeb084/src/java/org/apache/cassandra/dht/Range.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/dht/Range.java b/src/java/org/apache/cassandra/dht/Range.java
index f2c5996..34e91ea 100644
--- a/src/java/org/apache/cassandra/dht/Range.java
+++ b/src/java/org/apache/cassandra/dht/Range.java
@@ -32,6 +32,8 @@ import org.apache.cassandra.utils.Pair;
  * A Range is responsible for the tokens between (left, right].
  *
  * Used by the partitioner and by map/reduce by-token range scans.
+ *
+ * Note: this class has a natural ordering that is inconsistent with equals
  */
 public class Range<T extends RingPosition<T>> extends AbstractBounds<T>
implements Comparable<Range<T>>, Serializable
 {
@@ -254,18 +256,18 @@ public class Range<T extends RingPosition<T>> extends AbstractBounds<T>
implemen
        return left.compareTo(right) >= 0;
     }
 
+    /**
+     * Note: this class has a natural ordering that is inconsistent with equals
+     */
     public int compareTo(Range<T> rhs)
     {
-        /*
-         * If the range represented by the "this" pointer
-         * is a wrap around then it is the smaller one.
-         */
-        if ( isWrapAround(left, right) )
-            return -1;
-
-        if ( isWrapAround(rhs.left, rhs.right) )
-            return 1;
+        boolean lhsWrap = isWrapAround(left, right);
+        boolean rhsWrap = isWrapAround(rhs.left, rhs.right);
 
+        // if one of the two wraps, that's the smaller one.
+        if (lhsWrap != rhsWrap)
+            return Boolean.compare(!lhsWrap, !rhsWrap);
+        // otherwise compare by right.
         return right.compareTo(rhs.right);
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/ecbeb084/test/unit/org/apache/cassandra/dht/RangeTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/dht/RangeTest.java b/test/unit/org/apache/cassandra/dht/RangeTest.java
index 4255487..9fb49cf 100644
--- a/test/unit/org/apache/cassandra/dht/RangeTest.java
+++ b/test/unit/org/apache/cassandra/dht/RangeTest.java
@@ -661,4 +661,19 @@ public class RangeTest
     {
         return new Murmur3Partitioner.LongToken(t);
     }
+
+    @Test
+    public void testCompareTo_SameObject_WrapAround()
+    {
+        Range<Token> range = r(10, -10);
+        assertEquals(0, range.compareTo(range));
+    }
+
+    @Test
+    public void testCompareTo_BothWrapAround()
+    {
+        Range<Token> r0 = r(10, -10);
+        Range<Token> r1 = r(20, -5);
+        assertNotSame(r0.compareTo(r1), r1.compareTo(r0));
+    }
 }


Mime
View raw message