[
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)
