cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Branimir Lambov (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-11216) Range.compareTo() violates the contract of Comparable
Date Fri, 26 Feb 2016 18:29:18 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15169477#comment-15169477
] 

Branimir Lambov commented on CASSANDRA-11216:
---------------------------------------------

The first stage, and the lefts comparison isn't necessary at all (I assume you don't really
want to optimize for equality at the expense of slowing down all other comparisons). You get
0 in that case anyway.

I'd use
{code}
        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);
{code}

More importantly, comparison isn't consistent with equals, which will cause unexpected behaviour
with sorted sets. If we want to keep it that way, at lease we should give a big warning in
the method's and class's JavaDoc.

> Range.compareTo() violates the contract of Comparable
> -----------------------------------------------------
>
>                 Key: CASSANDRA-11216
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-11216
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: Jason Brown
>            Assignee: Jason Brown
>            Priority: Minor
>
> When running some quick-check style tests, I discovered that if both of the ranges being
compared wrap around, then the result of the comparison depends on which range is evaluated
first. For example, two ranges:
> A = { -1, 2 }
> B = { -2, 1 }
> and then compare them together:
> A.compareTo(B) == -1
> B.compareTo(A) == -1
> This is because the logic of the existing {{Range.compareTo()}} simply checks to see
if the {{this}} range wraps around, and returns -1. This bug does not appear to affect c*
until 3.0, and then only in one place ({{MerkleTrees.TokenRangeComparator#compare}}) that
I could identify.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message