cassandra-commits mailing list archives

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

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

Jason Brown commented on CASSANDRA-11216:
-----------------------------------------

Ok, if we don't need to worry about the lefts comparison, then I think your version makes
more sense.

WRT {{equals}}, you are correct that is not in sync with {{compareTo}}, both the existing
implementation as well as either of solutions suggested. According to the javadoc comment
for {{Comparable}}:

{quote}
It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y)).
Generally speaking, any class that implements the Comparable interface and violates this condition
should clearly indicate this fact. The recommended language is "Note: this class has a natural
ordering that is inconsistent with equals." 
{quote}

I think this is what you were referring to with "we should give a big warning ...". 

I can put together an updated patch with both of these changes.

> 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